Bug 107630 - Implement MouseEvent constructor
Summary: Implement MouseEvent constructor
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on:
Blocks: 67824
  Show dependency treegraph
 
Reported: 2013-01-22 22:06 PST by Kentaro Hara
Modified: 2013-01-23 23:43 PST (History)
11 users (show)

See Also:


Attachments
Patch (52.45 KB, patch)
2013-01-22 22:26 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff
Patch (51.04 KB, patch)
2013-01-22 22:35 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff
Fixed mac failures. Review? (50.87 KB, patch)
2013-01-22 23:41 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff
Patch (84.37 KB, patch)
2013-01-23 17:50 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff
patch for landing (84.29 KB, patch)
2013-01-23 21:31 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2013-01-22 22:06:08 PST
Spec: https://dvcs.w3.org/hg/d4e/raw-file/tip/source_respec.htm

The MouseEvent constructor should be implemented under a DOM4_EVENTS_CONSTRUCTOR flag.
Comment 1 Kentaro Hara 2013-01-22 22:26:31 PST
Created attachment 184141 [details]
Patch
Comment 2 Kentaro Hara 2013-01-22 22:27:34 PST
Comment on attachment 184141 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184141&action=review

> LayoutTests/fast/events/constructors/mouse-event-constructor-expected.txt:96
> +FAIL new MouseEvent('eventType', { clientX: 2147483647 }).clientX should be 2147483647. Was -1.
> +PASS new MouseEvent('eventType', { clientX: -1 }).clientX is -1
> +FAIL new MouseEvent('eventType', { clientX: -2147483648 }).clientX should be -2147483648. Was 0.

These two failures are binding issues. I'll file a bug.

> LayoutTests/fast/events/constructors/mouse-event-constructor-expected.txt:117
> +FAIL new MouseEvent('eventType', { clientY: 2147483647 }).clientY should be 2147483647. Was -1.
> +PASS new MouseEvent('eventType', { clientY: -1 }).clientY is -1
> +FAIL new MouseEvent('eventType', { clientY: -2147483648 }).clientY should be -2147483648. Was 0.

Ditto.
Comment 3 Kentaro Hara 2013-01-22 22:29:06 PST
(In reply to comment #2)
> > LayoutTests/fast/events/constructors/mouse-event-constructor-expected.txt:96
> > +FAIL new MouseEvent('eventType', { clientX: 2147483647 }).clientX should be 2147483647. Was -1.
> > +PASS new MouseEvent('eventType', { clientX: -1 }).clientX is -1
> > +FAIL new MouseEvent('eventType', { clientX: -2147483648 }).clientX should be -2147483648. Was 0.
> 
> These two failures are binding issues. I'll file a bug.

s/are/are not/.
Comment 4 Early Warning System Bot 2013-01-22 22:33:50 PST
Comment on attachment 184141 [details]
Patch

Attachment 184141 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/16077012
Comment 5 Early Warning System Bot 2013-01-22 22:34:55 PST
Comment on attachment 184141 [details]
Patch

Attachment 184141 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16067216
Comment 6 Kentaro Hara 2013-01-22 22:35:38 PST
Created attachment 184142 [details]
Patch
Comment 7 Kentaro Hara 2013-01-22 23:41:52 PST
Created attachment 184158 [details]
Fixed mac failures. Review?
Comment 8 Kentaro Hara 2013-01-23 01:28:49 PST
now bots are green:)
Comment 9 Adam Barth 2013-01-23 11:38:45 PST
Comment on attachment 184158 [details]
Fixed mac failures. Review?

View in context: https://bugs.webkit.org/attachment.cgi?id=184158&action=review

> Source/WebCore/bindings/v8/Dictionary.cpp:473
> +        // FIXME: When EventTarget is an interface and not a mixin, fix V8Node to V8EventTarget.
> +        v8::Handle<v8::Object> error = wrapper->FindInstanceInPrototypeChain(V8Node::GetTemplate());

