1773409 - Scrollbar dragging of subframe inside fixed content does not work when pinch-zoomed in

Status: NEW

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

Description

[Botond Ballo [:botond]]

Attachment: Testcase

STR

1. Run Firefox on a desktop machine with a touchpad or touchscreen (or, barring that, with the pref mousewheel.with_alt.action=5 set to allow simulating pinch-zooming via alt+mousewheel).
   • While not required to reproduce, on Linux I find it helpful to set widget.gtk.overlay-scrollbars.enabled=false as well.
2. Pinch-zoom in. The bug is more readily noticeable at larger zooms (e.g. 2x rather than 1.2x).
3. Scroll the root scroll frame down a bit. Since the page is pinch-zoomed in, this (initially) scrolls the visual viewport inside the layout viewport, so the fixed (blue) element should start to move offscreen, and the subframe inside it should move closer to the top of the viewport. Scroll until the top of the subframe is close to the top of the viewport.
4. Try to drag the (vertical) scrollbar of the subframe.

Expected results

Scrollbar dragging works.

Actual results

Dragging the scrollbar initially has no effect. Eventually, after you've moved the mouse a ways down, it starts moving and tracking the mouse (but its position remains offset by the initial distance for which it was non-responsive).

Diagnosis

APZ scrollbar dragging is one of the few remaining places where the numerical value of input events' APZC coordinates (i.e. the coordinates after applying GetScreenToApzcTransform but before applying GetApzcToGeckoTransform) is important for correctness.

In this situation (APZC for a subframe inside fixed content, and there is an offset between the visual and layout viewports), the APZC coordinates are incorrect. The target APZC is the subframe (as expected), but this APZC's parent chain does not include the root content APZC, whose async transform contains the offset between the visual and layout viewports. Rather, the target APZC's GetParent() is the enclosing (chrome document) APZC.

As a result, the target APZC's GetScreenToApzcTransform() does not include this offset. But visually, translation by this offset does apply to the fixed content and its descendants, including the subframe.

The offset is similarly missing from GetApzcToGeckoTransform(), which makes the Gecko coordinates work out to be correct (and thus e.g. buttons on the fixed content can still be clicked correctly).

Comment 1

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #0)

The target APZC is the subframe (as expected), but this APZC's parent chain does not include the root content APZC, whose async transform contains the offset between the visual and layout viewports. Rather, the target APZC's GetParent() is the enclosing (chrome document) APZC.

This reflects the fact that the subframe is inside fixed content which is not subject to the root content APZC's scroll transform.

However, this code has not been updated to properly reflect the changes made when we made fixed content be fixed to the layout viewport, thereby making it subject to the part of the root content APZC's async transform which reflects the offset of the visual viewport inside the layout viewport.

Note, when an event targets a part of fixed content that is not inside a subframe, the APZC coordinates are correct, because the target APZC is now the root content APZC. I believe this happens because the initial target APZC is determined by the scroll id returned by the WebRender hit test, and this is how we assign scroll ids to hit-test-info items during display list building here: if we're inside any sort of scrolled content, we assign the scroll frame's scroll id, otherwise if we're inside fixed content, we assign the scroll id of the scroll frame we are fixed to. An APZC's parent, however, is determined by the structure of the WebRenderScrollData, and there, nodes representing fixed content are deliberately outside of the node with the root content APZC's metadata.

It's not immediately clear to me how best to fix this. I suspect it's likely to require some refactoring.