Text in an overflow-x: auto container fails to render while scrolling after opening dev tools
Categories
(Core :: Panning and Zooming, defect, P2)
Tracking
()
People
(Reporter: hasan, Assigned: kats)
References
Details
Attachments
(9 files)
(deleted),
text/html
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
text/x-phabricator-request
|
Details |
User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:72.0) Gecko/20100101 Firefox/72.0
Steps to reproduce:
- Open test.html
- Open dev tools (hitting f12)
- Scrolling page
Actual results:
Region that I scrolled to was white until I clicked.
This behaviour goes away once you get rid of the overflow-x: auto on the pre element.
Expected results:
The scrolled to region should have the appropriately rendered text.
Comment 1•5 years ago
|
||
I can reproduce this issue on Windows 10 on the latest versions of the 4 main channels, however, this issue is somewhat partially reproduced on Mac OS. The content is rendered in Mac OS, just a little delayed.
Comment 2•5 years ago
|
||
To be clear, the devtools need to be docked to the bottom for the bug to repro. I have my devtools in a different window and I couldn't initially repro.
This smells like APZ rather than layout. I know that devtools has some interactions with APZ... Botond, any ideas?
Updated•5 years ago
|
Comment 3•5 years ago
|
||
What seems to be happening here is:
- Devtools disables APZ for the subframe (via this mechanism). APZ is still enabled for the root frame.
- As a result, we don't paint a displayport for the subframe, we only paint its viewport.
- Additionally, we clip the subframe's viewport to the viewport of the root frame. (This is to avoid rendering too much in case where a subframe has a huge viewport.)
- When you then scroll the root frame, it scrolls with APZ and paint skipping, but it brings into view parts of the subframe's viewport which haven't been rendered. Those parts are then never painted until something like a click event triggers a repaint.
Comment 4•5 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #3)
- Additionally, we clip the subframe's viewport to the viewport of the root frame. (This is to avoid rendering too much in case where a subframe has a huge viewport.)
I think conceptually the problem is here: we should be clipping the subframe's viewport to the displayport of the root frame. Then, the root frame can scroll with APZ and paint skipping just fine within the root frame's displayport. If we scroll the root frame far enough to move its displayport, we'll paint a new displayport including new regions of the subframe's viewport.
Comment 5•5 years ago
|
||
P2 as this is a perma-checkerboarding issue (albeit one we've been shipping for a while).
Assignee | ||
Comment 7•5 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #3)
What seems to be happening here is:
- Devtools disables APZ for the subframe (via this mechanism). APZ is still enabled for the root frame.
- As a result, we don't paint a displayport for the subframe, we only paint its viewport.
- Additionally, we clip the subframe's viewport to the viewport of the root frame. (This is to avoid rendering too much in case where a subframe has a huge viewport.)
- When you then scroll the root frame, it scrolls with APZ and paint skipping, but it brings into view parts of the subframe's viewport which haven't been rendered. Those parts are then never painted until something like a click event triggers a repaint.
This is mostly correct, except that strictly speaking, we do still have a displayport for the subframe. It's just that it uses zero margins to inflate (here) and so gets sized to the "display port base" size, rounded up to the nearest tile. The "display port base" size is computed in the block of code below this comment, and was the subject of much fiddling back in the day (see bug 1327095 and friends).
But yes, fundamentally the problem does seem to be that it uses the root composition bounds (i.e. the viewport of the root frame) as one of the clipping factors for the display port base, and increasing that to use the root displayport instead will likely fix the problem. Doing that might be nontrivial but I can try.
Assignee | ||
Comment 8•5 years ago
|
||
Braindump of current state:
- patches are at https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=7b25952b97afc2a34cc31701ffb185222727be72 and seem to work fine. I manually tested a few different scenarios including the case where the subframe has a CSS scale transform and things look good.
- trying to write a mochitest to exercise this bug is hard. My plan was to set up the situation that reproduces the checkerboard and then to somehow test for whether or not checkerboarding was happening
- setting up the checkerboard situation is working now, I had to add a DWU method to disable apz on an element but it wasn't too bad
- testing for checkerboarding is a little harder, because we need to compute the user-visible part of a scrollframe which involves walking up the APZ tree and doing intersections
- while doing this I discovered that the
IsCurrentlyCheckerboarding
function is not hooked up to anything
Assignee | ||
Comment 9•5 years ago
|
||
I finally got the mochitest working and hammered out all the intermittents I could find. Ran it with --verify
without the patch to ensure it failed consistently and then ran it with --verify
with the patch to validate the fix.
Try push is at https://treeherder.mozilla.org/#/jobs?repo=try&revision=552500bc43d22d23a8ed7fa5063228c871afe28e
Assuming that's good, I'll rebase to current master, do any clang-formatting/static analysis fixes, and put it up for review.
Assignee | ||
Comment 10•5 years ago
|
||
Hm, test doesn't work on android because I used wheel events which aren't supported there. I can probably adjust the test to not use wheel events, will give that a shot first.
Assignee | ||
Comment 11•5 years ago
|
||
Assignee | ||
Comment 12•5 years ago
|
||
If we can get the root frame's displayport, then we should use that
rect instead of the root composition bounds when clipping the scrollframe's
displayport. That way if APZ is disabled on the scrollframe, but the root
frame scrolls to bring a part of it into view, it will be fully painted and
not perma-checkerboard-y.
Note that this patch is the main fix, but leaves a bunch of comments/variables
with bad names; the next patch cleans that up.
Depends on D66420
Assignee | ||
Comment 13•5 years ago
|
||
Depends on D66421
Assignee | ||
Comment 14•5 years ago
|
||
Depends on D66423
Assignee | ||
Comment 15•5 years ago
|
||
Depends on D66424
Assignee | ||
Comment 16•5 years ago
|
||
Slight functional changes:
- the checkerboard event call site will now include mTestAsyncScrollOffset
when calculating the visible rect, which should impact overall behaviour if
there's a test that cares about checkerboard events (there currently isn't). - the IsCurrentlyCheckerboarding call site will use the compositing effective
scroll offset instead of the raw metrics scroll offset. This function is
not called from anywhere so it doesn't matter, but it makes sense to align
it with the other uses and I'll be using it in future patches.
Depends on D66425
Assignee | ||
Comment 17•5 years ago
|
||
This improves the implementation of IsCurrentlyCheckerboarding (which is not
invoked from anywhere prior to this patch) so that it takes into account the
recursive clipping applied by ancestor layers' composition bounds. In other
words, the visible rect for a layer may be additionally clipped because
ancestor scrollframes have scrolled, and this patch accounts for that.
It also records the currently-checkerboarding state into the APZTestData
at the time that the compositor APZTestData instance is fetched.
Depends on D66426
Assignee | ||
Comment 18•5 years ago
|
||
Depends on D66427
Comment 19•5 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #7)
But yes, fundamentally the problem does seem to be that it uses the root composition bounds (i.e. the viewport of the root frame) as one of the clipping factors for the display port base, and increasing that to use the root displayport instead will likely fix the problem. Doing that might be nontrivial but I can try.
So this means that we could get a "multiply effect", where the displayport base of a non-root scroll frame is set based on the displayport of the root, and then the displayport of the non-root element is expanded from the displayport base to it's displayport. The multiplication though is limited to that, we can't get a chain of displayport bases that are in turn based on an ancestor displayport, since we always look at the root displayport. So there is still some finite upper bound, which is the important part. We are increasing the upper bound though.
Assignee | ||
Comment 20•5 years ago
|
||
(In reply to Timothy Nikkel (:tnikkel) from comment #19)
So this means that we could get a "multiply effect", where the displayport base of a non-root scroll frame is set based on the displayport of the root, and then the displayport of the non-root element is expanded from the displayport base to it's displayport. The multiplication though is limited to that, we can't get a chain of displayport bases that are in turn based on an ancestor displayport, since we always look at the root displayport. So there is still some finite upper bound, which is the important part. We are increasing the upper bound though.
Yes, agreed.
Comment 21•5 years ago
|
||
Comment 22•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/296aad2e1950
https://hg.mozilla.org/mozilla-central/rev/275f9a696ae1
https://hg.mozilla.org/mozilla-central/rev/2e6aefd7a093
https://hg.mozilla.org/mozilla-central/rev/c1c55cb6cf31
https://hg.mozilla.org/mozilla-central/rev/ba8c8fe461d5
https://hg.mozilla.org/mozilla-central/rev/6af812a825de
https://hg.mozilla.org/mozilla-central/rev/35d9d70da6c9
https://hg.mozilla.org/mozilla-central/rev/0edab0f8fa94
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment 23•5 years ago
|
||
Successfully reproduced the issue on Firefox Nightly 75.0a1 (2020-02-22) under macOS 10.15 using the STR provided in Comment 0.
The issue is no longer reproducible on latest Nightly 77.0a1 (2020-04-15) and Firefox 76.0b5. Tests were performed on Windows 10 (x64), Ubuntu 18.04 (x64) and macOS 10.15.
Updated•4 years ago
|
Description
•