Bug 81109 – Make SVGUseElement respect & support externalResourcesRequired

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Nikolas Zimmermann]

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.

[Nikolas Zimmermann]

Created attachment 131855 [details]
Patch

[Rob Buis]

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 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]

(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.