1687067 - [Fission] Scrollwheel event hand off is broken on page https://www.dinacon.org/

Status: RESOLVED FIXED

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

Description

[Anny Gakhokidze [:annyG]]

Filing this before the weekend begins..

Scrolling through website https://www.dinacon.org/ is slow and the website doesn't seem to be as responsive as when I open it in a non-Fission window.

Comment 1

[Jonathan Kew [:jfkthame]]

Trying to scroll it with the touchpad on macOS, it seems to just freeze (stop responding to swipes altogether) after an initial scroll. Still scrolls fine if I drag the scrollbar, though.

Correction: touchpad scroll seems to work if the cursor is over one of the several embedded frames, but not if it's over the main page. Or something like that.

Comment 2

[Neha Kochar [:neha]]

Scrolling with the mouse is not working on this page, dragging the scrollbar is working. This sounds like Bug 1542057 (Fission APZ bug) so looping in Botond.

Comment 3

[Chris Peterson [:cpeterson]]

Nika says the page content is overlaid on a background YouTube video, but the video is stealing the scroll events.

Comment 4

[Botond Ballo [:botond]]

If the overlaying is done using z-index, this might be fixed by bug 1534549 (as per bug 1687072 comment 3).

Comment 5

[Timothy Nikkel (:tnikkel)]

I checked with a patch for bug 1675547 and that doesn't fix this bug.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Just confirmed patches for bug 1534549 didn't help this issue either.

Comment 7

[Timothy Nikkel (:tnikkel)]

Regression range is may 2019, so not recent either
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1e3244e602fc0373508049b6fe544332f800f033&tochange=3c70f36ad62c9c714db3199fc00e60800ee82bde

includes such basics as
Bug 1544811 - Use web processes on a per-site basis for fission-enabled windows
and
Bug 1533673 - Allow APZ to send fission matrices with the GPU process

Comment 8

[Botond Ballo [:botond]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #6)

Just confirmed patches for bug 1534549 didn't help this issue either.

Hmm, I just tried running with the bug 1534549 patches and the issue with the youtube video stealing mouse events seems to be resolved for me.

Comment 9

[Hiroyuki Ikezoe (:hiro)]

I did re-confirm again, it's still happening for me. A place where wheel events are not handoff-ed is top half of the beach image. I will investigate on this.

Comment 10

[Botond Ballo [:botond]]

You're right, I also can't scroll over the top half of the beach image.

However, bug 1534549 does improve the behaviour in that before, I couldn't scroll over most of the page either.

Comment 11

[Hiroyuki Ikezoe (:hiro)]

Attachment: A reduced test case

Here is a reduce test case.

Comment 12

[Hiroyuki Ikezoe (:hiro)]

Attachment: display-list-dump-of-parent

A display list dump of the parent document of attachment 9199171 [details].

Comment 13

[Hiroyuki Ikezoe (:hiro)]

Attachment: A display list dump of the iframe content

A display list dump of the iframe content of attachment 9199171 [details].

Comment 14

[Hiroyuki Ikezoe (:hiro)]

Note that I took both dumps with applying the bug 1534549 fix.

Comment 15

[Neha Kochar [:neha]]

Hiro is already looking into this, so setting the assignee field.

Comment 16

[Botond Ballo [:botond]]

Summarizing our discussion about this:

Based on the display list, the hit test info being provided to the compositor looks fine, the issue seems to be on the APZ side.

Based on additional APZ logs, the issue is that the scroll handoff chain being computed by APZ is incorrect.

There are three APZCs in the compositor layer tree for this page, which I'll give names to make the rest of the explanation easier:

• One for the browser chrome document, which I'll call C.
• One for the root content document's root scroll frame, which I'll call R.
• One for the nested content document's root scroll frame, which I'll call N.

The event initially targets N (which is correct), but the next element in the handoff chain is C, incorrectly bypassing R.

APZ computes links in the handoff chain in one of two ways:

1. If a scrollable element's ScrollMetadata has the scrollParent field set, the next element in the handoff chain is the scrollable element specified by that field. This is computed during display list building and therefore is only applicable to in-process links.
2. Otherwise, the next element in the handoff chain is the tree parent in the APZC tree (which is based on the hit testing tree).

In this case:

1. The link from N to R is a cross-process link, so scrollParent is not set for N.
2. The tree parent of N in the APZC tree is not R because the iframe is inside a position:fixed element, and since containerless scrolling, fixed nodes in the hit-testing tree do not have scroll metadata for the scrollable element with respect to which they are fixed.

The latter is technically a regression from containerless scrolling, but in the in-process case the handoff parent was set correctly based on scrollParent, so we never noticed.

Comment 17

[Botond Ballo [:botond]]

Thoughts on a solution:

We can't determine cross-process links using scrollParent, because that information comes from the content process.

I think we also don't want to solve this by arranging for R to be the APZC tree parent of N. This was the case with container scrolling, but we got rid of container scrolling for a reason and trying to do this would undo some of its advantages.

Instead, I think we want to update the fallback path for computing the next handoff element (in the case where there is no scrollParent) to do something more than just look at the APZC tree parent.

The APZC tree parent is determined by walking parent links in the hit testing tree until an ancestor node with an APZC is found.

I think what we want to do instead is:

• walk parent links in the hit testing tree
• at each step, check if we've encountered a fixed node
  • if so, use the APZC for the scrollable element specified by its mFixedPosTarget

Most of this logic is already implemented in GetTargetApzcForNode() (which is used to determine a correct initial target APZC when the initial target node is a fixed node or a descendant of a fixed node).

More concretely: in APZCTreeManager::BuildOverscrollHandoffChain(), in the case where there is no scrollParent, we can:

• ignore the tree parent that we currently use
• use GetTargetNode() to recover the hit testing node for the APZC
• walk one step to the node's parent
• call GetTargetApzcForNode() to get the correct target APZC for the node's parent

Comment 18

[Hiroyuki Ikezoe (:hiro)]

Thank you Botond for the detailed information. I actually tried the way you and Timothy discussed on the Matrix channel (here), but it didn't fix the original issue. Now I noticed that I missed cases where the parent node doesn't have APZC, we need to keep walking up the tree until we found an ancestor node having APZC.

Comment 19

[Botond Ballo [:botond]]

Ah, good point. There is a walk-the-parents loop in GetTargetApzcForNode(), but it doesn't cross process boundaries (due to the n->GetLayersId() == aNode->GetLayersId() condition).

Comment 20

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1687067 - Build OverscrollHandoffChain by walking up hit testing tree instead of APZC tree. r?botond

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2e296c018c3b
Build OverscrollHandoffChain by walking up hit testing tree instead of APZC tree. r=botond

Comment 22

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/2e296c018c3b