Bug 107430

Created attachment 183728 [details] Patch

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).

(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.)

Created attachment 183856 [details] Patch

(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.

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

Created attachment 183890 [details] Patch

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

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

Created attachment 183896 [details] Patch

Created attachment 183908 [details] Patch

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...

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

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.

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

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

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.

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

Created attachment 184065 [details] patch for landing

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

Created attachment 184067 [details] patch for landing

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

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

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