Bug 25452 – AX: Don't create addition space AXStaticText element for every bold or link tag

• Component: Accessibility
• Status: RESOLVED FIXED

[chris fleizach]

In the following simplified html example with two bold tag and two links, I would expect four uiElements represented thru accessibility.  Instead, I see eight uiElements.  Somehow WebKit is creating another AXStaticText that contains a space character as the AXValue after every bold or link tag.  See the screenshot for an example of one of these spacer AXStaticText.  Note that the start marker is the same as the end marker.

<b>First</b>
<b>Second</b>
<a href="http://www.apple.com">Apple</a>
<a href="http://www.yahoo.com">Yahoo</a>

[chris fleizach]

Created attachment 29855 [details]
patch

[Darin Adler]

Comment on attachment 29855 [details]
patch 

> +        // text elements that are just empty whitespace should not be returned
> +        String text = renderText->text()->simplifyWhiteSpace();
> +        if (text.isNull() || text.isEmpty())
> +            return true;
> +        return false;

This should just be:

    return renderText->text()->simplifyWhiteSpace().isEmpty();

There's no need for a separate isNull check since null strings are also guaranteed to be empty. But really, to be efficient, it should be:

    return renderText->text()->containsOnlyWhitespace();

Unfortunately, that function is currently defined for StringImpl* but not for String. Since RenderText guarantees that text() will never return null (you can see assertions to that effect inside the class), you can do this:

    return renderText->text()->impl()->containsOnlyWhitespace();

I値l say review- because it would be better to do the more efficient version.

[chris fleizach]

Created attachment 29856 [details]
patch

[Darin Adler]

Comment on attachment 29856 [details]
patch

Same comments on this patch, but this also seems to include other seemingly-unrelated changes. Maybe fixing two bugs at once?

[chris fleizach]

you're too quick for me. first time uploaded i forgot the DumpRenderTree changes
i obsoleted the first patch, then tried to upload again, but didn't see the comments until i had uploaded

[chris fleizach]

Created attachment 29857 [details]
patch

updated patch based on darin's review.

[chris fleizach]

it seems that i can use

renderText->text()->containsOnlyWhitespace();
without problem

[Darin Adler]

Comment on attachment 29857 [details]
patch

r=me

[chris fleizach]

http://trac.webkit.org/changeset/42946