So this only works with Nodes?  Maybe we need a toEventTarget function in WrapperTypeInfo similar to the toActiveDOMObject function.

> Source/WebCore/dom/MouseEvent.idl:35
> +    [Conditional=POINTER_LOCK]      readonly attribute long             webkitMovementX;
> +    [Conditional=POINTER_LOCK]      readonly attribute long             webkitMovementY;

These don't get InitializedByEventConstructor ?  Nor do the "extensions" below?
Comment 10 Kentaro Hara 2013-01-23 17:50:14 PST
Created attachment 184367 [details]
Patch
Comment 11 Kentaro Hara 2013-01-23 17:54:39 PST
(In reply to comment #9)
> > Source/WebCore/bindings/v8/Dictionary.cpp:473
> > +        // FIXME: When EventTarget is an interface and not a mixin, fix V8Node to V8EventTarget.
> > +        v8::Handle<v8::Object> error = wrapper->FindInstanceInPrototypeChain(V8Node::GetTemplate());
> 
> So this only works with Nodes?  Maybe we need a toEventTarget function in WrapperTypeInfo similar to the toActiveDOMObject function.

Done. Implemented WrapperTypeInfo::toEventTarget(). If you want to split the patch, I'd be happy to.

Added a test case for this:

  shouldBe("new MouseEvent('eventType', { relatedTarget: xhr }).relatedTarget", "xhr");


> > Source/WebCore/dom/MouseEvent.idl:35
> > +    [Conditional=POINTER_LOCK]      readonly attribute long             webkitMovementX;
> > +    [Conditional=POINTER_LOCK]      readonly attribute long             webkitMovementY;
> 
> These don't get InitializedByEventConstructor ?  Nor do the "extensions" below?

I'm not sure if we should do that. They are not yet speced. In particular, for 'extensions' attributes, there is no straightforward way to initialize WebCore values from constructor.
Comment 12 Adam Barth 2013-01-23 21:27:59 PST
Comment on attachment 184367 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184367&action=review

> Source/WebCore/bindings/scripts/CodeGenerator.pm:735
> +# FIXME: Rename to IsInheritInterface

IsInheritedInterface ?  I guess we can work out the proper name when we do the renaming.

> Source/WebCore/bindings/scripts/CodeGenerator.pm:755
> +sub IsInheritExtendedAttribute

IsInheritedExtendedAttribute ?  DoesInheritExtendedAttribute?  InheritsExtendedAttribute ?

> Source/WebCore/bindings/v8/Dictionary.cpp:470
> +        fprintf(stderr, "V8DOMWrapper::isDOMWrapper(v8Value)\n");

We should probably remove this line before landing.
Comment 13 Kentaro Hara 2013-01-23 21:31:37 PST
Created attachment 184397 [details]
patch for landing
Comment 14 Kentaro Hara 2013-01-23 21:31:59 PST
(In reply to comment #12)
> (From update of attachment 184367 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=184367&action=review
> 
> > Source/WebCore/bindings/scripts/CodeGenerator.pm:735
> > +# FIXME: Rename to IsInheritInterface
> 
> IsInheritedInterface ?  I guess we can work out the proper name when we do the renaming.
> 
> > Source/WebCore/bindings/scripts/CodeGenerator.pm:755
> > +sub IsInheritExtendedAttribute
> 
> IsInheritedExtendedAttribute ?  DoesInheritExtendedAttribute?  InheritsExtendedAttribute ?
> 
> > Source/WebCore/bindings/v8/Dictionary.cpp:470
> > +        fprintf(stderr, "V8DOMWrapper::isDOMWrapper(v8Value)\n");
> 
> We should probably remove this line before landing.

All fixed. Thanks!
Comment 15 WebKit Review Bot 2013-01-23 23:43:37 PST
Comment on attachment 184397 [details]
patch for landing

Clearing flags on attachment: 184397

Committed r140657: <http://trac.webkit.org/changeset/140657>
Comment 16 WebKit Review Bot 2013-01-23 23:43:44 PST
All reviewed patches have been landed.  Closing bug.