Summary: | Implement MouseEvent constructor | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kentaro Hara <haraken> | ||||||||||||
Component: | WebCore JavaScript | Assignee: | Kentaro Hara <haraken> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | abarth, eric.carlson, feature-media-reviews, gyuyoung.kim, japhet, ojan.autocc, rakuco, sam, syoichi, webkit-ews, webkit.review.bot | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Bug Depends on: | |||||||||||||||
Bug Blocks: | 67824 | ||||||||||||||
Attachments: |
|
Description
Kentaro Hara
2013-01-22 22:06:08 PST
Created attachment 184141 [details]
Patch
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. (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 on attachment 184141 [details] Patch Attachment 184141 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/16077012 Comment on attachment 184141 [details] Patch Attachment 184141 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/16067216 Created attachment 184142 [details]
Patch
Created attachment 184158 [details]
Fixed mac failures. Review?
now bots are green:) 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? Created attachment 184367 [details]
Patch
(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 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. Created attachment 184397 [details]
patch for landing
(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 on attachment 184397 [details] patch for landing Clearing flags on attachment: 184397 Committed r140657: <http://trac.webkit.org/changeset/140657> All reviewed patches have been landed. Closing bug. |