107430 – Implement UIEvent constructor

RESOLVED FIXED 107430

Implement UIEvent constructor

https://bugs.webkit.org/show_bug.cgi?id=107430

Summary Implement UIEvent constructor

Kentaro Hara

Reported 2013-01-21 00:58:36 PST

Editor's draft: https://dvcs.w3.org/hg/d4e/raw-file/tip/source_respec.htm UIEvent constructor is implemented under a DOM4_EVENTS_CONSTRUCTOR flag, which is enabled on Chromium only.

Attachments
Patch (24.83 KB, patch) 2013-01-21 01:03 PST, Kentaro Hara	no flags	Details Formatted Diff Diff
Patch (26.14 KB, patch) 2013-01-21 16:55 PST, Kentaro Hara	no flags	Details Formatted Diff Diff
Patch (31.55 KB, patch) 2013-01-21 22:11 PST, Kentaro Hara	no flags	Details Formatted Diff Diff
Patch (51.12 KB, patch) 2013-01-21 23:19 PST, Kentaro Hara	no flags	Details Formatted Diff Diff
Patch (52.42 KB, patch) 2013-01-22 00:12 PST, Kentaro Hara	no flags	Details Formatted Diff Diff
patch for landing (54.78 KB, patch) 2013-01-22 15:59 PST, Kentaro Hara	no flags	Details Formatted Diff Diff
patch for landing (55.20 KB, patch) 2013-01-22 16:14 PST, Kentaro Hara	webkit.review.bot: commit-queue-	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Kentaro Hara

Comment 1 2013-01-21 01:03:20 PST

Created attachment 183728 [details] Patch

Sam Weinig

Comment 2 2013-01-21 09:56:44 PST

I don't think an #ifdef is needed for such a simple change (I don't think we added one when we added Event constructors).

Kentaro Hara

Comment 3 2013-01-21 14:12:10 PST

