[Webkit-unassigned] [Bug 72589] [GTK] Expose accessibility hierarchy in WebKit2 to ATK/AT-SPI based ATs

[bugzilla-daemon at webkit.org]

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





--- Comment #11 from Carlos Garcia Campos <cgarcia at igalia.com>  2012-01-04 00:45:46 PST ---
(From update of attachment 120955)
View in context: https://bugs.webkit.org/attachment.cgi?id=120955&action=review

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:341
> +static AtkObject* webkitWebViewBaseGetAccessible(GtkWidget *widget)

GtkWidget *widget -> GtkWidget* widget

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBaseAccessible.cpp:64
> +static AtkStateSet* webkitWebViewBaseAccessibleRefStateSet(AtkObject *atkObject)

AtkObject *atkObject -> AtkObject* atkObject

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBaseAccessible.cpp:68
> +    AtkStateSet* state_set;

state_set -> stateSet

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBaseAccessible.cpp:83
> +static gint webkitWebViewBaseAccessibleGetIndexInParent(AtkObject *atkObject)

AtkObject *atkObject -> AtkObject* atkObject

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBaseAccessible.cpp:85
> +    g_return_val_if_fail(ATK_IS_OBJECT(atkObject), -1);

Don't use g_return macros in private static code, note that atk_object_get_index_in_parent() already has that check, so it doesn't get here if pointer is not a valid AtkObject.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBaseAccessible.cpp:128
> +    AtkObject* object = static_cast<AtkObject*>(g_object_new(WEBKIT_TYPE_WEB_VIEW_BASE_ACCESSIBLE, 0));

Use ATK_OBJECT() instead of static_cast. Use NULL instead of 0 in g_object_new() since it's a sentinel

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBaseAccessible.h:31
> +#include <webkit2/WebKitDefines.h>

Do you need this?

> Source/WebKit2/UIProcess/API/gtk/tests/AccessibilityTestServer.cpp:55
> +    WebKitWebLoaderClient* loaderClient = webkit_web_view_get_loader_client(webView);

Loader client has been already removed, use WebKitWebView::load-changed instead.

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitAccessibility.cpp:58
> +    int child_stdout = 0;

child_stdout -> childStdout

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitAccessibility.cpp:113
> +    GRefPtr<AtspiAccessible> desktop = atspi_get_desktop(0);

atspi_get_desktop returns a new ref, so you should use adoptGRef() here.

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitAccessibility.cpp:136
> +    GRefPtr<AtspiAccessible> currentChild = atspi_accessible_get_child_at_index(currentParent.get(), 0, 0);

I think you should use adoptGRef here too. Use Test::assertObjectIsDeletedWhenTestFinishes() to make sure you don't leak anything.

> Source/WebKit2/WebProcess/WebPage/gtk/WebPageAccessibilityObjectGtk.cpp:78
> +    g_return_val_if_fail(ATK_IS_OBJECT(atkObject), -1);

Don't use g_return macro here.

> Source/WebKit2/WebProcess/WebPage/gtk/WebPageAccessibilityObjectGtk.cpp:110
> +    accessible->m_page = 0;

accessible is already initialized to 0 when allocated.

> Source/WebKit2/WebProcess/WebPage/gtk/WebPageAccessibilityObjectGtk.cpp:128
> +    AtkObject* object = static_cast<AtkObject*>(g_object_new(WEB_TYPE_PAGE_ACCESSIBILITY_OBJECT, 0));

Use NULL instead of 0 in g_object_new

> Tools/Scripts/run-gtk-tests:98
> +    paths_to_check = [ "/usr/libexec",
> +                       "/usr/lib/at-spi2-core",
> +                       "/usr/lib32/at-spi2-core"
> +                       "/usr/lib64/at-spi2-core" ]

Maybe we can use pkg-config --variable=exec_prefix atspi-2 ?

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