Summary: | [GTK] Add support for DOM events in the GObject DOM bindings | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Xan Lopez <xan.lopez> | ||||||||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | tevaum, tonikitoo, webkit.review.bot | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | PC | ||||||||||||||
OS: | OS X 10.5 | ||||||||||||||
Bug Depends on: | |||||||||||||||
Bug Blocks: | 38576 | ||||||||||||||
Attachments: |
|
Description
Xan Lopez
2010-05-10 07:02:50 PDT
Created attachment 55553 [details]
events.diff
First patch, expose the DOM events as GObject signals.
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.
Created attachment 55687 [details]
domevents.diff
This fixes the style issues that are possible to fix.
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.
Created attachment 55690 [details]
signalnames.diff
GObject-ify signal names for DOM events.
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.
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.
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.
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.
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-.
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
Comment on attachment 55687 [details] domevents.diff Pushed with the suggested changes in r60225. Comment on attachment 55690 [details] signalnames.diff Pushed with the suggested change in r60226. Comment on attachment 55698 [details]
testsignals.diff
This is an ex-patch.
Comment on attachment 57017 [details] eventobject.diff Pushed together with previous patch as r60229. All patches committed, closing bug. |