1766805 - scrollBy() is trapped by snap points behind the scrolling direction

Status: VERIFIED FIXED

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

Description

[Hiroyuki Ikezoe (:hiro)]

Attachment: A test case

STR

1. open the attachment
2. click "scrollBy(+5)" button

Actually result;
Nothing happens

Expected result;
Scroll to the green bar labelled as "2"

The reason of this bug is we are using ScrollUnit::DEVICE_PIXELS for scrollBy() operations, and we don't ignore the snap points behind the scroll direction in the case of DEVICE_PIXELS.

I am going to fix this bug with introducing intended direction and intended end position concepts defined in the scroll snap spec.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1766805 - Give the proper destination to GetSnapPointForDestination for ScrollUnit::WHOLE on the main-thread. r?botond

Now the proper destination is same as what we do for ScrollUnit::WHOLE [1].

With the proper points we no longer need special handlings in
CalcSnapPoints::AddEdge for ScrollUnit::WHOLE. In my sense the special handling
wasn't necessary though.

[1] https://searchfox.org/mozilla-central/rev/dc09246dfbfd8dafeb6d55ebee18a6294d525443/gfx/layers/apz/src/AsyncPanZoomController.cpp#2105-2112

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1766805 - Introduce intended direction and intended end position concepts. r?botond

The scroll snap spec defines the concepts [1]. There are three type of scroll
operations. 1) intended end position, 2) intended direction and end position
and 3) intended direction.

Basically our existing ScrollUnits types correspond;

1. DEVICE_PIXELS, WHOLE => intended end position
2. PAGES => intended direction and end position
3. LINES => intended direction

There are two exceptions in the intended direction and end position case,
scrollBy() and fling gestures (on Linux). They were defined as scroll operations
with DEVICE_PIXELS unit, but the spec cleary says they are intended direction and end position operations.

Note that we will also use this information for scroll-snap-stop property since
the properly will only have effects on both 2) and 3) cases.

[1] https://drafts.csswg.org/css-scroll-snap/#scroll-types

Depends on D145190

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1766805 - Ignore snap points behind scroll direction on intended direction scroll operations. r?botond

Depends on D145191

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/898bd12a5da8
Give the proper destination to GetSnapPointForDestination for ScrollUnit::WHOLE on the main-thread. r=botond
https://hg.mozilla.org/integration/autoland/rev/9f9e2030fbc1
Introduce intended direction and intended end position concepts. r=botond
https://hg.mozilla.org/integration/autoland/rev/e35a130f2bce
Ignore snap points behind scroll direction on intended direction scroll operations. r=botond

Comment 5

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/34040 for changes under testing/web-platform/tests

Comment 6

[Norisz Fay [:noriszfay]]

Backed out for causing build bustage on AsyncPanZoomController.cpp

Backout link

Push with failures

Failure log

Comment 7

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR was closed without merging

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a868bbfa9f11
Give the proper destination to GetSnapPointForDestination for ScrollUnit::WHOLE on the main-thread. r=botond
https://hg.mozilla.org/integration/autoland/rev/01b20fe4faf5
Introduce intended direction and intended end position concepts. r=botond
https://hg.mozilla.org/integration/autoland/rev/acc8af72d36c
Ignore snap points behind scroll direction on intended direction scroll operations. r=botond

Comment 9

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/a868bbfa9f11
https://hg.mozilla.org/mozilla-central/rev/01b20fe4faf5
https://hg.mozilla.org/mozilla-central/rev/acc8af72d36c

Comment 10

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 11

[Giorgia Nichita, Release Desktop QA]

Reproduced this issue on an affected Nightly build from 2022-04-20, Win 10 x64.
Verified as fixed on Firefox 102.0b4 (20220605185654) on Win 10 x64 and Ubuntu 21.04, macOS 10.15.