[webkit-reviews] review denied: [Bug 34449] [Gtk] Evaluate and create tests for all the AtkRole's implemented by WebKitGtk : [Attachment 49618] testatkroles: test non-form roles

[bugzilla-daemon at webkit.org]

Xan Lopez <xan.lopez at gmail.com> has denied  review:
Bug 34449: [Gtk] Evaluate and create tests for all the AtkRole's implemented by
WebKitGtk
https://bugs.webkit.org/show_bug.cgi?id=34449

Attachment 49618: testatkroles: test non-form roles
https://bugs.webkit.org/attachment.cgi?id=49618&action=review

------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
>From 464999993ff6f20eaa76c4789cc607a5f4e9d84c Mon Sep 17 00:00:00 2001
>From: Diego Escalante Urrelo <descalante at igalia.com>
>Date: Thu, 25 Feb 2010 17:47:56 -0500
>Subject: [PATCH] testatkroles: test non-form roles
>
>This corresponds to joanmarie's original patch.
>
>Bug #34449

We need an actual ChangeLog entry.


>+/* vim: set sw=4 ts=4 sts=4 et: */

WebKit style does not allow to have this stuff.

>+
>+#include <errno.h>
>+#include <unistd.h>

Do you actually use these?

>+static gboolean _get_child_and_test_role(AtkObject *obj, gint pos, AtkRole
role)

The '*' are wrong, and there's no need at all to prefix the function with '_'
since it's already static.

>+{
>+    AtkObject *child;

Wrong '*'... happens in many other places, so just go through the file again.

>+    AtkRole child_role;
>+
>+    child = atk_object_ref_accessible_child(obj, pos);
>+    g_assert(child);
>+    child_role = atk_object_get_role(child);
>+    g_assert(child_role == role);
>+
>+    g_object_unref(child);
>+
>+    return TRUE;

Seems a bit pointless to make this a gboolean returning function since a) it
always returns TRUE b) you ignore it anyway?

Other than this, looks good.