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

Dominic Cooney
Reported 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.
Attachments
WIP Patch (9.26 KB, patch)
2012-06-04 07:57 PDT, Dominic Cooney 		no flags
DetailsFormatted DiffDiff
WIP Patch (10.81 KB, patch)
2012-06-04 16:11 PDT, Dominic Cooney 		no flags
DetailsFormatted DiffDiff
WIP Patch (15.49 KB, patch)
2012-06-05 07:42 PDT, Dominic Cooney 		no flags
DetailsFormatted DiffDiff
WIP Patch (16.08 KB, patch)
2012-06-05 13:46 PDT, Dominic Cooney 		no flags
DetailsFormatted DiffDiff
Patch (16.92 KB, patch)
2012-06-06 07:14 PDT, Dominic Cooney 		abarth: review+
DetailsFormatted DiffDiff
To run by bots (20.91 KB, patch)
2012-08-03 01:12 PDT, Dominic Cooney 		webkit-ews: commit-queue-
DetailsFormatted DiffDiff
To run by bots (22.98 KB, patch)
2012-08-06 19:38 PDT, Dominic Cooney 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Dominic Cooney
Comment 1 2012-06-04 07:57:48 PDT
Created attachment 145586 [details] WIP Patch
Dominic Cooney
Comment 2 2012-06-04 16:11:53 PDT
Created attachment 145647 [details] WIP Patch
Dominic Cooney
Comment 3 2012-06-05 07:42:40 PDT
Created attachment 145793 [details] WIP Patch
Gyuyoung Kim
Comment 4 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
Dominic Cooney
Comment 5 2012-06-05 13:13:25 PDT
Comment on attachment 145793 [details] WIP Patch EFL needs more IDL includes.
Dominic Cooney
Comment 6 2012-06-05 13:46:47 PDT
Created attachment 145861 [details] WIP Patch
Gyuyoung Kim
Comment 7 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
Dominic Cooney
Comment 8 2012-06-06 07:14:11 PDT
Created attachment 146017 [details] Patch
Adam Barth
Comment 9 2012-06-07 17:49:35 PDT
Sorry for the delay. Looking now.
Adam Barth
Comment 10 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
Dominic Cooney
Comment 11 2012-08-03 01:12:03 PDT
Created attachment 156281 [details] To run by bots
Early Warning System Bot
Comment 12 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
Early Warning System Bot
Comment 13 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
Build Bot
Comment 14 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
Dominic Cooney
Comment 15 2012-08-06 19:38:09 PDT
Created attachment 156836 [details] To run by bots
Dominic Cooney
Comment 16 2012-08-09 22:13:21 PDT
Commited r125251.
Darin Adler
Comment 17 2015-01-17 19:58:36 PST
This change broke garbage collection in JavaScriptCore. See bug 132148 for details.
Note You need to log in before you can comment on or make changes to this bug.