1682203 - When tabbing up/down, window.scrollY registers twice, one pixel apart

Status: NEW

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

Description

[rjm449]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:83.0) Gecko/20100101 Firefox/83.0

Steps to reproduce:

When tabbing up/down (and it activates a scroll), the scroll is instantaneous (not smooth) and doesn't land cleanly on one value.

Actual results:

It lands once, registers the window.scrollY value, and then moves a single pixel more before "landing" again. This messes up certain scroll scripts.

Expected results:

I tested this in Chrome after disabling its smooth scrolling and it handled the tab scroll's window.scrollY well. It doesn't stutter on the landing. It registers one value and then it's good.

So it seems to be an issue with Firefox.

Comment 1

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

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Botond Ballo [:botond]]

Could you attach an example page demonstrating the issue please?

Comment 3

[rjm449]

If you put this in the Console of any site, you can compare the different scrolling behaviors:

document.addEventListener("scroll", () => {
console.log("Scrolled");
console.log(window.scrollY);
})

1. Scroll up/down by mouse (Works fine)
   - Scroll up, the last print of window.scrollY will be smaller than the previous one because it's closer to the top of the viewport

   - Scroll down, the last print of window.scrollY will be larger than the previous one because it's farther from the top of the viewport
   

2. Scroll up/down by arrow keys (Works fine, the same way as above)

3. When tabbing, and it activates a scroll, this is where it works incorrectly

     - Scrolling down, the last print of window.scrollY will be 1 pixel less than the previous one  (*But NOT always. Sometimes it's okay.)
   
     - Scrolling up, the last print of window.scrollY will be 1 pixel more than the previous one
   
     E.g.   Tab DOWN --> Activates scroll (non-smooth)
                   3964.2666015625 
                   Scrolled 
                   4709.2666015625
                   Scrolled 
                   4708.2666015625           (It "lands" here)
   

I tested this same thing in Edge and Chrome and they print window.scrollY correctly and scripts work good there.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

I can see the issue unless I am misunderstanding what the tabbing means (I suppose it's tab key focus changes).

Anyway this probably also happens with scrollIntoView call, and I guess this is caused by layer pixel alignment

Comment 5

[Botond Ballo [:botond]]

Possibly related: bug 1230176.

Comment 6

[Razvan Cojocaru]

So basically just press TAB and keep it pressed until the page starts scrolling?
Could you please point to the code that's double-triggered? Thanks!

Comment 7

[Botond Ballo [:botond]]

(In reply to Razvan Cojocaru from comment #6)

So basically just press TAB and keep it pressed until the page starts scrolling?

I think it's easier to see the issue if you perform individual presses of <Tab> until scrolling is triggered, and then see what is printed on the console.

Usually you will see a single message printed, but sometimes you will see something like:

Scrolled debugger eval code:2:9
380 debugger eval code:3:9
Scrolled debugger eval code:2:9
379.20001220703125

which is the bug in question.

Could you please point to the code that's double-triggered? Thanks!

If you mean the place in C++ where we fire the "scroll" event that we're listening for in JS, that's here.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

This bug is indeed involving layer pixel alignment, specifically this ClampAndAlignWithLayerPixels call in ScrollFrameHelper::ScrollToImpl.

scrollIntoView calls end up calling this ScrollTo function which invokes ScrollFrameHelper::ScrollToImpl.

In the same scrollIntoView call we call PresShell()->ScrollToVisual which is at a few lines below of the scrollTo call. In the case of the ScrollToVisual we don't use ClampAndAlignWithLayerPixels at all, thus the non-alignment position will reach to the main-thread scroll position via an APZ's repaint request.

So, in short, the first scroll event is caused by alignment scroll position, the second one is caused by unalignment scroll position.