| Summary: | AX: Crash at WebCore::accessibleNameForNode when visiting Facebook | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | chris fleizach <cfleizach> | ||||
| Component: | Accessibility | Assignee: | chris fleizach <cfleizach> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | aboxhall, apinheiro, commit-queue, dmazzoni, jdiggs, mario, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Attachments: |
|
||||||
|
Description
chris fleizach
2013-10-09 13:46:57 PDT
Looks like they're probably doing something like <button aria-labeledby="item"> <div id="item" style="display:none">data</div> Created attachment 213818 [details]
patch
Comment on attachment 213818 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=213818&action=review > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1814 > - String text = node->document().axObjectCache()->getOrCreate(node)->textUnderElement(); > + // If the node can be turned into an AX object, we can use standard name computation rules. > + // If however, the node cannot (because there's no renderer e.g.) fallback to using the basic text underneath. > + AccessibilityObject* axObject = node->document().axObjectCache()->getOrCreate(node); > + String text; > + if (axObject) > + text = axObject->textUnderElement(); > + else if (node->isElementNode()) > + text = toElement(node)->innerText(); > + I wonder if we could fix this issue in a different way, by making AXObjectCache::getOrCreate(Node* node) able to handle this kind of situation, creating a AccessibilityNodeObject that wraps that not rendered node inside. What do you think? (In reply to comment #3) > (From update of attachment 213818 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=213818&action=review > > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1814 > > - String text = node->document().axObjectCache()->getOrCreate(node)->textUnderElement(); > > + // If the node can be turned into an AX object, we can use standard name computation rules. > > + // If however, the node cannot (because there's no renderer e.g.) fallback to using the basic text underneath. > > + AccessibilityObject* axObject = node->document().axObjectCache()->getOrCreate(node); > > + String text; > > + if (axObject) > > + text = axObject->textUnderElement(); > > + else if (node->isElementNode()) > > + text = toElement(node)->innerText(); > > + > > I wonder if we could fix this issue in a different way, by making AXObjectCache::getOrCreate(Node* node) able to handle this kind of situation, creating a AccessibilityNodeObject that wraps that not rendered node inside. > > What do you think? Certainly getOrCreate could return an object, but there's a bit of code to ensure we only return a AXNode based object under certain conditions (either in a canvas or aria-hidden=false). Hard for me to say if there's any negative consequence of doing that. It might allow more elements into the tree than we would like... For example, we definitely don't want to add objects that are just hidden to the tree, as in <div style="display:none;"> sub tree here </div> So it could be done, but I am less sure of the consequences of doing that right now. Thoughts Comment on attachment 213818 [details] patch (In reply to comment #4) > > I wonder if we could fix this issue in a different way, by making > > AXObjectCache::getOrCreate(Node* node) able to handle this kind of situation, > > creating a AccessibilityNodeObject that wraps that not rendered node inside. > > > > What do you think? > > Certainly getOrCreate could return an object, but there's a bit of code > to ensure we only return a AXNode based object under certain conditions > (either in a canvas or aria-hidden=false). Hard for me to say if there's > any negative consequence of doing that. It might allow more elements > into the tree than we would like... > Yes, I've seen that code and it's not clear to me either. That's why I ask. > For example, we definitely don't want to add objects that are just hidden > to the tree, as in <div style="display:none;"> sub tree here </div> > Agreed. > So it could be done, but I am less sure of the consequences of doing that > right now. > Fair enough. Let's give this a try then :) Comment on attachment 213818 [details] patch Clearing flags on attachment: 213818 Committed r157221: <http://trac.webkit.org/changeset/157221> All reviewed patches have been landed. Closing bug. |