Fuzzy hittesting breaks on LinkedIn post headers
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox116 | --- | fixed |
People
(Reporter: morgan, Assigned: morgan)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
STR:
- Visit LinkedIn.com
- In the main feed, attempt to hittest the name of the post author with VO enabled and "VO cursor follows mouse" enabled
Expected:
The post author is read
Actual:
Nothing is read
I think this issue stems from the fact that our fuzzy hittesting algorithm only works on generic containers with one child. On LinkedIn, these generic containers can sometimes contain two children: one text leaf with whitespace, and another generic containing the text leaf we're actually targeting.
Assignee | ||
Comment 1•1 year ago
|
||
I think this can be generalised to "we don't cope when the nested-generic chain we expect has occasional non-generic siblings".
I wonder if we can make this algorithm a bit more specific by relying on the clipped nature of these accs (the method linkedin is using to "hide" them). Right now we have a snippet that looks like this:
if (lastMatch->Role() == roles::TEXT_CONTAINER) {
// We've matched on a generic, we probably want its
// inner text leaf (if one exists). Drill down through
// subsequent generics, regardless of whether the point
// we want is actually contained therein."
while (lastMatch->ChildCount() == 1) {
if (lastMatch->Role() == roles::TEXT_CONTAINER) {
lastMatch = lastMatch->RemoteChildAt(0);
} else {
break;
}
}
// If we failed to find a text leaf, fall back to the
// original generic match.
lastMatch = lastMatch->IsTextLeaf() ? lastMatch : acc;
I wonder if we'd be better off with:
if (lastMatch->Role() == roles::TEXT_CONTAINER) {
// Check if this container has a clipped child.
// This usually indicates invisible text, and we're
// interested in returning the inner text content
// even if it doesn't contain the point we're hittesting.
RemoteAccessible* clippedContainer = nullptr;
for (RemoteAccessible* child = lastMatch->RemoteFirstChild(); child; child = child->RemoteNextSibling()) {
if (child->Role() == roles::TEXT_CONTAINER) {
if (child->Bounds.IsEmpty()) {
clippedContainer = child;
break;
}
}
}
// If we found a clipped container, descend it in search of
// meaningful text leaves. If we encounter multiple, or
// if there are additional child accs, fall back to the closest
// match.
RemoteAccessible* maybeTextLeaf = clippedContainer;
while (maybeTextLeaf && maybeTextLeaf->ChildCount() == 1) {
RemoteAccessible* child = maybeTextLeaf->RemoteChildAt(0);
// This feels like it'd be better written as a case statement, but I couldn't figure out how to do it
if (child->Role() == roles::TEXT_CONTAINER) {
maybeTextLeaf = child;
} else if (child->Role() == roles::TEXT_LEAF) {
maybeTextLeaf = child;
break;
} else {
break;
}
}
if (maybeTextLeaf && (maybeTextLeaf->IsTextLeaf() || maybeTextLeaf != clippedContainer)) {
// If we found something, it's either a text leaf or a non-clipped container deeper than the
// original match. Return it. This is allows for more fuzziness than our current code.
lastMatch = maybeTextLeaf;
}
}
break;
This won't fix situations where subsequent TEXT_CONTAINERS have siblings, but perhaps it makes sense to add a similar sibling walk there, too?
My thinking is that if we can narrow the initial scope of this case by only doing the deep dive on containers we anticipate finding invisible text in, maybe we can be more liberal with how much walking of that subtree we do. I don't want to end up in a situation where we're doing a complete subtree walk every time we match on a generic :(
Comment 2•1 year ago
|
||
if (child->Bounds.IsEmpty()) {
Will this ever work as expected? It may not because we use ink overflow if the frame has a 0 area, so I generally see 1 x 1 or something like that in this case. But maybe this would cover the cases it needs to cover...
Updated•1 year ago
|
Assignee | ||
Comment 3•1 year ago
|
||
Comment 5•1 year ago
|
||
bugherder |
Updated•1 year ago
|
Description
•