[webkit-reviews] review granted: [Bug 122572] AX: Crash at WebCore::accessibleNameForNode when visiting Facebook : [Attachment 213818] patch

[bugzilla-daemon at webkit.org]

Mario Sanchez Prada <mario at webkit.org> has granted chris fleizach
<cfleizach at apple.com>'s request for review:
Bug 122572: AX: Crash at WebCore::accessibleNameForNode when visiting Facebook
https://bugs.webkit.org/show_bug.cgi?id=122572

Attachment 213818: patch
https://bugs.webkit.org/attachment.cgi?id=213818&action=review

------- Additional Comments from Mario Sanchez Prada <mario at webkit.org>
(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 :)