Closed Bug 1747409 Opened 2 years ago Closed 2 years ago

Fixed position iframe breaks fixed position anchor tag

Categories

(Core :: Graphics: WebRender, defect)

Firefox 95
defect

Tracking

()

VERIFIED FIXED
98 Branch
Fission Milestone Future
Tracking Status
firefox-esr91 --- disabled
firefox95 --- wontfix
firefox96 --- wontfix
firefox97 --- verified
firefox98 --- verified

People

(Reporter: dstout, Assigned: emilio)

References

(Blocks 1 open bug, Regressed 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(7 files)

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

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.

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

Blocks: fission
Status: UNCONFIRMED → NEW
Has STR: --- → yes
Component: Untriaged → DOM: Content Processes
Ever confirmed: true
Product: Firefox → Core

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

Flags: needinfo?(hsivonen)
Fission Milestone: --- → ?
Fission Milestone: ? → Future
Component: DOM: Content Processes → DOM: UI Events & Focus Handling

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.

See Also: → 1676466

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

Flags: needinfo?(hsivonen) → needinfo?(emilio)
See Also: → 1745834

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.

Flags: needinfo?(emilio)
Attached file Standalone test-case

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.

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

Flags: needinfo?(hikezoe.birchill)
Flags: needinfo?(botond)

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

Flags: needinfo?(hikezoe.birchill) → needinfo?(mikokm)

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

Component: DOM: UI Events & Focus Handling → Graphics: WebRender

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

Flags: needinfo?(botond)
See Also: → 1749721
Has Regression Range: --- → yes
Keywords: regression
Severity: -- → S3

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

See Also: → 1750285
Assignee: nobody → emilio
Flags: needinfo?(mikokm)

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

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

Depends on D136893

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

Depends on D136894

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

Depends on D136895

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

Pushed by ealvarez@mozilla.com:
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
See Also: 1745834

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?

Flags: needinfo?(emilio)
Flags: in-testsuite+

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
Flags: needinfo?(emilio)
Attachment #9260636 - Flags: approval-mozilla-beta?
Attachment #9260660 - Flags: approval-mozilla-beta?
Flags: qe-verify+

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.

Attachment #9260636 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #9260660 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

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.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
Regressions: 1755188
You need to log in before you can comment on or make changes to this bug.