Closed Bug 1566599 Opened 4 years ago Closed 4 years ago

Respect ForceEmptyHitRegion flag in WR hit-test

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla77
Fission Milestone M6
Tracking Status
firefox-esr68 --- unaffected
firefox68 --- wontfix
firefox69 --- wontfix
firefox70 --- wontfix
firefox75 --- wontfix
firefox76 --- wontfix
firefox77 --- fixed

People

(Reporter: kats, Assigned: kats)

References

(Blocks 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(5 files)

There's the notion of the ForceEmptyHitRegion flag, which is used when an entire out-of-process subdocument should be ignored for hit-testing purposes. The way this works is the nsDisplayRemote puts the flag on the RefLayer here (for the non-WR codepath) or on the WR scroll data here. This then gets slurped into the HitTestingTreeNode here.

During hit-testing, the non-WR codepath checks this flag here. But in the WR codepath, the equivalent check is missing. Note that this flag is not part of the CompositorHitTestInfo items (because the flag is known in the parent process from where the CHTI items are generated) and so the check must be done in APZ code after the WR hit-test happens. Bug 1563622 adds a check for the very similar ForceDispatchToContent flag that can be used as a model.

The catch is that the semantics of the ForceEmptyHitRegion flag means that the item needs to be skipped over during hit-testing. When we do a WR hit-test, we get back a list of hit results in front-to-back ordering and iterate that list to pull out the frontmost item. Instead, we need to somehow modify this to keep iterating until we reach the frontmost item which does not have a ForceEmptyHitRegion item on the corresponding HitTestingTreeNode instance. Doing this will entail either passing in additional information (like a map of guids -> flags) to wr_api_hit_test, or returning the whole list of items from wr_api_hit_test, or some other way of combining the information from the WR hit-test with the flags stored in C++ code.

Similar machinery will be desirable for bug 1542020 (to determine exactly which hit-tests are ambiguous - see bug 1542020 comment 0) so it might make sense to write this with that in mind as well.

IIUC, this should only affect:

  1. With Fission enabled, pages where an OOP iframe has pointer-events: none.
  2. (Hypothetically) with Fission disabled, cases where the browser UI sets pointer-events:none on the entire content area.

#1 suggests that we should track this for Fission. (Probably M5 unless we run into the issue on popular sites.)

#2 presumably doesn't happen, or we would've run into it already.

It's technically a regression in that non-WR handled these scenarios and WR doesn't, but as it doesn't actually occur in shipping configurations, it's not a high priority one. I'm going to mark it as fix-optional for now.

Fission Milestone: --- → ?
Fission Milestone: ? → M5
Fission Milestone: M5 → M6
Blocks: 1625249
See Also: 1625249
Assignee: nobody → kats

No functional changes here, just plumbing to allow the caller to
access all the results instead of just the one picked out by
bindings.rs.

The WIPs attached here should be complete, but I want to write a test before requesting review.

I wrote a test and got it passing, but it uncovered a couple of other issues with these WIPs that I'm sorting through at the moment. Latest try push is at https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=8003eed4d2bf54ba1d92c66cdb88abe267a40013

We need to propagate the flags from the reflayer into the descendant subtree,
but remove the flag from the HTTN corresponding to the reflayer itself. See
comments in the patch for why.

Depends on D69203

For the case where we got a hit-result with a NULL_SCROLL_ID, we wouldn't
get a node, and would fall back to the root APZC for the layers id. But we
should actually still find the HTTN so that we can check the override flags.

Depends on D70395

Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b09ad6ab0220
Have the WR hit-test return all results to the caller. r=botond
https://hg.mozilla.org/integration/autoland/rev/97cb47649452
Add support for ForceEmptyHitRegion on the WR hit-test. r=botond
https://hg.mozilla.org/integration/autoland/rev/bef9f8968a42
Ensure EventRegionOverride flags don't end up on the HTTNs for reflayers. r=botond
https://hg.mozilla.org/integration/autoland/rev/c814b414cd82
Ensure we still check the flags on the node for the NULL_SCROLL_ID case. r=botond
https://hg.mozilla.org/integration/autoland/rev/02639d0327aa
Add a test for the ForceEmptyHitRegion flag. r=botond
Blocks: 1607375
Blocks: 1542020
See Also: 1542020
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.