[Webkit-unassigned] [Bug 121558] New: AX: Crash when trying to retrieve textual information for a heading after hiding it

[bugzilla-daemon at webkit.org]

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

           Summary: AX: Crash when trying to retrieve textual information
                    for a heading after hiding it
           Product: WebKit
           Version: 528+ (Nightly build)
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: Accessibility
        AssignedTo: webkit-unassigned at lists.webkit.org
        ReportedBy: mario at webkit.org
                CC: cfleizach at apple.com,
                    webkit-bug-importer at group.apple.com
            Blocks: 121556


While working on bug 121556, I found that the reason why it crashes in AccessibilityNodeObject::textUnderElement() is because some nodes in the accessibility tree are being detached between one iteration and the following one inside the for loop in that function.

More specifically too, the problem happens exactly after we call child->textUnderElement() in the snipped of code below:

    StringBuilder builder;
    for  (AccessibilityObject* child = firstChild(); child; child = child->nextSibling()) {

        [...]

        String childText = child->textUnderElement(mode);
        if (childText.length()) {
            if (shouldAddSpaceBeforeAppendingNextElement(builder, childText))
                builder.append(' ');
            builder.append(childText);
        }
    }

Before the call to child->textUnderElement(), and considering the accessibility/heading-level.html test as our scenario, a hypothetical call to child->nextSibling() might return 0x0, which is fine, suggesting that there no more children after that one. However, if we call again child->nextSibling() after the call to textUnderElement(), we get a beautiful segfault error, which is the crashing we are seeing in the bots.

After some investigation, I found that the reason for that seems to be that in r155378 a forced call to update the layout has been added to the constructor of the TextIterator, which is used in AccessibilityRenderObject::textUnderElement() for text objects. And while that addition seems to be correct according to the comments in bug 120891, it's also causing that in certain scenarios (like the one detected with this test) we have the accessibility tree being altered between iterations of that for loop, causing that the 'child' pointer that we have at the beginning of the iteration is no longer valid, thus leading to the crash.

I have trying to get a simplified version of a layout test based on heading-level.html to proof the problem and also a fix for that, which I hope to attach soon.

Btw, this only seems to happen in GTK/EFL for some reason, probably because of the way DRT/WKTR is implemented, but in any case I think there might be a bug here and that it could be as easy to call to updateLayout() before the loop so we make sure that further uses of TextIterator won't cause this weird behaviour.

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