[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 #3 from Gustavo Noronha (kov) <gns at gnome.org>  2011-12-03 03:45:43 PST ---
(From update of attachment 117752)
View in context: https://bugs.webkit.org/attachment.cgi?id=117752&action=review

It's looking good, just a first pass.

> Source/WebKit2/ChangeLog:11
> +        Implement the part in the UIProcess, by making the WebView widget

I think you can remove 'Implement the part in the UIProcess' pattern and have just the explanation.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:136
> +    if (priv->accessible) {
> +        g_object_unref(priv->accessible);
> +        priv->accessible = 0;
> +    }
> +

Is there a reason why we don't use GRefPtr for these?

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:357
> +    // If the socket as already been created and embedded a plug ID, return it.

s/as/has/

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:378
> +    gchar* plugIDString = g_strdup(plugID.utf8().data());

GOwnPtr?

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBaseAccessible.cpp:68
> +        /* If the widget is no longer alive, save some remote calls
> +           (because of AtkSocket's implementation of ref_state_set())
> +           and just return that this AtkObject is defunct. */

Comments should use C++ style, with // at the start of each line.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBaseAccessible.h:55
> +struct _WebKitWebViewBaseAccessible {
> +    AtkSocket parent;
> +    /*< private >*/
> +    WebKitWebViewBaseAccessiblePrivate* priv;
> +};

No need for padding?

> Source/WebKit2/UIProcess/API/gtk/tests/AccessibilityTestServer.cpp:38
> +    // Sent a message to the parent process telling we're ready.

s/Sent/Send/

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