Closed Bug 1687067 Opened 2 years ago Closed 2 years ago

[Fission] Scrollwheel event hand off is broken on page


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




87 Branch
Fission Milestone M6c
Tracking Status
firefox87 --- fixed


(Reporter: annyG, Assigned: hiro)


(Blocks 1 open bug, Regressed 1 open bug)


(Whiteboard: [ETA: waiting for Phabricator review])


(4 files)

Filing this before the weekend begins..

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

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.

Severity: -- → S2

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.

Blocks: apz-fission
Fission Milestone: --- → M6c
Component: Layout: Scrolling and Overflow → Panning and Zooming
Flags: needinfo?(botond)
Summary: [Fission] Scrolling through page is slow. → [Fission] Scrollwheel is broken on

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

Summary: [Fission] Scrollwheel is broken on → [Fission] Scrollwheel event hand off is broken on page

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

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

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

Regression range is may 2019, so not recent either

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

(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.

Flags: needinfo?(botond)

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.

Flags: needinfo?(hikezoe.birchill)

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.

Attached file A reduced test case

Here is a reduce test case.

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

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

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

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

Assignee: nobody → hikezoe.birchill

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.

Flags: needinfo?(hikezoe.birchill)

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
Priority: -- → P1

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.

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).

Whiteboard: [ETA: waiting for Phabricator review]
Pushed by
Build OverscrollHandoffChain by walking up hit testing tree instead of APZC tree. r=botond
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 87 Branch
Regressions: 1690209
Regressions: 1691997
You need to log in before you can comment on or make changes to this bug.