Bug 54198 - [GTK] Log signals from AtkDocument interface also in AccessibilityController
Summary: [GTK] Log signals from AtkDocument interface also in AccessibilityController
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks: 25831 53146
  Show dependency treegraph
 
Reported: 2011-02-10 05:49 PST by Mario Sanchez Prada
Modified: 2011-02-10 10:42 PST (History)
2 users (show)

See Also:


Attachments
Patch proposal (19.96 KB, patch)
2011-02-10 07:03 PST, Mario Sanchez Prada
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mario Sanchez Prada 2011-02-10 05:49:56 PST
Apart from logging all activity related to the AtkObject's signals being emitted, it would be useful as well to log those signals being emitted by objects implementing the AtkDocument interface.

Among other things, it would probably help to write a good layout test for patch for bug 25831, instead of having to work around the situation through a unit test.
Comment 1 Mario Sanchez Prada 2011-02-10 05:54:16 PST
I forgot to mention what exactly I'd like to get added in there. It would be the following signals:

  - load-complete
  - load-stopped
  - reload

http://library.gnome.org/devel/atk/unstable/AtkDocument.html#AtkDocument.signals
Comment 2 Mario Sanchez Prada 2011-02-10 07:03:45 PST
Created attachment 81976 [details]
Patch proposal

Patch proposal.

I've also made the most of this patch to fix some static functions and parameters names which were not following the code convention before (oops!)
Comment 3 Martin Robinson 2011-02-10 09:17:36 PST
Comment on attachment 81976 [details]
Patch proposal

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

Great. Please incorporate the following suggestions.

> Tools/DumpRenderTree/gtk/AccessibilityCallbacks.cpp:36
> +#include "DumpRenderTree.h"
> +#include "WebCoreSupport/DumpRenderTreeSupportGtk.h"
> +
> +#include <gtk/gtk.h>

No empty line needed here.

> Tools/DumpRenderTree/gtk/AccessibilityCallbacks.cpp:65
> +    gchar* actualObjectName = 0;
> +    if (!objectName || *objectName == '\0')
> +        actualObjectName = g_strdup("(No name)");
> +    else
> +        actualObjectName = g_strdup(objectName);
> +

I think you can just do this:
if (!objectName || *objectName == '\0')
    objectName = "No name";

and avoid some extra code.

> Tools/DumpRenderTree/gtk/AccessibilityCallbacks.cpp:86
> +    gchar* signalName = 0;

You can use GOwnPtr here.

> Tools/DumpRenderTree/gtk/AccessibilityCallbacks.cpp:102
> +    } else {
> +        signalName = g_strdup(signal_query.signal_name);
> +    }

No curlies here since this is only one line.
Comment 4 Mario Sanchez Prada 2011-02-10 10:42:40 PST
Committed r78244: <http://trac.webkit.org/changeset/78244>