Closed Bug 1805876 Opened 2 years ago Closed 2 years ago

[CTW] Better fix for Gmail hit testing issue (bug 1801756)

Categories

(Core :: Disability Access APIs, enhancement)

enhancement

Tracking

()

RESOLVED FIXED
110 Branch
Tracking Status
firefox110 --- fixed

People

(Reporter: Jamie, Assigned: Jamie)

References

(Blocks 1 open bug)

Details

(Whiteboard: [ctw-m4])

Attachments

(2 files)

Spun off bug 1801756 comment 20.

It occurred to me that perhaps the reason we're seeing weirdness with the viewport cache (necessitating the hacky patch in bug 1801756) is that when we're building the viewport cache, we call GetAccessibleOrContainer, not GetAccessible. That means that if a particular content node doesn't have an Accessible (e.g. a div we pruned from the a11y tree) but does have a frame, we'll insert the container in its place. There might have been other descendants of that container that do have Accessibles, though. In that case, we've just put the container ahead of some of its descendants!
I'm not quite sure how to reproduce that, but maybe something like this (haven't verified yet):
data:text/html,<div role="main"><p>hi</p><div style="width: 10px; height: 10px;"></div><p>bye</p>
In this case, the inner div doesn't get an Accessible because it has no semantic value. The frames will probably be something like:
[paragraph for hi, inner div, paragraph for bye, outer div]
But because the inner div has no Accessible and we fall back to its container, the Accessibles will be:
[paragraph for hi, outer div, paragraph for bye]
I'm wondering whether we should be calling GetAccessible instead of GetAccessibleOrContainer. I worry that might cause us to miss some Accessibles, but I can't come up with a case where that would happen. If this works, this would allow us to revert the hacky code in ChildAtPoint, avoiding the performance and potential z-index problems that code introduces.

Whiteboard: [ctw-m4]

That patch was always somewhat hacky, but we couldn't figure out the cause of the problem.
Now that I've figured it out, a more correct solution is forthcoming.

Calling GetAccessibleOrContainer meant that if a particular content node didn't have an Accessible but did have a frame, we'd insert its container in its place.
There might have been other descendants of that container that did have Accessibles, though.
In that case, we would have put the container ahead of some of its descendants!
To fix this, use GetAccessible instead.
We'll still fall back to the container if the user hit tests a point in an inaccessible node, since it will appear later in the viewport cache.

Pushed by jteh@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2b8beff27d4c part 1: Revert the RemoteAccessible hit testing changes in bug 1801756. r=morgan https://hg.mozilla.org/integration/autoland/rev/9119a0a1bb5d part 2: Use GetAccessible instead of GetAccessibleOrContainer when building the viewport cache. r=morgan
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 110 Branch
Duplicate of this bug: 1806027
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: