[Webkit-unassigned] [Bug 96932] [GTK] [Stable] Infinite recursion in WebCore::AXObjectCache::getOrCreate

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=96932





--- Comment #10 from Joanmarie Diggs (irc: joanie) <jdiggs at igalia.com>  2012-09-20 19:12:04 PST ---
(In reply to comment #9)
> (From update of attachment 164839 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=164839&action=review
> 
> > Source/WebCore/accessibility/gtk/AccessibilityObjectAtk.cpp:95
> > +        for (RenderObject* r = renderer()->firstChild(); r; r = r->nextSibling()) {
> > +            if (r->isAnonymousBlock())
> > +                return IncludeObject;
> 
> I notice here that the check for child->isLink() is gone now. Was it unimportant before or was it replaced by another check? I'm also a bit confused by the change in logic. I'm probably reading it incorrectly.
> 
> My understanding:
> Before: If the child is a link or the first child isn't an anonymous block, include the object.
> Now: If any child is an anonymous block, include the object.
> 
> Not sure I understand that fully.

The whole Accessible Anonymous Block (is that an oxymoron?) situation makes my eyes cross. So I am sure I'm doing a poor job of explaining.

The change in logic is admittedly trading a bit of certainty for probability in order to achieve stability. FWIW there are already regression tests which I added back when I was initially sorting all of this out to ensure the critical problems don't creep back in. I think we'll find a new edge case here and there, but we can deal with those when they arise. Besides I'll take hypothetical edge case over a very real crasher any day of the week. ;)

Anyhoo.... 

Given a Para/Div which has at least one anonymous block child, we want to include the Para/Div.
Exception 1: Nested anonymous blocks are a mess. Defer to WebCore.
Exception 2: The first child of the anonymous block is a link. In that case we also want to include the Para/Div because that Para/Div is the HTML element which contains the link.

The old/current/crashy logic was:
  * If none of Para/Div's children is an anonymous block: Defer to WebCore.
  * If the first child of the first anonymous block found is a link: include Para/Div.
  * If the first child of the first anonymous block has no anonymous block children, we are not in a nested anonymous block situation: Include Para/Div.
  * Otherwise fall back on the default WebCore behavior to sort out the nested anonymous block mess.

The new/proposed logic is:
  * If we're pretty confident [1] none of Para/Div's children is an anonymous block: Defer to WebCore.
  * If Para/Div's parent is an anonymous block we may [2] have a mess of nested anonymous blocks: Defer to WebCore.
  * We are not in a nested anonymous block situation. Thus if Para/Div has any anonymous block children: include Para/Div. [3]

[1] What really matters / has been problematic is Anonymous Blocks *which contain text* getting included at the expense of ignoring the parent Para/Div. If there is no text under the Para/Div element at all, there cannot be Anonymous Blocks with text as immediate children.

[2] Then again, we may not. There's a chance for an unaddressed edge case here. Haven't found it yet. And perhaps WebCore is just doing the RightThing(tm) for us in these cases.

[3] The Para/Div->Anon Block->Link case gets addressed here because we include all Para/Div->Anon Block cases as long as the parent of the Para/Div is not also an Anon Block. And perhaps we are missing another edge case here. But falling back to WebCore to sort out this mess is not, to me, a huge risk.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.