Closed Bug 1774197 Opened 1 year ago Closed 11 months ago

[CTW] Cached hit testing problems

Categories

(Core :: Disability Access APIs, defect)

defect

Tracking

()

RESOLVED FIXED
103 Branch
Tracking Status
firefox103 --- fixed

People

(Reporter: Jamie, Assigned: Jamie)

References

(Blocks 1 open bug)

Details

(Whiteboard: [ctw-m2])

Attachments

(3 files)

If you call cached ChildAtPoint on a document with the coordinates of something in an iframe, it will only go as deep as the iframe. Bug 1758689 did add code to support this, but it isn't working as intended.

I didn't spot this in review, but I think I understand the problem having looked into it briefly. We check for a doc using IsDoc() and recurse into it... but we'll never get an inner doc in the viewport, nor could we handle it if we did! Instead, we will get an OuterDocAccessible (an iframe). We need to detect that (IsOuterDoc) and then recurse into the document inside it.

I have a patch to fix this locally, but it's still not quite working. It seems to work for a few nodes near the top of the iframe document and then fail below that. I guess this could have something to do with bug 1772861, but it seems a bit too extreme for that.

Blocks: a11y-ctw
Depends on: 1758689
Whiteboard: [ctw-m2]

We were previously looking for a doc (IsDoc), but we'll never get any doc in a viewport cache except for the doc whose viewport we are searching.
Instead, if we encounter an iframe, we'll hit an OuterDoc (IsOuterDoc) which is the Accessible for the iframe element.
In that case, we walk inside that OuterDoc to get its embedded document, then recurse from there.

Assignee: nobody → jteh
Status: NEW → ASSIGNED

Widening the scope of this bug to cover several other issues that impacted tests and real usage and are all somewhat dependent.

Summary: [CTW] Cached hit testing won't descend into iframes → [CTW] Cached hit testing problems

This was moved to a post refresh observer on the misunderstanding that this would allow us to benefit from retained display lists.
In reality, it doesn't; work would need to be done in layout to benefit from those, and since there are different display lists for hit testing and painting, this may not be feasible anyway.
Furthermore, the post refresh implementation was broken because WillRefresh often removes the post refresh observer, which meant that a viewport cache update was never actually pushed!
We could fix this by adding the post refresh observer only if the viewport cache is dirty, removing the post refresh observer in DidRefresh and also removing the post refresh observer in Shutdown only if the viewport cache is dirty.
However, given that we can't benefit from retained display lists anyway, using a post refresh observer doesn't serve any purpose at this stage.
Among other things, this fixes intermittent problems with image maps, which often get inserted into the tree after the initial tree is built.

Sometimes, the document occurs too early in the viewport cache, perhaps even right at the start.
We weren't benefitting from it being in the cache anyway, since we always skipped it.
We already have a fallback in ChildAtPoint for the case where we didn't find a matching Accessible, so we rely on that to handle returning the document when appropriate.

Attachment #9281192 - Attachment description: Bug 1774197 WIP: Correctly recurse into iframe documents in RemoteAccessibleBase::ChildAtPoint. → Bug 1774197 part 3: Correctly recurse into iframe documents in RemoteAccessibleBase::ChildAtPoint.
Pushed by jteh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/47d9bd41b394
part 1: Move sending of the viewport cache to ProcessQueuedCacheUpdates. r=morgan
https://hg.mozilla.org/integration/autoland/rev/b29d022250df
part 2: Don't include the document in the viewport cache. r=morgan
https://hg.mozilla.org/integration/autoland/rev/d5c7090114bf
part 3: Correctly recurse into iframe documents in RemoteAccessibleBase::ChildAtPoint. r=morgan
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → 103 Branch
You need to log in before you can comment on or make changes to this bug.