1840166 - Difference in scroll anchoring suppression between Firefox and Chrome

Status: NEW

(Core :: Layout: Scrolling and Overflow, defect, P3)

Description

[Ksenia Berezina [:ksenia] on leave until 09/15]

This was originally reported in https://github.com/webcompat/web-bugs/issues/123254

To reproduce, visit https://www.tvmovie.de/tv/fernsehprogramm in Nightly 116.0a1 (2023-06-23) and try to scroll down.

Actual:
Scrollbar returns to the top of the page

Expected:
Scrollbar stays in the same position where you finished scrolling

From mozregression: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=5eb0c339e4a6c919642f1e70d15347b766ceec00&tochange=4adf1e46346688c72a6dbaeb5e965cfee658401b

Comment 1

[Ksenia Berezina [:ksenia] on leave until 09/15]

Hello Hiroyuki, wonder if you could take a look at this, please?

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Attachment: A minimized test case

It still depends on sticky-kit jQuery plugin though

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: An example demonstrating an issue which triggers unexpected scroll anchoring

I suppose this is a sort of our table layout issue.

Attaching file is an example to see what's going on there basically.

In the example, in every 2000ms we do either one of the following repeatedly

1. making a table-row element position:fixed and inserting an element to compensate the table-row element
2. making the table-row element back to position: static and removing the element for compensation

Basically with these steps, the table height should be unchanged, but somehow we layout a display: table-cell element in a different row differently. You can easily see it on the example. "Bella Italia - Camping auf Deutsch" is the text inside the table-cell element.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Moving into Layout:Tables. Setting P3:S3 for now since I believe such kind of cases (i.e. using sticky-kit with table layout) are rarely used. If it's widely used we should bump the severity.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Hiro, your test-case in comment 4 is hitting the "disable scroll anchoring" heuristic, do you know why the page is not hitting it? I also don't quite follow, the example in comment 4 prints:

110.33332824707031 != 110.33332824707031?

For me. So there's something else going on in that site. Maybe we should flip the pref you added in non-Nightly to get to the bottom of this?

Comment 7

[Emilio Cobos Álvarez (:emilio)]

I also don't think this is S3, this makes the page unusable.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The component has been changed since the backlog priority was decided, so we're resetting it.
For more information, please visit BugBot documentation.

Comment 9

[Hiroyuki Ikezoe (:hiro)]

IIRC the sticky-ish element is the anchor node itself, thus our scroll anchoring implementation thinks the anchor has been moved, and unfortunately the sticky-kit library changes the sticky-ish element position repeatedly (with forcible layout flush) back and forth due to the table height difference (i.e. the library thinks the content height has been changed, thus it tries to align the stick-ish element again and again..).

(In reply to Emilio Cobos Álvarez (:emilio) from comment #6)

For me. So there's something else going on in that site. Maybe we should flip the pref you added in non-Nightly to get to the bottom of this?

I am reluctant disable it for this specific site. We should rather figure out a way to fix this janky sticky position scrolling issue with our web intervention extension?

I also start thinking our behavior on this table-layout: fixed difference may be correct, given that each table column width depends on the first row cell width..

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

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

Comment 11

[Daniel Holbert [:dholbert]]

Just ran across this in our S2 bug triage list. hiro, do you have cycles to push this forward at some point this half, since you've looked a bit? If not, I can try to find an owner for it. Thanks!

Comment 12

[Hiroyuki Ikezoe (:hiro)]

Attachment: A proper test acse without using sticky-kit jQuery plugin

I was wrong. This is truly an issue in our scroll anchor implementation, specifically scroll anchoring suppression.

Here is a test case to reproduce the issue. Once after you scrolled down the content, it keeps scrolling.

So what the issue in the scroll anchoring suppression is, it's asymmetric in this test case.

The test case does flip position value from "fixed" to "static" and from "static" back to "fixed" repeatedly, but in the case of flipping to "static" the target element gets removed and re-inserted at the same time, thus we don't have nsChangeHint_ReconstructFrame change hint for the change, thus we do suppress scroll anchoring in the case of "static" to "fixed", but we don't in the case of "fixed" to "static", it keeps scrolling in a single direction unfortunately.

Emilio, do you have any ideas how we address this asymmetric of the scroll anchoring suppression?

Comment 13

[Hiroyuki Ikezoe (:hiro)]

I figured out a way to fix this issue by using our webcompat intervention extension.

.has-sticky * {
  overflow-anchor: none ! important;
}

Adding this style to the site would solve the issue.

Ksenia, would you mind adding this into our intervention extension? Thanks!

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Should still try to look into it tho.

:root {
  overflow-anchor: none;
}

Would be a more efficient intervention.

Comment 15

[Ksenia Berezina [:ksenia] on leave until 09/15]

Ksenia, would you mind adding this into our intervention extension? Thanks!

Sounds good, I filed bug1868345 for the intervention.

Comment 16

[Dennis Schubert [:denschub]]

Firefox 123 now includes an intervention for this.

Comment 17

[Daniel Holbert [:dholbert]]

Retriaging as S3 given that we're shipping an intervention for the known-affected site. The intervention applies the fix that we identified.

If we run across other affected sites, it might make sense to retriage as S2 again [or ship more interventions :) ].

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

The severity field for this bug is set to S3. However, this bug has a P1 WebCompat priority.
:hiro, could you consider increasing the severity of this web compatibility bug?

For more information, please visit BugBot documentation.

Comment 19

[Hiroyuki Ikezoe (:hiro)]

I'd keep this bug as S3, given that the Webcompat score is quite low, 10.

Comment 20

[Hsin-Yi Tsai (she/her) | away [:hsinyi]]

I changed the diagnosis-team to layout, which aligns with the current component.