New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
When performing subtree layout, perform scroll anchoring notifications on ancestors. #20141
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The review process for this patch is being conducted in the Chromium project.
902b356
to
5c750fb
Compare
…s on ancestors. Previously, we would fail to call NotifyBeforeLayout for all such ancestors that are scrollers, causing subtree layouts to completely fail to maintain a scroll anchor for such ancestors. Bug: 920289 Change-Id: I899634949739f5d4f3af1f282b598cbc4ef0e3e4 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1902767 Commit-Queue: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: Nick Burris <nburris@chromium.org> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Cr-Commit-Position: refs/heads/master@{#713561}
5c750fb
to
2f13872
Compare
Looks like a flaky failure in Firefox:
I'm a little surprised not to see the screenshot dumped into the logs (do we not do that for the stability checks?), so I'm going to run this locally on Firefox and see if I can reproduce before pinging the Chromium author. |
Stability checks eat most of the logs because the log size was occasionally massive (when we were on Travis it would blow through the 50Mb limit). We could make screenshots come through, but some work would be required. |
Ah, it looks like the class includes reftest-wait.js, but is missing any use of the |
That appeared to have worked (stability checks now pass, CL was merged automatically). FYI @chrishtr - I had to make a slight modification to your patch, see 904b1cf. This should require no action from you; the next WPT import will just import the diff and update Chromium accordingly. Please just take a quick look to make sure my change is sane, thanks. |
Looks good. |
Previously, we would fail to call NotifyBeforeLayout for all such ancestors
that are scrollers, causing subtree layouts to completely fail to maintain
a scroll anchor for such ancestors.
Bug: 920289
Change-Id: I899634949739f5d4f3af1f282b598cbc4ef0e3e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1902767
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Nick Burris <nburris@chromium.org>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713561}