Scroll position jumps out-of-range on IRCCloud

RESOLVED FIXED in Firefox 68

Status

()

defect
RESOLVED FIXED
3 months ago
3 months ago

People

(Reporter: mstange, Assigned: botond)

Tracking

(Regression, {regression})

Trunk
mozilla69
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox66 unaffected, firefox67 unaffected, firefox68+ fixed, firefox69 fixed)

Details

Attachments

(3 attachments)

Posted video screen recording

[Tracking Requested - why for this release]:

Steps to reproduce:

  1. Go to irccloud.mozilla.com and log in with your mozilla LDAP credentials.
  2. In any channel, scroll up slightly so that you're no longer on the bottom.
  3. Scroll back down to the bottom.

Expected results:
As you arrive at the bottom of the scroll range, the bar at the top disappears, but the scroll position should not jump.

Actual results:
When the bar disappears, the scrolled contents move up by the bar height and there's a "gap" at the bottom for one frame. Then the scroll position is adjusted and the gap disappears again.

I narrowed the regression range to https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2f94885dcf94f823f37c99231c79146b6b907e4c&tochange=4440662cd0177f3fc3200cff190daed47d71a167 so I'm pretty sure this is a regression from bug 1549625.

Flags: needinfo?(botond)

A similar phenomenon occurs when marking a channel as read with the Esc key: The channel contents jump upwards and stay there, leaving a semi-permanent gap at the bottom. This state only resets once I scroll or once I open the devtools (which disable APZ for subframes due to the inspector highlighter).

Assignee: nobody → botond
Flags: needinfo?(botond)

I can sort of see how bug 1549625 might cause this:

  • The top bar disppearing causes the scroll port to expand while the content size remains the same. This results in the (layout) scroll range shrinking.
  • We re-clamp the scroll position to the new layout scroll range, and due to bug 1549625 suppress the resulting main-thread scroll offset update, leaving us at the out-of-bounds offset visually.

However, we should still be sending a transaction with the new scroll port (composition bounds), which should cause APZ to re-clamp the scroll offset visually as well. I'd like to understand why this is not happening, before considering a change to the bug 1549625 mechanism.

Unfortunately, despite a multitude of attempts in various configurations, I have not been able to reproduce this on either Linux, Windows, or Android. It's possible it only reproduces on Mac (if so, it would be interesting to understand why that is, too).

(In reply to Markus Stange [:mstange] from comment #1)

A similar phenomenon occurs when marking a channel as read with the Esc key: The channel contents jump upwards and stay there, leaving a semi-permanent gap at the bottom. This state only resets once I scroll or once I open the devtools (which disable APZ for subframes due to the inspector highlighter).

I can repro on Linux with these steps, but unfortunately these are not amenable to repeated repros.

Today I can suddenly repro the issue on Linux! I think the only thing that changed is that more backscroll accumulated in the channel I had open in IRCCloud, so perhaps the scrollable element needs to have a tall enough content size to trigger it.

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

However, we should still be sending a transaction with the new scroll port (composition bounds), which should cause APZ to re-clamp the scroll offset visually as well. I'd like to understand why this is not happening, before considering a change to the bug 1549625 mechanism.

Hah! We are re-clamping the visual scroll offset, but we're not re-clamping the "one frame old" copy of it that's used for the APZ frame delay, so it sticks around for a frame.

I have a candidate fix for this:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=75187158c5ab673e64368a7c672ebdf6d2b58ea1

I'd like to write a test as well.

Depends on D32248

Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2189dcd5c8d4
Re-clamp the composited scroll offset if the scrollable rect changes. r=kats
https://hg.mozilla.org/integration/autoland/rev/95f54abcc586
Add a gtest. r=kats
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69

Please request beta uplift when you get a chance.

Flags: needinfo?(botond)
Flags: in-testsuite+

Comment on attachment 9066911 [details]
Bug 1551582 - Re-clamp the composited scroll offset if the scrollable rect changes. r=kats

Beta/Release Uplift Approval Request

  • User impact if declined: On some page structures (such as IRCCloud), scrolling to the bottom of a scrollable element can sometimes result in an unexpected jump of the scroll position.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The fix is small and enforces an invariant that should always hold, making the potential for regressions low.
  • String changes made/needed:
Flags: needinfo?(botond)
Attachment #9066911 - Flags: approval-mozilla-beta?
Attachment #9066912 - Flags: approval-mozilla-beta?

Comment on attachment 9066911 [details]
Bug 1551582 - Re-clamp the composited scroll offset if the scrollable rect changes. r=kats

scrolling regression fix, approved for 68.0b6

Attachment #9066911 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9066912 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

(In reply to Markus Stange [:mstange] [on PTO until Jun 4] from comment #1)

A similar phenomenon occurs when marking a channel as read with the Esc key: The channel contents jump upwards and stay there, leaving a semi-permanent gap at the bottom. This state only resets once I scroll or once I open the devtools (which disable APZ for subframes due to the inspector highlighter).

This part is still happening. I've filed bug 1557424 about it.

You need to log in before you can comment on or make changes to this bug.