1777619 - Firefox and Chrome differ on scroll (anchor?) position when content shrinks and then grows such that scrollable height changes

Status: NEW

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

Description

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

STR:

1. Load attached testcase.
2. Hover the yellow area, and mousewheel-scroll to the end.
3. Click the button to activate a larger line-height.
4. Scroll to the end again (with the now-taller lines).
5. Click the button to restore the original line-height.
   (Note that the scrollable area now looks like it did in step 2.)
6. Click the button once more, to grow the lines again.

Intuitively I'd expect the rendering to look the same after step 3 and after step 6, but the rendering is different between those points, in both Firefox and Chrome. And moreover, Firefox and Chrome disagree with each other.

CHROME RESULTS (as best as I can undesrstand them):

• After step 3: the scrollbar is not at the end; instead, the text at the top of the scrollport seems to be chosen as an "anchor" and the lines expand downward from that point, giving us a good ways that we can scroll.
• After step 6: the scrollbar is at the end. It seems like maybe they have a "memory" of the earlier large scrollTop value (from step 4 when we scrolled to the end with tall lines), which became out-of-bounds in step 5 but then became in-bounds again in step 6?

FIREFOX RESULTS:

• After step 3: same as Chrome (see above)
• After step 6: the scrollbar is near the end. On my system, we go from showing lines 35-49 (before step 6) to showing lines 42-46 (after step 6). I don't have any intuition about where this scroll position comes from.

I think this is essentially the source of our remaining behavior-difference that I discussed in bug 1515255 comment 13. (In that case the height change is coming from text rewrapping rather than line-height adjustments, but I think it's the same underlying phenomenon.)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: screencast showing the difference between Chrome (left) vs. Firefox (right)

Here's a screencast to demonstrate the behavior-difference.

In this screencast:

• I do the "setup", i.e. steps 1-5, in Chrome and then in Firefox.
• Then I do step 6 (the final button-click) in Chrome, and then Firefox.

We look the same for steps 1-5 but then have a different outcome in step 6, as you can see.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Note that on Chrome (on Linux FWIW), I do sometimes see a jump similar to the Firefox one when I pressed the button after I tried to scroll down further even if it has reached to the bottom edge (after the step 6). The jumped scroll position is pretty much same as the jumped position at step 3 , FWIW.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

(In reply to Daniel Holbert [:dholbert] from comment #0)

FIREFOX RESULTS:

• After step 3: same as Chrome (see above)
• After step 6: the scrollbar is near the end. On my system, we go from showing lines 35-49 (before step 6) to showing lines 42-46 (after step 6). I don't have any intuition about where this scroll position comes from.

I can explain it. After step 4, Firefox chooses "line 45" as the scroll anchor node, then after step 5, the node is still the selected node, thus after step 6, Firefox tries to keep the node position as it is. You can see it by carefully looking at "line 45" position in the video in comment 1.

The code in question here in ScrollAnchorContainer::ApplyAdjustments. We try to reuse the last selected anchor node after scroll anchoring happened. If we call InvalidateAnchor() rather than the reusing the last anchor node, the behavior will match Chrome.

Daniel, do you think it's worth trying it (behind a pref) and see whether it fixes the facebook case (bug 1779404) or not?

Comment 4

[Timothy Nikkel (:tnikkel)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #3)

The code in question here in ScrollAnchorContainer::ApplyAdjustments. We try to reuse the last selected anchor node after scroll anchoring happened. If we call InvalidateAnchor() rather than the reusing the last anchor node, the behavior will match Chrome.

Daniel, do you think it's worth trying it (behind a pref) and see whether it fixes the facebook case (bug 1779404) or not?

I can reproduce at will so if you provide a patch I can test.

Comment 5

[Daniel Holbert [:dholbert]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #3)

I can explain it. After step 4, Firefox chooses "line 45" [...] carefully looking at "line 45" position in the video

Thank you -- yeah, that clarifies what's going on behind the scenes.

Daniel, do you think it's worth trying it (behind a pref) and see whether it fixes the facebook case (bug 1779404) or not?

Sounds like tnikkel can get us faster results here, given a proposed patch. :)

But yeah -- if he ends up no-longer-being-able to reliably repro etc, then it seems quite-reasonable to land the experimental change behind a pref & ask for feedback.

Without having dug too deeply, I'm assuming your proposed InvalidateAnchor call would represent a small perf cost [perhaps imperceptible]. Probably worth it that cost, it gets us real-world webcompat/intuitiveness wins. We should also see what [if anything] the spec says about this scenario, and maybe file a spec bug if needed.

Comment 6

[Timothy Nikkel (:tnikkel)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #3)

The code in question here in ScrollAnchorContainer::ApplyAdjustments. We try to reuse the last selected anchor node after scroll anchoring happened. If we call InvalidateAnchor() rather than the reusing the last anchor node, the behavior will match Chrome.

I replaced the highlighted lines in the searchfox link with an InvalidateAnchor() call (was that the intended change?) and I was still able to reproduce the scroll jumps on fb.

Comment 7

[Timothy Nikkel (:tnikkel)]

(In reply to Timothy Nikkel (:tnikkel) from comment #6)

(In reply to Hiroyuki Ikezoe (:hiro) from comment #3)

The code in question here in ScrollAnchorContainer::ApplyAdjustments. We try to reuse the last selected anchor node after scroll anchoring happened. If we call InvalidateAnchor() rather than the reusing the last anchor node, the behavior will match Chrome.

I replaced the highlighted lines in the searchfox link with an InvalidateAnchor() call (was that the intended change?) and I was still able to reproduce the scroll jumps on fb.

The scroll jumps might have gotten worse actually with that change? I'm not sure, I only tested enough to see a jump. If that's a useful finding I can test more to confirm or disprove that.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Attachment: WIP: Bug 1777619 - Invalidarte the scroll anchor node in the case of scroll anchoring itself.

Comment 9

[Hiroyuki Ikezoe (:hiro)]

https://phabricator.services.mozilla.com/D156368 is what I had in my mind. But if it makes the Facebook case worse, it's not worth trying.

(In reply to Daniel Holbert [:dholbert] from comment #5)

Without having dug too deeply, I'm assuming your proposed InvalidateAnchor call would represent a small perf cost [perhaps imperceptible]. Probably worth it that cost, it gets us real-world webcompat/intuitiveness wins. We should also see what [if anything] the spec says about this scenario, and maybe file a spec bug if needed.

In terms of the spec, I think this section is the relevant part. There's description;

Conceptually, a new anchor node is computed for every scrolling box whenever the scroll position of any scrolling box changes.

So, I understand it means that the new anchor needs to be re-computed again even if the scroll position change is caused by scroll anchoring.

Comment 10

[Timothy Nikkel (:tnikkel)]

Unfortunately, I can still reproduce a scroll jump on fb with the patch Hiro posted.

Comment 11

[Botond Ballo [:botond]]

Reclassifying as a defect since it seems like this difference in scroll anchor position has the potential to cause web compat issues, but marking as S3 since, based on comment 10, it's not the cause of the Facebook jumping issue, and we're not aware of other major websites being affected.