2038480 - Avoid calling `SetDelayedTransformEnd(false)` in ScrollSnapToDestination if the snap destination will not be changed

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect, P2)

Description

[Hiroyuki Ikezoe (:hiro)]

The code is:

https://searchfox.org/firefox-main/rev/574d441bd2fdc78f8487aa317ea2f56aad28621f/gfx/layers/apz/src/AsyncPanZoomController.cpp#7085-7088

// Ensure that any queued transform-end due to a pan-end is not
// sent. Instead rely on the transform-end sent due to the
// scroll snap animation.
SetDelayedTransformEnd(false);

Due to the unconditional calling ofSetDelayedTransformEnd(false) `, scrollend event is not fired.

This is another cause of the test failure caused by D294084.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 2038480 - Use CSSSnapDestination::mPosition to output the snap destination in logs. r?botond

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 2038480 - Do not mutate aStartPosition in MaybeAdjustDeltaForScrollSnapping. r?botond

The modified aStartPosition is never used.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 2038480 - Preserve the queued transform-end when a pan-end snap doesn't move the scroll position. r?botond

When OnPanEnd queues a delayed transform-end notification and then calls
ScrollSnapToDestination, taking the snap branch in
MaybeAdjustDeltaForScrollSnapping unconditionally clears the queued
transform-end via SetDelayedTransformEnd(false), on the assumption that
the subsequent SmoothScrollTo will deliver its own. If the snap
destination is identical to the start position, SmoothScrollTo has
nothing to animate and no transform-end is sent, so the scrollend event
is lost.

Only call SetDelayedTransformEnd(false) when the snap destination
actually differs from the start position. We still enter the snap branch
and call SmoothScrollTo so that the last snap target IDs are updated.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/7eebc8bfecfa
https://hg.mozilla.org/integration/autoland/rev/38d462a75ed5
Use CSSSnapDestination::mPosition to output the snap destination in logs. r=botond
https://github.com/mozilla-firefox/firefox/commit/1198bff6df78
https://hg.mozilla.org/integration/autoland/rev/600f6178d6b1
Do not mutate aStartPosition in MaybeAdjustDeltaForScrollSnapping. r=botond
https://github.com/mozilla-firefox/firefox/commit/24d4cc512b1e
https://hg.mozilla.org/integration/autoland/rev/3a473ace49be
Preserve the queued transform-end when a pan-end snap doesn't move the scroll position. r=botond

Comment 5

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/38d462a75ed5
https://hg.mozilla.org/mozilla-central/rev/600f6178d6b1
https://hg.mozilla.org/mozilla-central/rev/3a473ace49be