Bug 106342 – [GTK][WK2] accessibility/language-attribute.html is failing

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Zan Dobersek]

The accessibility/language-attribute.html layout test is failing on GTK WK2 port.

Here's the diff:
--- /home/zan/Dev/webkit/webkit/WebKitBuild/Release/layout-test-results/accessibility/language-attribute-expected.txt
+++ /home/zan/Dev/webkit/webkit/WebKitBuild/Release/layout-test-results/accessibility/language-attribute-actual.txt
@@ -4,4 +4,4 @@
 elvish
 
 and more english
-Passed
+Failed. Could not find AXLanguages: -

[Krzysztof Czech]

Created attachment 197020 [details]
Patch

[Mario Sanchez Prada]

Comment on attachment 197020 [details]
Patch

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

> Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:593
> +    for (GSList* textAttributes = atk_text_get_default_attributes(ATK_TEXT(m_element)); textAttributes; textAttributes = textAttributes->next) {
> +        AtkAttribute* atkAttribute = static_cast<AtkAttribute*>(textAttributes->data);
> +        if (!strcmp(atkAttribute->name, atk_text_attribute_get_name(ATK_TEXT_ATTR_LANGUAGE))) {
> +            language.set(g_strdup_printf("AXLanguage: %s", atkAttribute->value));
> +            return JSStringCreateWithUTF8CString(language.get());
> +        }
> +    }

You are leaking both the returned GSList* and the data in each of its elements here, since atk_text_get_default_attributes() is transfer-full.

So, you should use a GOwnPtr<GSList> for that least, although you would still need to make sure that both the AtkAttribute structs and the gchar* pointers inside them (name, value) are freed before returning from this function.

[Krzysztof Czech]

Created attachment 198729 [details]
Patch

[Krzysztof Czech]

> You are leaking both the returned GSList* and the data in each of its elements here, since atk_text_get_default_attributes() is transfer-full.
> 
> So, you should use a GOwnPtr<GSList> for that least, although you would still need to make sure that both the AtkAttribute structs and the gchar* pointers inside them (name, value) are freed before returning from this function.

Good point. Applied suggestions.

[Mario Sanchez Prada]

(In reply to comment #4)
> > You are leaking both the returned GSList* and the data in each of its elements here, since atk_text_get_default_attributes() is transfer-full.
> > 
> > So, you should use a GOwnPtr<GSList> for that least, although you would still need to make sure that both the AtkAttribute structs and the gchar* pointers inside them (name, value) are freed before returning from this function.
> 
> Good point. Applied suggestions.

Thanks for considering those. The patch looks good to me now.

[WebKit Commit Bot]

Attachment 198729 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/platform/gtk-wk2/TestExpectations', u'Tools/ChangeLog', u'Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp']" exit_code: 1
Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:48:  Missing space before ( in for(  [whitespace/parens] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Krzysztof Czech]

Created attachment 198819 [details]
Patch

[Krzysztof Czech]

Corrected style errors.

[Gyuyoung Kim]

Comment on attachment 198819 [details]
Patch

rs=me based on Mario informal review.

[Mario Sanchez Prada]

Comment on attachment 198819 [details]
Patch

Adding to the commit queue, as per Krzystof request

[WebKit Commit Bot]

Comment on attachment 198819 [details]
Patch

Clearing flags on attachment: 198819

Committed r148737: <http://trac.webkit.org/changeset/148737>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.