Bug 88232

Summary: SVGElementInstance should have EventTarget on the prototype chain
Product: WebKit Reporter: Dominic Cooney <dominicc>
Component: SVGAssignee: Dominic Cooney <dominicc>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, darin, gyuyoung.kim, haraken, japhet, jochen, ojan, rakuco, vestbo, webkit.review.bot, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 93812, 93819    
Bug Blocks: 67312    
Attachments:
Description Flags
WIP Patch
none
WIP Patch
none
WIP Patch
none
WIP Patch
none
Patch
abarth: review+
To run by bots
webkit-ews: commit-queue-
To run by bots none

Description Dominic Cooney 2012-06-04 07:41:28 PDT
Per the spec here: <http://www.w3.org/TR/SVG/struct.html#InterfaceSVGElementInstance> SVGElementInstance is declared to extend EventTarget. So it should have EventTarget on the prototype chain instead of declaring addEventListener, removeEventListener and dispatchEvent itself.
Comment 1 Dominic Cooney 2012-06-04 07:57:48 PDT
Created attachment 145586 [details]
WIP Patch
Comment 2 Dominic Cooney 2012-06-04 16:11:53 PDT
Created attachment 145647 [details]
WIP Patch
Comment 3 Dominic Cooney 2012-06-05 07:42:40 PDT
Created attachment 145793 [details]
WIP Patch
Comment 4 Gyuyoung Kim 2012-06-05 10:26:44 PDT
Comment on attachment 145793 [details]
WIP Patch

Attachment 145793 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/12894661
Comment 5 Dominic Cooney 2012-06-05 13:13:25 PDT
Comment on attachment 145793 [details]
WIP Patch

EFL needs more IDL includes.
Comment 6 Dominic Cooney 2012-06-05 13:46:47 PDT
Created attachment 145861 [details]
WIP Patch
Comment 7 Gyuyoung Kim 2012-06-05 16:00:21 PDT
Comment on attachment 145861 [details]
WIP Patch

Attachment 145861 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/12902644
Comment 8 Dominic Cooney 2012-06-06 07:14:11 PDT
Created attachment 146017 [details]
Patch
Comment 9 Adam Barth 2012-06-07 17:49:35 PDT
Sorry for the delay.  Looking now.
Comment 10 Adam Barth 2012-06-07 18:12:49 PDT
Comment on attachment 146017 [details]
Patch

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

Cool!

> Source/WebCore/ChangeLog:51
> +        * svg/SVGElementInstance.h: Must extend EventTarget first so that
> +        static_cast<EventTarget*>(elementInstance) is the same pointer as
> +        elementInstance, similar to how static_cast<Node*>(element) is the
> +        same pointer as element.

Should we add an ASSERT for this?

> Source/WebCore/bindings/js/JSEventTargetCustom.cpp:76
> +    TRY_TO_UNWRAP_WITH_INTERFACE(EventTarget)

Can you add a FIXME about removing this once all the event targets are smarter?

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1339
> +    my $eventTarget = $dataNode->extendedAttributes->{"EventTarget"} || $codeGenerator->IsStrictSubtype($dataNode, "EventTarget");

Should we add a helper function for this pattern since we're using it in a bunch of places?

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:3480
> +    # TODO: When EventTarget is an interface and not a mixin, fix this so that

TODO -> FIXME

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:3609
> +    # TODO: When EventTarget is an interface and not a mixin, fix this to use

TODO -> FIXME

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:3735
> +    # TODO: When EventTarget is an interface and not a mixin, fix this so that

TODO -> FIXME
Comment 11 Dominic Cooney 2012-08-03 01:12:03 PDT
Created attachment 156281 [details]
To run by bots
Comment 12 Early Warning System Bot 2012-08-03 01:26:23 PDT
Comment on attachment 156281 [details]
To run by bots

Attachment 156281 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13430315
Comment 13 Early Warning System Bot 2012-08-03 01:37:51 PDT
Comment on attachment 156281 [details]
To run by bots

Attachment 156281 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13432117
Comment 14 Build Bot 2012-08-03 01:46:16 PDT
Comment on attachment 156281 [details]
To run by bots

Attachment 156281 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13426514
Comment 15 Dominic Cooney 2012-08-06 19:38:09 PDT
Created attachment 156836 [details]
To run by bots
Comment 16 Dominic Cooney 2012-08-09 22:13:21 PDT
Commited r125251.
Comment 17 Darin Adler 2015-01-17 19:58:36 PST
This change broke garbage collection in JavaScriptCore. See bug 132148 for details.