1747409 - Fixed position iframe breaks fixed position anchor tag

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect)

Description

[dstout]

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

Steps to reproduce:

See this codepen for a reproduction: https://codepen.io/dstoutimmediac/pen/XWeePRL

I have an iframe and an anchor tag, both with position fixed. The anchor tag is made to be above the iframe by having a higher z-index.

Actual results:

Even though the anchor tag is visible, it is not clickable. This does not happen in Chrome, and does not happen if a div is used instead of an iframe

Expected results:

The anchor tag should be clickable if it appears above the iframe

Comment 1

[dstout]

Also, this issue happens if no z-index is specified and the "over" element is just after the iframe in the DOM. The iframe still causes the anchor tag to become non-functional.

Comment 2

[Alice0775 White]

I can reproduce the issue in Nightly97.0a1 Windows10.
However, if Fission is disabled, the issue is gone.

Comment 3

[Andrew McCreight [:mccr8]]

This sounds like another Fission hit testing issue. Henri, do you have any idea where this bug should go? Thanks.

Comment 4

[Chris Peterson [:cpeterson]]

kmag thinks this bug is probably related to lowes.com bug 1676466, which we didn't fix before the problem went away after the website changed.

Comment 5

[Henri Sivonen (:hsivonen) (not reading bugmail while on leave)]

Seems potentially related to bug 1745834. Emilio, does this look related to you?

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Well, somewhat related, but not quite the same. That bug is about a matrix being miscomputed, this one is about events going to the wrong frame.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Standalone test-case

Comment 8

[Emilio Cobos Álvarez (:emilio)]

If I translate the fixed-pos I get it to behave correctly:

.over { transform: translate(1px, 1px) }

So probably it's (wrongly) not included in the hit-testing tree.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Botond, Hiro, do you know what's different between fixed and transformed items? I don't know if I should look at WrHitTester or HitTestingTreeNode or...

Comment 10

[Hiroyuki Ikezoe (:hiro)]

Maybe this is due to incorrect display items? Bouncing to Miko.

Comment 11

[Botond Ballo [:botond]]

Attachment: Gecko+WR display list from root content process

Attached is a combined Gecko+WR display list from the root content process.

The Iframe item is inside a FixedPosition item (p=0x7f4ef585e418), which does come before the FixedPosition item (p=0x7f4ef585d5d8) containing the anchor. So, the display list looks correct to me (the display list being ordered from bottom to top).

Nonetheless, when we perform a compositor hit test here, the results returned by WebRender, which are supposed to be arranged in order from top to bottom, contain items from the iframe before items from the root content document.

This suggests to me that the place to look further is inside WebRender's hit-testing code (and/or code that turns the display list into the data structures used for the hit-test, such as scene building).

Comment 12

[Botond Ballo [:botond]]

(In reply to Henri Sivonen (:hsivonen) from comment #5)

Seems potentially related to bug 1745834. Emilio, does this look related to you?

I doubt it, I believe that bug is specific to perspective, and this test case does not involve perspective.

Comment 13

[Timothy Nikkel (:tnikkel)]

Regression range with fission prefed on
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=cc4889143d653957d83a6e2ca9ea89691d39b435&tochange=f72030ee528939167d88804b61faa200246e037c
->bug 1534549

Comment 14

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

Set release status flags based on info from the regressing bug 1534549

Comment 16

[Emilio Cobos Álvarez (:emilio)]

If I remove this early-return then it works: https://searchfox.org/mozilla-central/rev/02bd3e02b72d431f2c600a7328697a521f87d9b6/gfx/layers/wr/HitTestInfoManager.cpp#89

Comment 17

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1747409 - Minimal cleanup to WR bindings hit-tester usage. r=#gfx-reviewers

This doesn't change behavior but wrote it drive-by.

Comment 18

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1747409 - Deindent a bit the HitTester code by early-continue-ing. r=#gfx-reviewers

Shouldn't change behavior but wrote this drive-by.

Depends on D136893

Comment 19

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1747409 - Use iter().all() in clip nodes during hit testing rather than a nested loop. r=#gfx-reviewers

Shouldn't change behavior but wrote this drive-by.

Depends on D136894

Comment 20

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1747409 - Don't optimize away hit-test info items across remote frame boundaries. r=miko,#gfx-reviewers

This is the actual fix. Any tests to crib from would be greatly appreciated.

Depends on D136895

Comment 21

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1747409 - Add a test. r=miko,botond

Confirmed that it fails without the patch with:

Expected at least one hit result in the APZTestData
    SimpleTest.ok@SimpleTest/SimpleTest.js:417:16
    runSubtestsSeriallyInFreshWindows/</advanceSubtestExecution/spawnTest/w.ok@gfx/layers/apz/test/mochitest/apz_test_utils.js:479:32
    hitTest@gfx/layers/apz/test/mochitest/apz_test_utils.js:833:5
    test@gfx/layers/apz/test/mochitest/helper_hittest_fixed_item_over_oop_iframe.html:48:18

Depends on D136896

Comment 22

[Emilio Cobos Álvarez (:emilio)]

https://treeherder.mozilla.org/jobs?repo=try&revision=a07e22514aff6da954b3d8c276e4a92d2ac85a3e

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/da757ff89f75
Minimal cleanup to WR bindings hit-tester usage. r=gfx-reviewers,kvark
https://hg.mozilla.org/integration/autoland/rev/0ec8430f12cf
Deindent a bit the HitTester code by early-continue-ing. r=gfx-reviewers,bradwerth
https://hg.mozilla.org/integration/autoland/rev/e6df83a893d1
Use iter().all() in clip nodes during hit testing rather than a nested loop. r=gfx-reviewers,bradwerth,kvark
https://hg.mozilla.org/integration/autoland/rev/650ff3e9d748
Don't optimize away hit-test info items across remote frame boundaries. r=miko,gfx-reviewers
https://hg.mozilla.org/integration/autoland/rev/f54686ba4658
Add a test. r=botond

Comment 24

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/da757ff89f75
https://hg.mozilla.org/mozilla-central/rev/0ec8430f12cf
https://hg.mozilla.org/mozilla-central/rev/e6df83a893d1
https://hg.mozilla.org/mozilla-central/rev/650ff3e9d748
https://hg.mozilla.org/mozilla-central/rev/f54686ba4658

Comment 27

[Ryan VanderMeulen [:RyanVM]]

Looks like rev 650ff3e9d748 (D136896) grafts cleanly to Beta. Given the number of duplicate reports this is getting, I'm thinking we probably do want to get this fixed on 97 if possible. Do you agree, Emilio?

Comment 28

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9260636 [details]
Bug 1747409 - Don't optimize away hit-test info items across remote frame boundaries. r=miko,#gfx-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Correctness issues with Fission iframes.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: see various test-cases and dupes.
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Relatively simple patch that disables an optimization that is unsound across cross-origin iframes.
• String changes made/needed: none

Comment 29

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9260636 [details]
Bug 1747409 - Don't optimize away hit-test info items across remote frame boundaries. r=miko,#gfx-reviewers

Fixes various site regressions from the Fission rollout. Approved for 97.0b9.

Comment 30

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8c45eea9b81d
https://hg.mozilla.org/releases/mozilla-beta/rev/bd2fca6a31b5

Comment 31

[Peter Magyari (Desktop QA)]

I have reproduced the issue using Firefox 96.0.2 and verified the fix using Nightly 98.0a1 (20220127094620) and Beta 97.0b9 (20220126222842) on MacOS 11.6, Windows 10 and Ubuntu 20.04.