Summary: | SVG animatable bindings are improperly being generated with "fastGetAttribute" | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brent Fulgham <bfulgham> | ||||||
Component: | WebCore Misc. | Assignee: | Brent Fulgham <bfulgham> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | bfulgham, cdumez, commit-queue, darin, haraken, rniwa, sam, simon.fraser, thorton | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 118845, 125527 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Brent Fulgham
2013-12-12 18:09:40 PST
Created attachment 219139 [details]
Patch
Comment on attachment 219139 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=219139&action=review > Source/WebCore/bindings/scripts/CodeGenerator.pm:576 > + } elsif ($generator->IsSVGAnimatedType($attributeType)) { This change looks good. > Source/WebCore/bindings/scripts/CodeGenerator.pm:578 > + } elsif (($interfaceName =~ /SVGScriptElement/) && ($attribute->signature->name =~ /type/)) { I don't understand this one. The "type" attribute on SVGScriptElement is not animatable in the specification (svg 1.1 or 2): http://www.w3.org/TR/SVG2/script.html#ScriptElementTypeAttribute Also, the attribute type in our IDL is a simple DOMString, not a SVGAnimatedString. Comment on attachment 219139 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=219139&action=review >> Source/WebCore/bindings/scripts/CodeGenerator.pm:578 >> + } elsif (($interfaceName =~ /SVGScriptElement/) && ($attribute->signature->name =~ /type/)) { > > I don't understand this one. The "type" attribute on SVGScriptElement is not animatable in the specification (svg 1.1 or 2): > http://www.w3.org/TR/SVG2/script.html#ScriptElementTypeAttribute > > Also, the attribute type in our IDL is a simple DOMString, not a SVGAnimatedString. Patch in bug 125527 corrects things so that SVGScriptElement’s type attribute is correctly identified as not animatable, so we should land that first, and omit this change. (In reply to comment #3) > (From update of attachment 219139 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=219139&action=review > > >> Source/WebCore/bindings/scripts/CodeGenerator.pm:578 > >> + } elsif (($interfaceName =~ /SVGScriptElement/) && ($attribute->signature->name =~ /type/)) { > > > > I don't understand this one. The "type" attribute on SVGScriptElement is not animatable in the specification (svg 1.1 or 2): > > http://www.w3.org/TR/SVG2/script.html#ScriptElementTypeAttribute > > > > Also, the attribute type in our IDL is a simple DOMString, not a SVGAnimatedString. > > Patch in bug 125527 corrects things so that SVGScriptElement’s type attribute is correctly identified as not animatable, so we should land that first, and omit this change. Ah! That makes great sense, I'll revise the patch and wait for 125527 to land. Thanks for looking it over. Created attachment 219151 [details] Updated to take advantage of fixes in Bug 125527. Comment on attachment 219151 [details] Updated to take advantage of fixes in Bug 125527. Clearing flags on attachment: 219151 Committed r160578: <http://trac.webkit.org/changeset/160578> All reviewed patches have been landed. Closing bug. |