Respect ForceEmptyHitRegion flag in WR hit-test
Categories
(Core :: Panning and Zooming, defect, P3)
Tracking
()
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.
Updated•4 years ago
|
Comment 1•4 years ago
|
||
IIUC, this should only affect:
- With Fission enabled, pages where an OOP iframe has
pointer-events: none
. - (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.
Updated•4 years ago
|
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 2•4 years ago
|
||
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.
Assignee | ||
Comment 3•4 years ago
|
||
Depends on D69202
Assignee | ||
Comment 4•4 years ago
|
||
The WIPs attached here should be complete, but I want to write a test before requesting review.
Assignee | ||
Comment 5•4 years ago
|
||
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
Assignee | ||
Comment 6•4 years ago
|
||
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
Assignee | ||
Comment 7•4 years ago
|
||
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
Assignee | ||
Comment 8•4 years ago
|
||
Depends on D70396
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
Comment 10•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b09ad6ab0220
https://hg.mozilla.org/mozilla-central/rev/97cb47649452
https://hg.mozilla.org/mozilla-central/rev/bef9f8968a42
https://hg.mozilla.org/mozilla-central/rev/c814b414cd82
https://hg.mozilla.org/mozilla-central/rev/02639d0327aa
Updated•4 years ago
|
Assignee | ||
Updated•3 years ago
|
Updated•2 years ago
|
Description
•