Bug 98358 – [GTK] accessibility/aria-scrollbar-role.html is failing

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Zan Dobersek]

accessibility/aria-scrollbar-role.html is failing on all GTK platforms.
http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=accessibility%2Faria-scrollbar-role.html

[Krzysztof Czech]

Created attachment 193565 [details]
Patch

[Martin Robinson]

Comment on attachment 193565 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=193565&action=review

Some unnecessary string duplication going on, but otherwise looks good to me. :)

> Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:514
> +    GOwnPtr<gchar> axOrientation;
> +    if (checkElementState(m_element, ATK_STATE_HORIZONTAL))
> +        axOrientation.set(g_strdup_printf("AXOrientation: AXHorizontalOrientation"));
> +    else if (checkElementState(m_element, ATK_STATE_VERTICAL))
> +        axOrientation.set(g_strdup_printf("AXOrientation: AXVerticalOrientation"));
> +
> +    if (!axOrientation)
> +        return JSStringCreateWithCharacters(0, 0);
> +
> +    return JSStringCreateWithUTF8CString(axOrientation.get());

See below.

> Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:552
> +    GOwnPtr<gchar> axOrientation;
> +    if (checkElementState(m_element, ATK_STATE_HORIZONTAL))
> +        axOrientation.set(g_strdup_printf("AXOrientation: AXHorizontalOrientation"));
> +    else if (checkElementState(m_element, ATK_STATE_VERTICAL))
> +        axOrientation.set(g_strdup_printf("AXOrientation: AXVerticalOrientation"));
> +

g_strdump_printf can just be g_strdump and even more simply can just be like this:

const char* axOrientation = 0;
if (checkElementState(m_element, ATK_STATE_HORIZONTAL))
     axOrientation = "AXOrientation: AXHorizontalOrientation";
...

[Martin Robinson]

Would love to have Joanie or Mario double-check this one.

[Mario Sanchez Prada]

(In reply to comment #3)
> Would love to have Joanie or Mario double-check this one.

I agree with martin comments and suggestions. Apart from that, the patch looks good to me, a11y-wise.

[Krzysztof Czech]

Created attachment 193782 [details]
Patch

[Krzysztof Czech]

(In reply to comment #4)
> (In reply to comment #3)
> > Would love to have Joanie or Mario double-check this one.
> 
> I agree with martin comments and suggestions. Apart from that, the patch looks good to me, a11y-wise.

Thanks.
Applied Martin's suggestions

[WebKit Review Bot]

Comment on attachment 193782 [details]
Patch

Clearing flags on attachment: 193782

Committed r146312: <http://trac.webkit.org/changeset/146312>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.