Closed Bug 1554572 Opened 6 months ago Closed 4 months ago

Firefox's scroll anchoring breaks "page-down" keyboard scrolling behavior in BBC articles

Categories

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

defect

Tracking

()

VERIFIED FIXED
mozilla70
Tracking Status
firefox-esr68 --- fixed
firefox69 --- verified
firefox70 --- verified

People

(Reporter: dholbert, Assigned: emilio)

References

(Blocks 1 open bug, )

Details

Attachments

(2 files)

STR:

  1. Load this article:
    http://www.bbc.com/future/story/20190514-the-global-internet-is-disintegrating-what-comes-next
    (Other BBC articles trigger this too, e.g. this one.)

  2. Scroll down several times with "pagedown" key or spacebar.

ACTUAL RESULTS:
Once the first image is visible (photo of a crowd and someone holding a "Putin Net" sign), subsequent pagedown actions only scroll down a few pixels.

EXPECTED RESULTS:
These pagedown keypresses should continue scrolling a full page down.

Firefox Nightly 69.0a1 (2019-05-26) gives ACTUAL RESULTS.
Chrome 74 gives EXPECTED RESULTS. (Though if I use downarrow to scroll, I do notice that the scrolling slows down a bit when the photo comes into view. But pagedown/spacebar work just fine in Chrome, scrolling a full page.)

The issue goes away if I set layout.css.scroll-anchoring.enabled, so this is scroll anchoring intervening in the scroll behavior.

Attached file Reduced testcase.

This took blood and tears to reduce.

It took a while until I figured out why one of the scripts in the page mattered (because it attached a global keyup event listener).

What is interesting is that if you remove the event listener, it works even with apz.keyboard.enabled=false. That's a bit wild.

Flags: needinfo?(emilio)
Assignee: nobody → emilio
Flags: needinfo?(emilio)
Blocks: 1552399

Looks like this bug's fix is in https://phabricator.services.mozilla.com/D38077 on the wrong bug right now. ni=emilio to remind him to fix that once he's back at his laptop. :)

Depends on: 1546027
Flags: needinfo?(emilio)
Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1af0dca460dd
Don't apply scroll anchor adjustments if we're processing an async scroll animation. r=dholbert
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70

Comment on attachment 9078215 [details]
Bug 1554572 - Don't apply scroll anchor adjustments if we're processing an async scroll animation. r=rhunt,dholbert

Beta/Release Uplift Approval Request

  • User impact if declined: Pageup / Pagedown navigation is broken in pages with keyup listeners and such.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: Comment 0 or the reduced test-case
  • List of other uplifts needed: Bug 1546027
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Same justification as for bug 1546027. Behavior when the new condition is taken is disabling effective scroll anchoring, which is what we had until FF66.
  • String changes made/needed: none

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: See above.
  • User impact if declined: See above.
  • Fix Landed on Version: 70
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): See above.
  • String or UUID changes made by this patch: none
Attachment #9078215 - Flags: approval-mozilla-esr68?
Attachment #9078215 - Flags: approval-mozilla-beta?
Flags: qe-verify+

Comment on attachment 9078215 [details]
Bug 1554572 - Don't apply scroll anchor adjustments if we're processing an async scroll animation. r=rhunt,dholbert

Low-risk scroll anchoring fix. Approved for 69.0b6 and 68.1esr.

Attachment #9078215 - Flags: approval-mozilla-esr68?
Attachment #9078215 - Flags: approval-mozilla-esr68+
Attachment #9078215 - Flags: approval-mozilla-beta?
Attachment #9078215 - Flags: approval-mozilla-beta+
Duplicate of this bug: 1542549
QA Whiteboard: [qa-triaged]

I verified this using the reduced test case on Mac OS X 10.14 and Windows 10 x64 with FF Nightly 70.0a1(2019-07-17) and I can confirm the fix.
I will put a need info for myself to verify this after the beta version is available for testing.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+ → needinfo?(ovidiu.boca)

Beta version 69.0b6 is working as expected, I can confirm the fix.
I will leave the NI? for ESR verification.

Flags: needinfo?(ovidiu.boca)
Duplicate of this bug: 1562963
Duplicate of this bug: 1529530
You need to log in before you can comment on or make changes to this bug.