1519541 - Gmail flashes and jumps when scrolling an email thread with an open Reply when scroll-anchoring is enabled

Status: VERIFIED FIXED

(Core :: Layout, defect, P1)

Description

[Chris Peterson [:cpeterson]]

[Tracking Requested - why for this release]:

This is a regression in 66 Nightly build 2019-01-11 from scroll-anchoring bug 1305957.

STR:

1. Load Gmail.
2. Open a long email thread.
3. Click "Reply".
4. Scroll up and down. (I'm using my Windows 10 laptop's trackpad scroll gesture, if that matters.)

RESULT:
The page will flash and the page's scroll position will jump up and down.

I bisected this regression to this pushlog:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=fbe6548db11ded24b5221180719ce66e785dc3c6&tochange=ad851d4345c08f7e0e5d5578652004194a6e667f

Comment 1

[Chris Peterson [:cpeterson]]

The bug is sometimes easier to reproduce if you click outside of the Reply textarea to remove the input focus.

Comment 2

[Brian Pitts]

I've seen this on Gmail today as well and also experienced scrolling seeming to get "stuck" and constantly flicker without moving.

Comment 3

[Ryan Hunt [:rhunt]]

(In reply to Brian Pitts from comment #2)

I've seen this on Gmail today as well and also experienced scrolling seeming
to get "stuck" and constantly flicker without moving.

If you find reproducible cases of being “stuck” with a flicker, please file them! We should have heuristics in place to avoid that, but they’re not perfect.

Comment 4

[Ryan Hunt [:rhunt]]

I've half figured this out, and a possibility to fix it.

1. We select a table element as an anchor that has a massive and mostly negative scrollable overflow rect
2. The user scrolls down to move the 'reply' widget into view
3. A scroll event changes 'something' and causes the table's scrollable overflow rect to shift negative (i.e up)
4. We perform an adjustment, causing the page to scroll up and dispatch a scroll event
5. The following scroll event changes 'something' and causes the table's scrollable overflow rect to shift positive (i.e down)
6. We perform an adjustment, go to step 3

The interesting thing about the scrollable overflow rect, is that it's so big (like 10000px) and much of it is negative. Inspecting with devtools, I wasn't able to find an evidence of this in any bounding rects. But it's possible I missed something, the Gmail HTML and CSS is a bit gnarly.

I happened to remember reading something odd in Blink's implementation of scroll anchoring, and double checked how they calculate the scrollable overflow rect [1]. For 'boxes' (not text), they will use the border box, and then extend it downward to contain the furthest amount of vertical overflow.

Changing our implementation to do something similar resolved this issue locally.

I don't fully understand what's going on here, as I don't know how we calculate our scrollable overflow rect. It's possible we're doing it wrong, like in bug 1517287. It's possible that the scroll anchoring code isn't doing the calculation correctly. It's also possible that the spec doesn't match Blinks behavior here [2].

[1] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/layout/scroll_anchor.cc?l=99&rcl=5a256e18e9d0d0b942a6cb2853f8ebb1edd1b9c7
[2] https://www.w3.org/TR/css-overflow-3/#scrollable

Comment 5

[Ryan Hunt [:rhunt]]

Attachment: Bug 1519541 - Clamp negative portions of relative scrollable overflow rect. r?dholbert

Blink calculates the scrollable overflow rect for 'boxes' for scroll anchoring
by taking the border box and then extending the height for the furthest
vertical overflow.

This commit matches tries to match their behavior by removing negative portions
of the relative scrollable overflow rect that we get from nsIFrame.

Depends on D16404

Comment 6

[Liz Henry (:lizzard) (relman/hg->git project)]

Came to file this very same issue. Marking this as a release blocker. I don't think it has to block 66 going to beta, but it would be nice to fix before beta 3 (the first beta build that goes to the beta channel users)

Comment 9

[Daniel Holbert [:dholbert]]

rhunt is mitigating this by temporarily disabling scroll anchoring (by default) in Nightly, in bug 1520304.

So (assuming landings/merges go smoothly over on that bug), users should stop being affected by this bug tomorrow. (Unless they explicitly turn scroll anchoring back on.)

[EDIT: Actually, maybe we'll get this fixed in time for tomorrow's nightly, rather than disabling scroll snapping.]

Comment 10

[Ryan Hunt [:rhunt]]

Attachment: Bug 1519541 - Clamp negative portions of relative scroll anchoring bounding rect. r?dholbert

The scroll anchoring bounding rect of a node can be influenced by absolutely
positioned descendants with very negative offsets. This can cause undesired
scroll adjustments, and has been seen on the web in Gmail. The spec needs to
be amended to say what to do here. Chrome currently will clamp the vertical
offset. This commit implements a stop-gap to clamp the negative portions to
fix this issue, while we do more research and spec-work.

Comment 11

[Ryan Hunt [:rhunt]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=039b7735575e24da579d0ddf8bb4e48f607fc531

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c20f8b989a2f
Clamp negative portions of relative scroll anchoring bounding rect. r=dholbert

Comment 13

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/c20f8b989a2f

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

I'm no longer hitting the issue using today's nightly.

Comment 16

[Cristian Fogel [:cfogel]]

Reproduced issue with Firefox 66.0a1(2019-01-11).
Fix verified with 66.0b3 on Windows 10, macOS 10.11.6, Ubuntu 18.04 as well as per comment 14 marking the bug as verified.

Comment 17

[Firefox Bug Husbandry Bot]

Please specify a root cause for this bug. See :tmaity for more information.