User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:104.0) Gecko/20100101 Firefox/104.0

Steps to reproduce:

See example: https://codepen.io/tombigel/pen/xxEPxvg?editors=0100

I have an element with position: sticky inside a parent with clip-path: inset(0), or same with overflow: clip.

Actual results:

when scrolling it's not painting it correctly as I scroll.

Expected results:

Should always paint the clipped area of the sticky positioned element.

Seems to date back to our initial implementation of sticky.

So, position: fixed used to be similarly broken, and bug 1382534 seemed to fix it: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e81094853e1d7035db30a54deaf60a0108924b85&tochange=b2f459b88cab5525c785a7fa70a01be3e9cdcb23

In the reduced test-case, something like this helps make the bug not reproduce:

diff --git a/layout/painting/nsDisplayList.cpp b/layout/painting/nsDisplayList.cpp
index 6949daddfe8c8..e47cb5a929103 100644
--- a/layout/painting/nsDisplayList.cpp
+++ b/layout/painting/nsDisplayList.cpp
@@ -1585,10 +1585,7 @@ static bool IsStickyFrameActive(nsDisplayListBuilder* aBuilder,
   MOZ_ASSERT(aFrame->StyleDisplay()->mPosition ==
              StylePositionProperty::Sticky);
 
-  StickyScrollContainer* stickyScrollContainer =
-      StickyScrollContainer::GetStickyScrollContainerForFrame(aFrame);
-  return stickyScrollContainer &&
-         stickyScrollContainer->ScrollFrame()->IsMaybeAsynchronouslyScrolled();
+  return true;
 }
 
 bool nsDisplayListBuilder::IsAnimatedGeometryRoot(nsIFrame* aFrame,

I'm unsure what that means in practice tho, not familiar with AGRs... Even with that the bug still repros when the window is unfocused. Tim / Botond (when you're back, no rush), do you know what is the right fix here?

In bug 1382534, the problem was that a mask display item had an incorrect ASR (active scrolled root), and the fix was to give it a different ASR (the rest of the work in that bug was about computing a correct clip to go with the ASR).

However, the codepath modified in comment 5 seems not to be related to ASR assignments, but rather only used by retained-DL? Which makes me wonder what the behaviour is with retained-DL disabled.

It reproduces with retained DL disabled. The code in comment 5 doesn't seem only related to retained DL?

The only call chain of IsStickyFrameActive that I could find using Searchfox is:

RetainedDisplayListBuilder::ProcessFrame
  nsDisplayListBuilder::FindAnimatedGeometryRootFrameFor
    nsDisplayListBuilder::IsAnimatedGeometryRoot

pin @emilio any update on this?

What I'm seeing in local testing is:

• The change in comment 5 does not fix the bug (or change anything about when in reproduces, as far as I could tell) for me.
• The bug does reproduce with retained DL disabled for me.

One note: the bug reproduces reliably if you click on the page (presumably causing a repaint) after scrolling down and before scrolling back up.

Here is a testcase that's further reduced still.

One note: the bug reproduces reliably if you click on the page (presumably causing a repaint) after scrolling down and before scrolling back up.

Slight clarification here: the bug reproduces reliably if you click below the green area after scrolling down and before scrolling back up.

Here is the Gecko display list when the page is scrolled to the top after initially loading the page (rendering is correct).

Here is the display list with the page scrolled to the top after first scrolling down and clicking below the green area, i.e. the last time the page was painted was when it was scrolled down. Here, rendering is incorrect.

I'm not spotting anything particularly surprising about the display lists, the only things that differ are vertical offsets reflecting the fact that in the second case we were scrolled down at the time of the paint. There are no differences related to ASRs or clips (beyond their offsets).

One thing I've noticed is that async scrolling after the repaint does scroll the green content, but how much it scrolls it is limited, with the limit related to the value of the margin-bottom: as the margin-bottom approaches 0, it's less limited (such that at 0 the problem goes away completely), whereas as the margin-bottom approaches -90vh (90vh being the height of the parent comp element), it's more limited (such that at 90vh you can cause the green area to not be rendered at all even when scrolled to the top, if you've scrolled it completely out of view at the time of the repaint).

This is making me think there is a clip that should be scrolling in the compositor but isn't.

The previous comment was based on a slight misunderstanding of what the page is doing.

