1827975 - Scrollend fires on keyboard scrolls that don't change scroll position

Status: VERIFIED FIXED

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

Description

[David Awogbemila]

Steps to reproduce:

Please view this codepen: https://codepen.io/awogbemila/pen/zYmrRVV
The div should be scrolled all the way up (scrollTop is 0).
Press and release the up arrow key
Observe that scrollend is fired even though no scrolling happened (scrollend count is incremented).

You can also scroll the div all the way down and then press and release arrow down.
You'll see that scrollend is fired even though no scrolling happened.

Actual results:

Scrollend events fire to the div even though the div does not actually scroll (scrollTop does not change).

Expected results:

Scrollend events should not fire since there was no scrolling as this working group spec notes[1].
Also note that firefox behaves differently with mouse wheels: when no scrolling happens, it does not fire scrollend which I believe is the correct behavior.

[1] https://drafts.csswg.org/cssom-view/#scrolling

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Layout: Scrolling and Overflow' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Dan Robertson (:dlrobertson)]

Good catch! On mac I don't see this, but I don't see scrollend events fired for keyboard scrolls that should fire scrollend events. What OS did you see this on?

Comment 3

[Hiroyuki Ikezoe (:hiro)]

I am able to see the events on Linux. The events come from here in nsHTMLScrollFrame::SetTransformingByAPZ.

Comment 4

[David Awogbemila]

Hmm, originally I saw this running firefox on Linux but I've tried it on mac as well and I see it there too.

On a somewhat related note: this issue will affect a wpt: https://wpt.fyi/results/dom/events/scrolling/scrollend-event-not-fired-on-no-scroll.html?label=master&label=experimental&aligned&q=scrollend which tests this use case though it is currently masked by another issue which I filed
https://bugzilla.mozilla.org/show_bug.cgi?id=1827912 for.

Comment 5

[Botond Ballo [:botond]]

I can reproduce this on Linux as well.

(In reply to Dan Robertson (:dlrobertson) from comment #2)

On mac I [...] don't see scrollend events fired for keyboard scrolls that should fire scrollend events.

That might be worth filing an additional bug about for tracking purposes.

Comment 6

[Šime Vidas]

I’m testing in Firefox Nightly on macOS.

When I click the scroll container and then press and hold down the Space bar, the scrollend counter keeps increasing indefinitely, even after the end of the scroll container was reached.

In Chrome, the scrollend counter increases by 2. I guess the logic is, one for pressing Space bar, and another one for holding it down.

Comment 7

[Dan Robertson (:dlrobertson)]

Attachment: Bug 1827975 - Do not fire scrollend events for keyboard scrolls that do not change the scroll position. r=hiro,botond

When a APZC handles a keyboard scroll or other smooth scroll,
no scrollend event should be fired if the scroll position will not
change as a result of the scroll.

Comment 8

[Dan Robertson (:dlrobertson)]

Attachment: Bug 1827975 - Add scrollend tests for keyboard scrolls. r=hiro,botond

Add scrollend event tests for keyboard scrolls.

Depends on D187050

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8d034bde14e5
Do not fire scrollend events for keyboard scrolls that do not change the scroll position. r=hiro
https://hg.mozilla.org/integration/autoland/rev/87f29c301be9
Add scrollend tests for keyboard scrolls. r=hiro

Comment 10

[Serban Stanca [:SerbanS]]

Backed out for causing mochitests failures in test_group_scrollend.html.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | gfx/layers/apz/test/mochitest/test_group_scrollend.html | Error: Native key events not supported on platform linux

Comment 11

[Dan Robertson (:dlrobertson)]

Updating the test now.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/93a891338556
Do not fire scrollend events for keyboard scrolls that do not change the scroll position. r=hiro
https://hg.mozilla.org/integration/autoland/rev/d9d3c8186de0
Add scrollend tests for keyboard scrolls. r=hiro

Comment 13

[Natalia Csoregi [:nataliaCs]]

Backed out with Bug 1831580 for causing scrollend failures.

• backout: https://hg.mozilla.org/integration/autoland/rev/7365616b9b4dbef532aa1410981d76c3b932bb63
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=f3de6f924333ec7c359d81d9fb16a9fca07ee42e&selectedTaskRun=GezXxOfjQ9CT5-DT3j5Tjw.0
• failure logs:
  • TEST-UNEXPECTED-PASS | /dom/events/scrolling/scrollend-event-not-fired-on-no-scroll.html | No scroll via keys on div shouldn't fire scrollend. - expected FAIL
  • TEST-UNEXPECTED-FAIL | gfx/layers/apz/test/mochitest/test_group_scrollend.html | Test timed out. -

Comment 14

[Dan Robertson (:dlrobertson)]

Fixed and running some more tests on try. I should have caught the scrollend tests in wpt that we now pass.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15086b08cea5
Do not fire scrollend events for keyboard scrolls that do not change the scroll position. r=hiro
https://hg.mozilla.org/integration/autoland/rev/7ce85e940886
Add scrollend tests for keyboard scrolls. r=hiro

Comment 16

[Sandor Molnar[:smolnar]]

Backed out for causing mochitest failures at gfx/layers/apz/test/mochitest/test_group_scrollend.html

Backout link: https://hg.mozilla.org/integration/autoland/rev/183b1ebcb4ab0497430f16896bf7b69b2fbb5684

Push with failures

Failure log -> TEST-UNEXPECTED-FAIL | gfx/layers/apz/test/mochitest/test_group_scrollend.html | helper_keyboard_scrollend.html?direction=up | A user gesture with no scroll should have no scrollend

Comment 17

[Dan Robertson (:dlrobertson)]

The failing test will be fixed by scroll position rounding, but perhaps we should put in place a workaround of some kind.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dab9c40ca808
Do not fire scrollend events for keyboard scrolls that do not change the scroll position. r=hiro
https://hg.mozilla.org/integration/autoland/rev/c02a1959f381
Add scrollend tests for keyboard scrolls. r=hiro

Comment 19

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/dab9c40ca808
https://hg.mozilla.org/mozilla-central/rev/c02a1959f381

Comment 20

[Alexandru Trif, Desktop Test Engineering [:atrif]]

Reproduced the issue with Firefox 114.0a1 (2023-04-13) on Windows 10x64. Pressing the UP arrow keyboard key on the test codepen from comment 0 will increase the scrolled count.
The issue is verified fixed with Firefox 121.0b3 on Windows 10x64, macOS 12 and Ubuntu 22. The scrolled count is no longer increased after following STR from comment 0.