Closed Bug 1519541 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: Layout, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla66
Root Cause Corner Case
Tracking Status
firefox-esr60 --- unaffected
firefox64 --- unaffected
firefox65 --- unaffected
firefox66 blocking verified

People

(Reporter: cpeterson, Assigned: rhunt)

References

(Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

[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

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

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

Flags: needinfo?(rhunt)
Priority: -- → P3

(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.

Flags: needinfo?(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

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
Assignee: nobody → rhunt

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)

Priority: P3 → P1
Depends on: 1520306
No longer depends on: 1520306

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.]

Severity: normal → critical
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.
Attachment #9036183 - Attachment is obsolete: true
Pushed by rhunt@eqrion.net:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c20f8b989a2f
Clamp negative portions of relative scroll anchoring bounding rect. r=dholbert
See Also: → 1520124
See Also: → 1520095
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

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

Blocks: 1520095
See Also: → 1521812

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.

Status: RESOLVED → VERIFIED

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

Root Cause: --- → ?
Root Cause: ? → Corner Case
You need to log in before you can comment on or make changes to this bug.