Closed Bug 1827337 Opened 1 year ago Closed 1 year ago

put overlay scrollbars only above content they scroll

Categories

(Core :: Web Painting, defect)

defect

Tracking

()

RESOLVED FIXED
114 Branch
Tracking Status
firefox114 --- fixed

People

(Reporter: tnikkel, Assigned: tnikkel)

References

Details

Attachments

(1 file)

So in bug 1756831 we had a case where overlay scrollbars being overtop of all content descendants, whether or not those content descendants were out of flow with a parent outside the scroll frame and hence weren't scrolled by those scrollbars. There was another bug in the code that showed overlay scrollbars, it would activate when we are over these content descendants that aren't scrolled by the scrollbars. So we fixed that bug in bug 1756831 which fixed most causes of those scrollbars showing over content they don't scroll. And we left actually moving down the scrollbars below content they don't scroll to a followup bug, because that code is delicate and has been changed many times over the years. Bug 1756831, comment 21 has a nice explanatory comment. This is the follow up bug.

Since then we've had a couple more cases of this come up (bug 1823418, bug 1803910).

I did some research into the history of this area to make sure we don't regress anything.

Bug 461843 and https://hg.mozilla.org/mozilla-central/rev/0cf0c2f57f29 (Oct 2010) first did something different for overlay scrollbars. Before that patch we would add the scrollbars to the background layer before descending into the scrollframe, this was fine because we reserved space for the scrollbars. That patch added the scrollbars to the background layer after descending into the scrollframe, thereby putting them on top of the background layer.

Bug 636564 and https://hg.mozilla.org/mozilla-central/rev/7f8c69ad0895 (Dec 2011) put overlay scrollbars in the positioned descendants list.

Bug 926292 (Mar 2014) then looks at the positioned descendants list and calculates the max zindex in it to use for the scrollbars. Of note here is bug 926292, comment 15, which is a nice overview of the problem. It called for a slightly different solution then what got implemented, it called for "if the scrolled element has any visible positioned descendants that are scrolled by the element, put the overlay scrollbars above the topmost such positioned descendants in z-order", the difference being the "positioned descendants that are scrolled by the element" part, which we aim to correct with this bug.

A related case is the hit test information for inactive scroll frames so that they can be activated. It needs to be above the things that it scrolls so that it can get hit, but it needs to be below other content that it doesn't scroll. There's history of this involving scroll container items and merging of them and containerful scrolling, but we'll skip past that and pick things up in bug 1087453 (Oct 2014). This is where we position this layer for containerless scrolling. We put it in the outlines list, unless there are positioned descendants, then we put it in the positioned descendants list, but only above things that we actually scroll. This is the first time that distinction has been introduced into the code I think. I think this was because I had to deal with many such issues during containerful scrolling on subframes.

Bug 1448841 (Mar 2018). We are now in the retained display list era. Having display items that depend on other content in the display list is not good for the merging step of retained display lists as we have to make sure to build all of that content when doing an incremental update, which is difficult/a lot of rebuilding. To fix this the hit info for inactive scroll frames is moved to INT32_MAX zindex. For overlay scrollbars, partial updates are just disabled if we encounter one (the current partial update will succeed the first time we build an overlay scrollbar, but the next paint will forced to be a full rebuild, this avoids the problematic step of trying to merge with an overlay scrollbar in the existing and new list.) Bug 1448841, comment 8 is a good overview comment.

Bug 1460482 (May 2018) walks back disabling retained display lists for all overlay scrollbars: it allows retained display list for root scrollbars, which is okay because those have to be on top of all content.

Bug 1645433 (Nov 2020) moves the hit info for inactive scrollframes back down from INT32_MAX due to it being above other content that should be targeted. See bug 1645433, comment 13 for diagnosis, and bug 1645433, comment 16 for the solution. The solution is to look at the existing display items on the element and including that in the max z-index calculation so that the item can only move up in the z-index, which makes it okay.

So given that, I don't think there should be any issue making the proposed change, in particular there shouldn't be an issue with RDL because RDL is disabled for overlay scrollbars.

Pushed by tnikkel@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9dbc4cd2913b
Put overlay scrollbars only above content they scroll. r=mstange
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 114 Branch
Duplicate of this bug: 1823418
Blocks: 1829516
Duplicate of this bug: 1803910
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: