RESOLVED FIXED 38844
[GTK] Add support for DOM events in the GObject DOM bindings
https://bugs.webkit.org/show_bug.cgi?id=38844
Summary [GTK] Add support for DOM events in the GObject DOM bindings
Xan Lopez
Reported 2010-05-10 07:02:50 PDT
SSIA
Attachments
events.diff (21.00 KB, patch)
2010-05-10 07:48 PDT, Xan Lopez
xan.lopez: commit-queue-
domevents.diff (23.95 KB, patch)
2010-05-11 03:44 PDT, Xan Lopez
no flags
signalnames.diff (3.73 KB, patch)
2010-05-11 05:31 PDT, Xan Lopez
no flags
testsignals.diff (7.02 KB, patch)
2010-05-11 06:43 PDT, Xan Lopez
no flags
eventobject.diff (24.69 KB, patch)
2010-05-25 07:44 PDT, Xan Lopez
no flags
Xan Lopez
Comment 1 2010-05-10 07:48:06 PDT
Created attachment 55553 [details] events.diff First patch, expose the DOM events as GObject signals.
WebKit Review Bot
Comment 2 2010-05-10 07:52:06 PDT
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
Comment 3 2010-05-11 03:44:16 PDT
Created attachment 55687 [details] domevents.diff This fixes the style issues that are possible to fix.
WebKit Review Bot
Comment 4 2010-05-11 03:47:49 PDT
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
Comment 5 2010-05-11 05:31:53 PDT
Created attachment 55690 [details] signalnames.diff GObject-ify signal names for DOM events.
Xan Lopez
Comment 6 2010-05-11 06:43:48 PDT
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
Comment 7 2010-05-25 07:44:35 PDT
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 8 2010-05-25 21:38:23 PDT
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 9 2010-05-25 21:41:53 PDT
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 10 2010-05-25 21:48:20 PDT
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 11 2010-05-25 21:56:13 PDT
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 12 2010-05-26 03:52:50 PDT
Comment on attachment 55687 [details] domevents.diff Pushed with the suggested changes in r60225.
Xan Lopez
Comment 13 2010-05-26 03:53:19 PDT
Comment on attachment 55690 [details] signalnames.diff Pushed with the suggested change in r60226.
Xan Lopez
Comment 14 2010-05-26 03:53:41 PDT
Comment on attachment 55698 [details] testsignals.diff This is an ex-patch.
Xan Lopez
Comment 15 2010-05-26 03:54:10 PDT
Comment on attachment 57017 [details] eventobject.diff Pushed together with previous patch as r60229.
Xan Lopez
Comment 16 2010-05-26 03:54:30 PDT
All patches committed, closing bug.
Note You need to log in before you can comment on or make changes to this bug.