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

Status: VERIFIED FIXED

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

Description

[Daniel Holbert [:dholbert]]

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.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: 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.

Comment 2

[Daniel Holbert [:dholbert]]

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. :)

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1554572 - Don't apply scroll anchor adjustments if we're processing an async scroll animation. r=rhunt,dholbert

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1af0dca460dd
Don't apply scroll anchor adjustments if we're processing an async scroll animation. r=dholbert

Comment 5

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/1af0dca460dd

Comment 6

[Emilio Cobos Álvarez (:emilio)]

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

Comment 7

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 8

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/releases/mozilla-beta/rev/53ec0f2a6b50

Comment 10

[ovidiu boca[:Ovidiu]]

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.

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr68/rev/bc6fa9b448fc

Comment 12

[ovidiu boca[:Ovidiu]]

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