1548397 - Sticky position will hide a popup menu above its lexical container.

Status: RESOLVED FIXED

(Core :: Web Painting, defect, P3)

Description

[Douglas Morris]

Attachment: demo_ff66_sticky_bug.htm

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:66.0) Gecko/20100101 Firefox/66.0

Steps to reproduce:

Open the attached HTML document in the Firefox browser of your choice and follow the instructions to reproduce the bug. I have the problem using Firefox 66.0.3 on Linux.

Actual results:

The absolutely positioned element is not show over the document portion that is above the element's lexical container. See the code of the attached HTML document.

Expected results:

The absolutely positioned element should have remained visible regardless of where the document was scrolled vertically.

Comment 1

[Alice0775 White]

I can reproduce the issue on Windows10 Nightly68.0a1 if WebRender is disabled or HWA is disabled.
If WebRender is enabled, the problem is gone.

Regression window:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ce1a86d3b4db161c95d1147676bbed839d7a4732&tochange=b39904cff06b5b74798214ff8057a0ed2833fb0b

Regressed by: Bug 1404181

Comment 2

[Matt Woodrow (:mattwoodrow)]

Attachment: dump.txt

Thanks for the super detailed testcase!

Botond, do you know how the APZ side of scrolling position:sticky is supposed to work?

I've attached a display list and Layer tree dump, plus a compositor side dump after we've async scrolled.

We scroll the sticky layer (0x14290c800), and it appears that the resulting clip also scrolls. The layer has a scrolled-clip, but that has a y value of -255, so it seems that we're scrolling the clip from the metrics, which is not what I'd expect.

Scrolling the clip causes us to clip out some of the menu, depending on APZ scroll position.

I am struggling to reconcile this with the regression range though, which shouldn't have had any functional effects.

Comment 3

[Botond Ballo [:botond]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #2)

Botond, do you know how the APZ side of scrolling position:sticky is supposed to work?

In broad strokes:

• FrameLayerBuilder computes a "sticky scroll ranges" layer attribute for nsDisplaySticky items.
• AsyncCompositionManager runs the same processing as for fixed layers, with an extra step that looks at the sticky scroll ranges and makes an adjustment.

I've attached a display list and Layer tree dump, plus a compositor side dump after we've async scrolled.

We scroll the sticky layer (0x14290c800), and it appears that the resulting clip also scrolls. The layer has a scrolled-clip, but that has a y value of -255, so it seems that we're scrolling the clip from the metrics, which is not what I'd expect.

Scrolling the clip causes us to clip out some of the menu, depending on APZ scroll position.

I'll have a look at the dumps when I get a chance (leaving needinfo on me for now), but cc'ing Markus in the meantime as he might be able to answer your question about which clip should be scrolled.

Comment 4

[Botond Ballo [:botond]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #2)

We scroll the sticky layer (0x14290c800), and it appears that the resulting clip also scrolls. The layer has a scrolled-clip, but that has a y value of -255, so it seems that we're scrolling the clip from the metrics, which is not what I'd expect.

Ok, I refreshed myself on the various types of clips on layers and scroll metadata, and I agree, the clip from the scroll metadata should be scrolled.

It's not immediately clear from code inspection what is scrolling the clip here, but tracking that down seems like a good next step.

Comment 5

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #4)

It's not immediately clear from code inspection what is scrolling the clip here, but tracking that down seems like a good next step.

The clip (x=0, y=0, w=3330, h=1764) appears in two different places on the layer: as Layer::mClipRect, and as ScrollMetadata::mScrollClip. It's the first instance that gets scrolled, and the result intersected with the (unscrolled) second instance, resulting in the shadow clip of (x=0, y=2, w=3330, h=1762).

Comment 6

[Botond Ballo [:botond]]

This comment describes the conceptual model that governs which clips are affected by which transforms.

Layer::mClipRect is the "local clip", which is described as follows:

        //  - Its local clip, which is fixed to the layer contents, i.e. it
        //    moves with those async transforms which the layer contents move
        //    with.

In this case, the layer contents are scrolling, so we scroll Layer::mClipRect as well. So, the behaviour is consistent with the model.

This suggests that either the layer shouldn't have this clip in Layer::mClipRect, or that the model is wrong for sticky elements. Markus, do you know which is the case?

Comment 7

[Douglas Morris]

(In reply to Matt Woodrow (:mattwoodrow) from comment #2)

Thanks for the super detailed testcase!

You're welcome. I use Firefox regularly and am grateful. Browser technology confuses me for mere Web page design, and I know the simple truth matters with what matters.

Comment 8

[Patricia Lawless]

Bulk change to wontfix for 68 (P3+ carryover with needinfo).

Comment 9

[Kartikaya Gupta (email:[e-mail])]

Attachment: Screenshot of buggy behaviour

Bug still manifests in a recent nightly, but only if WR is disabled. Here's a screenshot since it's not totally obvious from the textual description what to look for, in the case where the bug doesn't manifest.