Bug 81109

Summary: Make SVGUseElement respect & support externalResourcesRequired
Product: WebKit Reporter: Nikolas Zimmermann <zimmermann>
Component: SVGAssignee: Nikolas Zimmermann <zimmermann>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, webkit.review.bot, zimmermann
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 12499    
Attachments:
Description Flags
Patch rwlbuis: review+, webkit.review.bot: commit-queue-

Nikolas Zimmermann
Reported 2012-03-14 08:01:40 PDT
Make SVGUseElement respect & support externalResourcesRequired. Without externalResourcesRequired support there's no way to delay SVGLoad events until the external resource is actually needed. This currently makes 4 svg/dynamic-updates/SVGUseElement* tests flaky, that reni introduced in bug 12499. I'll have a fix for this, generalizing the existing support for that in SVGScriptElement.
Attachments
Patch (114.06 KB, patch)
2012-03-14 08:23 PDT, Nikolas Zimmermann 		rwlbuis: review+
webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Nikolas Zimmermann
Comment 1 2012-03-14 08:23:43 PDT
Created attachment 131855 [details] Patch
Rob Buis
Comment 2 2012-03-14 09:06:17 PDT
Comment on attachment 131855 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=131855&action=review Looks great, please fix my concerns before landing. > Source/WebCore/svg/SVGExternalResourcesRequired.cpp:79 > + // If we've already fired an load event and externalResourcesRequired is set to 'true' fire *a* load > Source/WebCore/svg/SVGExternalResourcesRequired.cpp:85 > + // HTML and SVG differ completly in the 'onload' event handling of <script> elements. Typo: completely. > Source/WebCore/svg/SVGExternalResourcesRequired.cpp:86 > + // HTML fires the 'load' event after it sucessfully loaded a remote resource, otherwhise an error event. Typo: otherwise. > Source/WebCore/svg/SVGExternalResourcesRequired.cpp:88 > + // is set to 'false', otherwhise it dispatches the 'SVGLoad' event just after loading the remote resource. Ditto. > Source/WebCore/svg/SVGExternalResourcesRequired.cpp:89 > + if (!externalResourcesRequired) The externalResourcesRequired logic does not make sense for isParserInserted = false and externalResourcesRequired=true, as discussed on irc. > Source/WebCore/svg/SVGExternalResourcesRequired.cpp:106 > + // Eventually send SVGLoad event now for the dynamically inserted script element Lacks period.
WebKit Review Bot
Comment 3 2012-03-14 09:43:41 PDT
Comment on attachment 131855 [details] Patch Attachment 131855 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11957084 New failing tests: svg/dynamic-updates/SVGUseElement-dom-href2-attr.html svg/dynamic-updates/SVGUseElement-svgdom-href2-prop.html
Nikolas Zimmermann
Comment 4 2012-03-15 03:22:35 PDT
(In reply to comment #3) > svg/dynamic-updates/SVGUseElement-dom-href2-attr.html > svg/dynamic-updates/SVGUseElement-svgdom-href2-prop.html Oh I thought those were still listed as failing in the expectations, someone already rebaselined them. Now they need another rebaseline. I'll talk to the gardener. Closing this bug, as the patch landed in r110711.
Note You need to log in before you can comment on or make changes to this bug.