Skip to content
  • zimmermann@webkit.org's avatar
    SVGLoad event fires too early · 3d2114c5
    zimmermann@webkit.org authored
    https://bugs.webkit.org/show_bug.cgi?id=78084
    
    Reviewed by Hajime Morita.
    
    Source/WebCore:
    
    SVGLoad event fires too early, making it impossible to use the vanilla repaint.js harness (runRepaintTest).
    
    We're using a hack called runSVGRepaintTest() at the moment in trunk, which runs runRepaintTest() from a 0ms timer,
    which is not reliable. The main difference between HTML onload and SVG onload is that HTMLs event is a "window event",
    thus dispatched through DOMWindow (eg. <body onload="alert(event.target)" will yield Document,
    <svg onload="alert(evt.target)"> will say SVGSVGElement).
    
    Consider:
    <svg onload="alert('1')>
    <g onload="alert('2)">
    <rect onload="alert('3')"/>
    </svg>
    
    As soon as the <rect> finishes parsing (SVGElement::finishedParsingChildren), it's SVGLoad event is fired.
    So first you'll see '3', then '2', then '1'.
    
    Using:
    <svg onload="alert('1')>
    <g onload="alert('2)">
    <image xlink:href="someExternal.jpg" onload="alert('3')"/>
    </svg>
    
    will yield the same SVGLoad order. When using <image externalREsourcesRequired="true", first the '1' will fire,
    then '3', then '2', all as expected and specified in SVG.
    
    http://www.w3.org/TR/SVG/interact.html#LoadEvent says:
    "The event is triggered at the point at which the user agent has fully parsed the element and its descendants and is
    ready to act appropriately upon that element, such as being ready to render the element to the target device. Referenced
    external resources that are required must be loaded, parsed and ready to render before the event is triggered. Optional
    external resources are not required to be ready for the event to be triggered."
    
    What we don't implement correctly is the second part of the first sentence: "and is ready to act appropriately upon that
    element, such as being ready to render the element to the target device". We currently fire the SVGLoad event, right after
    </svg> is seen, if no externalResourceRequired="true" attributes are set anywhere. This is not wrong, but not correct for
    WebKit, as we're not yet "ready to render".
    
    HTML fires its window onload event from Document::implicitClose(), where it calls Document::dispatchWindowLoadEvent.
    At this point we're ready to render. So I'm now aligning the timing of the outermost <svg> elements SVGLoad event, to be
    equal to HTML. This lets use use the repaint.js harness w/o any special SVG tricks.
    
    Covered by existing tests.
    
    * dom/Document.cpp:
    (WebCore::Document::implicitClose): Dispatch SVGLoad event for outermost <svg> elements from here, as HTML does for its window onload event.
    * svg/SVGDocumentExtensions.cpp:
    (WebCore::SVGDocumentExtensions::dispatchSVGLoadEventToOutermostSVGElements): Sends a SVGLoad event to all outermost <svg> elements in a document, if possible.
    There can be multiple ones, if using <svg><foreignObject><svg>... - the <svg> in the <fO> also acts as outermost <svg> element.
    * svg/SVGDocumentExtensions.h: Add new dispatchSVGLoadEventToOutermostSVGElements() helper.
    * svg/SVGElement.cpp:
    (WebCore::SVGElement::isOutermostSVGSVGElement): Moved from SVGSVGElement into SVGElement, and renamed from isOutermostSVG().
    (WebCore::SVGElement::sendSVGLoadEventIfPossible): Don't dispatch load events to outermost <svg> elements, if Document::implicitClose() wasn't called yet.
    (WebCore::SVGElement::finishParsingChildren): Stop using the default SVGLoad dispatching logic for outermost <svg> elements.
    * svg/SVGElement.h: Add isOutermostSVGSVGElement().
    * svg/SVGSVGElement.cpp: Rename isOutermostSVG to isOutermostSVGSVGElement.
    (WebCore::SVGSVGElement::currentScale):
    (WebCore::SVGSVGElement::setCurrentScale):
    (WebCore::SVGSVGElement::localCoordinateSpaceTransform):
    (WebCore::SVGSVGElement::createRenderer):
    * svg/SVGSVGElement.h: Move isOutermostSVG() to SVGElement.
    * svg/SVGStyledElement.cpp:
    (WebCore::SVGStyledElement::title): Rename isOutermostSVG to isOutermostSVGSVGElement.
    
    LayoutTests:
    
    Remove runSVGRepaintTest() from repaint.js again, and convert all *.svg tests to use runRepaintTest() directly.
    This is now possible as the outermost <svg> elements load event timing as aligned with HTML.
    
    * fast/repaint/resources/repaint.js: Remove runSVGRepaintTest(), it's no longer needed.
    (runRepaintTest): s/document.rootElement/document.documentElement/ to make it work for all HTML/XHTML and SVG documents (XHTML was broken).
    * platform/chromium/test_expectations.txt:
    * platform/mac/svg/custom/SVGPoint-matrixTransform-expected.png:
    * platform/mac/svg/custom/SVGPoint-matrixTransform-expected.txt:
    * platform/mac/svg/custom/getTransformToElement-expected.png:
    * platform/mac/svg/custom/getTransformToElement-expected.txt:
    * platform/mac/svg/custom/polyline-setattribute-points-null-expected.png:
    * platform/mac/svg/custom/polyline-setattribute-points-null-expected.txt:
    * platform/mac/svg/custom/text-ctm-expected.png:
    * platform/mac/svg/custom/text-ctm-expected.txt:
    * platform/mac/svg/custom/text-hit-test-expected.png:
    * platform/mac/svg/custom/text-hit-test-expected.txt:
    * platform/mac/svg/filters/filter-refresh-expected.png:
    * svg/carto.net/tabgroup.svg:
    * svg/carto.net/window.svg:
    * svg/css/shadow-changes.svg:
    * svg/custom/loadevents-externalresourcesrequired.svg:
    * svg/dom/SVGPathSegList-segment-modification.svg:
    * svg/dom/SVGPathSegList-xml-dom-synchronization2.xhtml:
    * svg/dom/SVGRectElement/rect-modify-rx.svg:
    * svg/filters/animate-fill.svg:
    * svg/filters/feImage-reference-invalidation.svg:
    * svg/filters/feImage-target-add-to-document.svg:
    * svg/filters/feImage-target-changes-id.svg:
    * svg/filters/feImage-target-id-change.svg:
    * svg/filters/feImage-target-reappend-to-document.svg:
    * svg/filters/feImage-target-remove-from-document.svg:
    * svg/filters/filter-refresh.svg:
    * svg/filters/filter-width-update.svg:
    * svg/filters/invalidate-on-child-layout.svg:
    * svg/hixie/perf/001.xml:
    * svg/hixie/perf/002.xml:
    * svg/hixie/perf/003.xml:
    * svg/hixie/perf/004.xml:
    * svg/hixie/perf/005.xml:
    * svg/hixie/perf/006.xml:
    * svg/hixie/perf/007.xml:
    * svg/repaint/container-repaint.svg:
    * svg/repaint/filter-child-repaint.svg:
    * svg/repaint/image-href-change.svg:
    * svg/repaint/image-with-clip-path.svg:
    * svg/text/text-text-05-t.svg:
    * svg/zoom/page/absolute-sized-document-no-scrollbars.svg:
    * svg/zoom/page/absolute-sized-document-scrollbars.svg:
    * svg/zoom/page/relative-sized-document-scrollbars.svg:
    * svg/zoom/page/zoom-coords-viewattr-01-b.svg:
    * svg/zoom/page/zoom-foreignObject.svg:
    * svg/zoom/page/zoom-mask-with-percentages.svg:
    * svg/zoom/resources/testPageZoom.js:
    (repaintTest):
    * svg/zoom/text/absolute-sized-document-no-scrollbars.svg:
    * svg/zoom/text/absolute-sized-document-scrollbars.svg:
    * svg/zoom/text/relative-sized-document-scrollbars.svg:
    * svg/zoom/text/zoom-coords-viewattr-01-b.svg:
    * svg/zoom/text/zoom-foreignObject.svg:
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@107057 268f45cc-cd09-0410-ab3c-d52691b4dbfc
    3d2114c5