Bug 38844 – [GTK] Add support for DOM events in the GObject DOM bindings

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

[Xan Lopez]

SSIA

[Xan Lopez]

Created attachment 55553 [details]
events.diff

First patch, expose the DOM events as GObject signals.

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:764:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:766:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:767:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/bindings/gobject/GObjectEventListener.cpp:39:  One line control clauses should not use braces.  [whitespace/braces] [4]
WebCore/bindings/gobject/GObjectEventListener.h:25:  Alphabetical sorting problem.  [build/include_order] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:124:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:126:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:127:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestCallback.cpp:171:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestCallback.cpp:173:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestCallback.cpp:174:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 11 in 9 files


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

[Xan Lopez]

Created attachment 55687 [details]
domevents.diff

This fixes the style issues that are possible to fix.

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:766:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestInterface.cpp:126:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestCallback.cpp:173:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 3 in 9 files


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

[Xan Lopez]

Created attachment 55690 [details]
signalnames.diff

GObject-ify signal names for DOM events.

[Xan Lopez]

Created attachment 55698 [details]
testsignals.diff

Add the infrastructure to test the DOM event signals and test the one signal in DOMWindow which does not require interactivity.

[Xan Lopez]

Created attachment 57017 [details]
eventobject.diff

This patch creates a WebKitDOMEvent class to go with every event signal, and the WebKitDOMEventTarget interface to be implemented by all classes that can be the target of an event. Still no support for the event subclasses in the DOM spec, or for userCapture events, those will come in future patches.

[Gustavo Noronha (kov)]

Comment on attachment 55687 [details]
domevents.diff

WebCore/bindings/gobject/GObjectEventListener.h:31
 +          static PassRefPtr<GObjectEventListener> create(GObject* object, const char* signalName) { return adoptRef(new GObjectEventListener(object, signalName)); }
Indentation looks weird here.

WebCore/bindings/gobject/GObjectEventListener.h:49
 +      virtual void handleEvent(ScriptExecutionContext*, Event*); /*  */
What's the /*  */ for? =)

WebCore/bindings/scripts/test/GObject/WebKitDOMTestCallback.cpp:184
 +  
One too many empty lines here, for my taste.

A good first step IMO. And good improvement on style conformance, too.

[Gustavo Noronha (kov)]

Comment on attachment 55690 [details]
signalnames.diff

WebCore/bindings/scripts/CodeGeneratorGObject.pm:494
 +      my ${listenerName} =  $domSignalName . "Listener";
there's an additional space that seems unnecessary here

Clever idea, the breakWords one.

[Gustavo Noronha (kov)]

Comment on attachment 55698 [details]
testsignals.diff

 56     g_idle_add((GSourceFunc)finish_loading, fixture);
 57     g_main_loop_run(fixture->loop);

Why are you doing this here? I would expect the string to be loaded synchronously, as it is now. Also, I think this test is lacking an assertion that fixture->counter has been incremented at all. I don't think this test is really testing what it's supposed to test, so I'll r-.

[Gustavo Noronha (kov)]

Comment on attachment 57017 [details]
eventobject.diff

Looks good to me. And this is more like it, test-wise. I recommend bringing the creation of the test to this patch, with the changes this one contains already in place, instead of cooking up a separate patch for the test. r=me with this change

[Xan Lopez]

Comment on attachment 55687 [details]
domevents.diff

Pushed with the suggested changes in r60225.

[Xan Lopez]

Comment on attachment 55690 [details]
signalnames.diff

Pushed with the suggested change in r60226.

[Xan Lopez]

Comment on attachment 55698 [details]
testsignals.diff

This is an ex-patch.

[Xan Lopez]

Comment on attachment 57017 [details]
eventobject.diff

Pushed together with previous patch as r60229.

[Xan Lopez]

All patches committed, closing bug.