Bug 107630 – Implement MouseEvent constructor

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Kentaro Hara]

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.

[Kentaro Hara]

Created attachment 184141 [details]
Patch

[Kentaro Hara]

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.

[Kentaro Hara]

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

[Early Warning System Bot]

Comment on attachment 184141 [details]
Patch

Attachment 184141 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/16077012

[Early Warning System Bot]

Comment on attachment 184141 [details]
Patch

Attachment 184141 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/16067216

[Kentaro Hara]

Created attachment 184142 [details]
Patch

[Kentaro Hara]

Created attachment 184158 [details]
Fixed mac failures. Review?

[Kentaro Hara]

now bots are green:)

[Adam Barth]

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?

[Kentaro Hara]

Created attachment 184367 [details]
Patch

[Kentaro Hara]

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

[Adam Barth]

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.

[Kentaro Hara]

Created attachment 184397 [details]
patch for landing

[Kentaro Hara]

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

[WebKit Review Bot]

Comment on attachment 184397 [details]
patch for landing

Clearing flags on attachment: 184397

Committed r140657: <http://trac.webkit.org/changeset/140657>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.