(In reply to comment #2) > I don't think an #ifdef is needed for such a simple change (I don't think we added one when we added Event constructors). But in webkit-dev, maciej asked us to implement it under a flag because it's still on an editor's draft. (If you want, I can enable the flag in Safari too once I implemented all events.)

Kentaro Hara

Comment 4 2013-01-21 16:55:56 PST

Created attachment 183856 [details] Patch

Sam Weinig

Comment 5 2013-01-21 17:17:00 PST

(In reply to comment #3) > (In reply to comment #2) > > I don't think an #ifdef is needed for such a simple change (I don't think we added one when we added Event constructors). > > But in webkit-dev, maciej asked us to implement it under a flag because it's still on an editor's draft. (If you want, I can enable the flag in Safari too once I implemented all events.) I talked with Maciej, and rescind my objection.

Build Bot

Comment 6 2013-01-21 18:04:07 PST

Comment on attachment 183856 [details] Patch Attachment 183856 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16041280 New failing tests: fast/events/constructors/ui-event-constructor.html

Kentaro Hara

Comment 7 2013-01-21 22:11:02 PST

Created attachment 183890 [details] Patch

Build Bot

Comment 8 2013-01-21 22:57:11 PST

Comment on attachment 183890 [details] Patch Attachment 183890 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16040389 New failing tests: fast/events/constructors/ui-event-constructor.html

Kentaro Hara

Comment 9 2013-01-21 23:12:43 PST

hmm, DOM4_EVENTS_CONSTRUCTOR is not enabled on Safari... Looks like I'm missing something.

Kentaro Hara

Comment 10 2013-01-21 23:19:39 PST

Created attachment 183896 [details] Patch

Kentaro Hara

Comment 11 2013-01-22 00:12:30 PST

Created attachment 183908 [details] Patch

Adam Barth

Comment 12 2013-01-22 00:14:54 PST

Comment on attachment 183908 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183908&action=review > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:-409 > - my $conditionalString = $codeGenerator->GenerateConstructorConditionalString($interface); > - push(@headerContent, "#if $conditionalString\n") if $conditionalString; I'm surprised you're reverting these changes. > Source/WebCore/dom/UIEvent.idl:21 > + ConstructorConditional=DOM4_EVENTS_CONSTRUCTOR, Especially since you're using the attribute here...

Adam Barth

Comment 13 2013-01-22 00:15:08 PST

I must be confused. I'll look again in the morning.

Kentaro Hara

Comment 14 2013-01-22 00:22:17 PST

Comment on attachment 183908 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=183908&action=review >> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:-409 >> - push(@headerContent, "#if $conditionalString\n") if $conditionalString; > > I'm surprised you're reverting these changes. This is just a nit. Before (Compile fails if a flag is disabled): // V8XXXEvent.h #if ENABLE(...) static Handle<Value> constructorCallback(); #endif // V8XXXEvent.cpp Handle<Value> constructorCallback() { ... } static void ConfigureV8XXXEventTemplate() { ... #if ENABLE(...) desc->SetCallHandler(V8XXXEvent::constructorCallback); #endif ... } After: // V8XXXEvent.h static Handle<Value> constructorCallback(); // V8XXXEvent.cpp Handle<Value> constructorCallback() { ... } static void ConfigureV8XXXEventTemplate() { ... #if ENABLE(...) desc->SetCallHandler(V8XXXEvent::constructorCallback); #endif ... } I just didn't want to surround the definition of constructorCallback() because it slightly messes up CodeGeneratoV8.pm.

Adam Barth

Comment 15 2013-01-22 00:27:34 PST

Ok. I'll look again when less tired. :)

Build Bot

Comment 16 2013-01-22 08:31:15 PST

Comment on attachment 183908 [details] Patch Attachment 183908 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16035632 New failing tests: fast/dom/dom-constructors.html fast/dom/constructed-objects-prototypes.html

Adam Barth

Comment 17 2013-01-22 09:47:22 PST

Comment on attachment 183908 [details] Patch This looks good to me, but there seems to be some trouble with the compile flag on apple-mac.

Build Bot

Comment 18 2013-01-22 10:15:12 PST

Comment on attachment 183908 [details] Patch Attachment 183908 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16033711 New failing tests: fast/dom/dom-constructors.html fast/dom/constructed-objects-prototypes.html

Kentaro Hara

Comment 19 2013-01-22 15:59:19 PST

Created attachment 184065 [details] patch for landing

WebKit Review Bot

Comment 20 2013-01-22 16:05:02 PST

Comment on attachment 184065 [details] patch for landing Rejecting attachment 184065 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: utTests/platform/efl/TestExpectations Hunk #1 succeeded at 1232 (offset -5 lines). patching file LayoutTests/platform/gtk/TestExpectations Hunk #1 succeeded at 364 with fuzz 2 (offset -8 lines). patching file LayoutTests/platform/qt/TestExpectations patching file LayoutTests/platform/win/TestExpectations patching file LayoutTests/platform/wincairo/TestExpectations Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output: http://queues.webkit.org/results/16063265

Kentaro Hara

Comment 21 2013-01-22 16:14:25 PST

Created attachment 184067 [details] patch for landing

WebKit Review Bot

Comment 22 2013-01-22 17:02:05 PST

Comment on attachment 184067 [details] patch for landing Rejecting attachment 184067 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: um/third_party/v8-i18n --revision 159 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 54>At revision 159. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Total errors found: 0 in 5 files Full output: http://queues.webkit.org/results/16040894

WebKit Review Bot

Comment 23 2013-01-22 17:42:32 PST

Comment on attachment 184067 [details] patch for landing Rejecting attachment 184067 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: WebKit/chromium/v8 --revision 13451 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 53>At revision 13451. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Total errors found: 0 in 5 files Full output: http://queues.webkit.org/results/16038869

Kentaro Hara

Comment 24 2013-01-22 17:51:34 PST

Committed r140493: <http://trac.webkit.org/changeset/140493>

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware Unspecified

OS Unspecified

Product WebKit

Component WebCore JavaScript

Assignee

Kentaro Hara

Reported

2013-01-21 00:58 PST

Modified

2013-01-22 17:51 PST History

CC List

11 users Show

URL

Keywords

Depends on

Blocks

67824

Dependencies

tree graph