I'm uploading a revised testcase which uses a gradient rather than a solid color as the sticky element's background, which makes it more obvious whether the sticky element is itself being scrolled vs. staying in place and just having its clip be scrolled.

If you scroll this page, you can see there are two distinct phases: initially, only the clip scrolls (the background stays in place), and then at one point (once the margin-box of the sticky element, which is only 40vh high due to the margin-bottom: -50vh, reaches the bottom edge of the #comp element which is the sticky element's containing block), the sticky element itself starts to scroll out of view.

Similarly, when scrolling back up, first the sticky element scrolls partially into view, and then the clip starts scrolling to reveal more of it.

The rendering bug occurs if a repaint was last triggered at the time the clip was partially clipping away the sticky element. After such a repaint, async scrolling successfully moves the clip but any additional area revealed in this way is blank (until the next repaint).

So, given that, I think a revised diagnosis is more along the lines of us applying a clip to a display item prematurely during painting, not recognizing that the clip can scroll asynchronously and we should paint the entire item.

Discussed this with Markus today.

The problem here is that the StickyPosition display item's ASR is the root scroll frame's ASR (this is how we currently model position:sticky in the ASR model), and the clip that applies to this item from the overflow:clip is also the root scroll frame's ASR (since that clip scrolls with the root scroll frame).

As a result, we think that the clip and item scroll together, and so if the clip obscures part of the item at display list building time, it will continue to obscure that part of the item even after async scrolling.

However, this is not in fact the case -- while the clip scrolls with the root scroll frame, the sticky element only does so when it's not in its "stuck" position, and therefore it doesn't really scroll together with the clip.

Unfortunately this does not seem like an easy fix -- it seems like fixing this would require the refactor described in bug 1730749.

I'll also mention that I discovered two reftests in our test suite that exercise a very similar scenario:

position-sticky-scrolled-clip-1.html
position-sticky-scrolled-clip-2.html

They use the clip property rather than overflow:clip, but the effect at the display list level is the same: we have a sticky display item with a scrolled clip that sometimes moves relative to the item.

The first test is only testing that the clip can in fact be scrolled in the compositor, and it passes. The second test is basically testing the same scenario discussed in this bug, and it's marked as a known failure, and has been ever since it was added in bug 1298218, the bug that introduced the ASR model.

Canceling needinfo on Timothy as the original question has been substantially answered.

(But perhaps you might find the diagnosis here, and the additional motivation for the refactor described in bug 1730749, an interesting consideration to keep in mind as part of the ongoing/planned display list refactoring work?)

Thanks so much for this Botond! We are watching this with great anticipation (:

(In reply to maggotfish from comment #20)

We are watching this with great anticipation (:

As mentioned at the end of comment 17, this is unfortunately not likely to be an easy fix as it depends on a refactor (bug 1730749) that will require a larger amount of work.

To help inform the prioritization of this work, I have a few questions for you:

• Does this issue affect a website currently in production? If so, can you identify it, and/or give a sense of its size (order-of-magnitude estimate of # of users or # of visits)?
• Are you aware of other affected websites?
• Are you aware of any workarounds you can employ?

Thanks!

As mentioned at the end of comment 17, this is unfortunately not likely to be an easy fix as it depends on a refactor (bug 1730749) that will require a larger amount of work.

Yes, it's understood (: but still wanted to add an optimistic tone.

Does this issue affect a website currently in production? If so, can you identify it, and/or give a sense of its size (order-of-magnitude estimate of # of users or # of visits)?

No that we know of. This is mostly blocking us from replacing our current scroll effects technique with a better one that should allow us adding more features and possibly improve the effects' performance.

Are you aware of other affected websites?

No.

Are you aware of any workarounds you can employ?

Alternatively, if Gecko ships the new scroll-animations per spec we may be able to avoid resorting to this technique.

Thanks!

(In reply to maggotfish from comment #22)

Alternatively, if Gecko ships the new scroll-animations per spec we may be able to avoid resorting to this technique.

Support for scroll-driven animations is definitely planned, with significant implementation work done already. The tracking issue for that is bug 1676779.

The following field has been copied from a duplicate bug:

For more information, please visit auto_nag documentation.

Set release status flags based on info from the regressing bug 1382534

Set release status flags based on info from the regressing bug 1382534