1780701 - Double tap triggers scroll snap on zoomed document

Status: VERIFIED FIXED

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

Description

[Hiroyuki Ikezoe (:hiro)]

Attachment: A case to be able to see the issue

STR;

1. Open the attachment
2. Zoom in the content
3. Scroll down a bit to see the green box
4. Double tap

Double tap results scrolling to (0, 0) position on my Mac.

The snap is triggered by this nsIScrollableFrame::ScrollSnap() call in nsIFrame::HandleRelease, the comment for the call explains it's for autoscrolling. I don't quite understand why it gets called on double tap.

CCing Timothy, he might know something why nsIFrame::HandleRelease gets called or he might have seen similar things.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Phew, it turned out this problem has been there before re-snapping feature.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

I am suspecting the root cause of this bug is bug 1665932.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Šime, I just saw your comment in bug 1779909 comment 20, if the issue happens only with scroll-behavior: smooth, it's pretty likely to be the same root cause of bug 1665932.

And I just figured out a way to fix it specific to scroll-snap. Here is a build with the fix. Mind trying to see whether the issue you are seeing is fixed or not? Thanks

https://treeherder.mozilla.org/jobs?repo=try&revision=02d917f5c32c8cfa556397553f4f0d2aa81457e0&selectedTaskRun=R82HjquRSLKZRztomrvoaQ.0

Comment 4

[Šime Vidas]

Unfortunately, the issue still occurs. I installed target.dmg from your link and performed the steps from https://bugzilla.mozilla.org/show_bug.cgi?id=1779909#c20.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1780701 - Collect mSnapTargets even if ComputeScrollSnapInfo gets called for ScrollMetadata. r?botond

mSnapTargets stores the ids for all candidate snap taget elements in this scroll
container. Before this change, we collect the ids when we snap or re-snap on the
main-thread. It's used to check whether the last snap target ids comming from
APZ are not stale or not. So if we haven't done any snapping or re-snapping on
the main-thread, we mis-consider all incoming last snap target ids are stale,
thus we fail to re-snap to the position where we snapped on APZ. This issue
had been wall-papered, will be revealed by the next change in this commit series
(we had been unnecessary snappings and re-snappings).

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1780701 - Skip returning the snap target if the snap position is same as the scroll destination. r?botond

Depends on D152617

Comment 7

[Hiroyuki Ikezoe (:hiro)]

(In reply to Šime Vidas from comment #4)

Unfortunately, the issue still occurs. I installed target.dmg from your link and performed the steps from https://bugzilla.mozilla.org/show_bug.cgi?id=1779909#c20.

Thanks for testing. That's unfortunate.

I found another issue while I was trying to make a test for this bug pass locally, I guess it's unlikely related to the issue you are seeing, though.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/00746ffaaf58
Collect mSnapTargets even if ComputeScrollSnapInfo gets called for ScrollMetadata. r=botond
https://hg.mozilla.org/integration/autoland/rev/1d599dd89c5f
Skip returning the snap target if the snap position is same as the scroll destination. r=botond

Comment 9

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/00746ffaaf58
https://hg.mozilla.org/mozilla-central/rev/1d599dd89c5f

Comment 10

[Bogdan Maris, Desktop QA]

Reproduced the initial issue using old Nightly from (2022-07-21) and the steps from comment 0. Verified that using latest Firefox 105.0 RC on macOS 11 this is no longer reproducible.