Title: [257897] trunk
Revision
257897
Author
commit-qu...@webkit.org
Date
2020-03-04 19:33:22 -0800 (Wed, 04 Mar 2020)

Log Message

Crash in SVGElement::removeEventListener with symbol element
https://bugs.webkit.org/show_bug.cgi?id=207920

Patch by Doug Kelly <do...@apple.com> on 2020-03-04
Reviewed by Ryosuke Niwa.

Source/WebCore:

Resolves a crash in SVGElement::removeEventListener by only attaching the events to the window if the SVG element is both the outermost
SVG element, in addition to ensuring the SVG element is attached to the tree.  The symbol element's behavior when referenced by a use
tag actually creates an svg tag instead, so the SVGSVGElement's special behavior for copying attributes is vital.

Note that Chrome and Firefox have a similar behavior for detached SVG elements as to what this change creates: in both other browsers,
onerror is not fired for a detached svg element, and in Firefox, onresize is not fired for a detached svg element (it is however fired
in Chrome).

Tests: fast/events/detached-svg-parent-window-events.html
       fast/events/onerror-svg-symbol.html

* svg/SVGSVGElement.cpp:
(WebCore::SVGSVGElement::parseAttribute):

LayoutTests:

* fast/events/detached-svg-parent-window-events-expected.txt: Added.
* fast/events/detached-svg-parent-window-events.html: Added.
* fast/events/onerror-svg-symbol-expected.txt: Added.
* fast/events/onerror-svg-symbol.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (257896 => 257897)


--- trunk/LayoutTests/ChangeLog	2020-03-05 03:28:11 UTC (rev 257896)
+++ trunk/LayoutTests/ChangeLog	2020-03-05 03:33:22 UTC (rev 257897)
@@ -1,3 +1,15 @@
+2020-03-04  Doug Kelly  <do...@apple.com>
+
+        Crash in SVGElement::removeEventListener with symbol element
+        https://bugs.webkit.org/show_bug.cgi?id=207920
+
+        Reviewed by Ryosuke Niwa.
+
+        * fast/events/detached-svg-parent-window-events-expected.txt: Added.
+        * fast/events/detached-svg-parent-window-events.html: Added.
+        * fast/events/onerror-svg-symbol-expected.txt: Added.
+        * fast/events/onerror-svg-symbol.html: Added.
+
 2020-03-04  Zalan Bujtas  <za...@apple.com>
 
         [ Mac wk2 Debug ] fast/layoutformattingcontext/block-only/replaced-intrinsic-width-simple.html is crashing.

Added: trunk/LayoutTests/fast/events/detached-svg-parent-window-events-expected.txt (0 => 257897)


--- trunk/LayoutTests/fast/events/detached-svg-parent-window-events-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/events/detached-svg-parent-window-events-expected.txt	2020-03-05 03:33:22 UTC (rev 257897)
@@ -0,0 +1,12 @@
+CONSOLE MESSAGE: line 1: error
+This tests creating a disconnected SVG element with resize event handler. The event handler should not get dispatched unless the element is connected
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS didFireResize is false
+PASS didFireOnError is false
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/events/detached-svg-parent-window-events.html (0 => 257897)


--- trunk/LayoutTests/fast/events/detached-svg-parent-window-events.html	                        (rev 0)
+++ trunk/LayoutTests/fast/events/detached-svg-parent-window-events.html	2020-03-05 03:33:22 UTC (rev 257897)
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<html>
+    <body>
+        <script src=""
+        <script>
+            description('This tests creating a disconnected SVG element with resize event handler. The event handler should not get dispatched unless the element is connected');
+            const iframe = document.createElement('iframe');
+            iframe.style.width = '100px';
+            iframe.style.height = '100px';
+            jsTestIsAsync = true;
+            didFireResize = false;
+            didFireOnError = false;
+            iframe._onload_ = function() {
+                iframe.contentWindow.requestAnimationFrame(() => {
+                    const svg = iframe.contentDocument.createElementNS('http://www.w3.org/2000/svg', 'svg');
+                    svg.setAttribute('onresize', 'top.didFireResize = true');
+                    svg.setAttribute('onerror', 'top.didFireOnError = true');
+                    iframe.style.width = '200px';
+                    iframe.contentWindow.requestAnimationFrame(() => {
+                        shouldBeFalse('didFireResize');
+                        shouldBeFalse('didFireOnError');
+                        finishJSTest();
+                    });
+                    iframe.contentWindow.eval('throw "error"');
+                });
+            };
+            document.body.appendChild(iframe);
+        </script>
+    </body>
+</html>

Added: trunk/LayoutTests/fast/events/onerror-svg-symbol-expected.txt (0 => 257897)


--- trunk/LayoutTests/fast/events/onerror-svg-symbol-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/events/onerror-svg-symbol-expected.txt	2020-03-05 03:33:22 UTC (rev 257897)
@@ -0,0 +1 @@
+Tests a symbol element with onError event being referenced by a use element. Test passes if WebKit does not crash. PASS

Added: trunk/LayoutTests/fast/events/onerror-svg-symbol.html (0 => 257897)


--- trunk/LayoutTests/fast/events/onerror-svg-symbol.html	                        (rev 0)
+++ trunk/LayoutTests/fast/events/onerror-svg-symbol.html	2020-03-05 03:33:22 UTC (rev 257897)
@@ -0,0 +1,12 @@
+<script>
+function load() {
+    a._onerror_ = undefined;
+    document.body.textContent = "Tests a symbol element with onError event being referenced by a use element.  Test passes if WebKit does not crash.  PASS";
+    if (window.testRunner)
+        testRunner.dumpAsText();
+}
+</script>
+<body _onload_=load()>
+<svg>
+<use xlink:href=""
+<symbol id="a" _onerror_="eventhandler5()" />

Modified: trunk/Source/WebCore/ChangeLog (257896 => 257897)


--- trunk/Source/WebCore/ChangeLog	2020-03-05 03:28:11 UTC (rev 257896)
+++ trunk/Source/WebCore/ChangeLog	2020-03-05 03:33:22 UTC (rev 257897)
@@ -1,3 +1,24 @@
+2020-03-04  Doug Kelly  <do...@apple.com>
+
+        Crash in SVGElement::removeEventListener with symbol element
+        https://bugs.webkit.org/show_bug.cgi?id=207920
+
+        Reviewed by Ryosuke Niwa.
+
+        Resolves a crash in SVGElement::removeEventListener by only attaching the events to the window if the SVG element is both the outermost
+        SVG element, in addition to ensuring the SVG element is attached to the tree.  The symbol element's behavior when referenced by a use
+        tag actually creates an svg tag instead, so the SVGSVGElement's special behavior for copying attributes is vital.
+
+        Note that Chrome and Firefox have a similar behavior for detached SVG elements as to what this change creates: in both other browsers,
+        onerror is not fired for a detached svg element, and in Firefox, onresize is not fired for a detached svg element (it is however fired
+        in Chrome).
+
+        Tests: fast/events/detached-svg-parent-window-events.html
+               fast/events/onerror-svg-symbol.html
+
+        * svg/SVGSVGElement.cpp:
+        (WebCore::SVGSVGElement::parseAttribute):
+
 2020-03-04  Andres Gonzalez  <andresg...@apple.com>
 
         Fix for crash in AXIsolatedObject::fillChildrenVectorForProperty.

Modified: trunk/Source/WebCore/svg/SVGSVGElement.cpp (257896 => 257897)


--- trunk/Source/WebCore/svg/SVGSVGElement.cpp	2020-03-05 03:28:11 UTC (rev 257896)
+++ trunk/Source/WebCore/svg/SVGSVGElement.cpp	2020-03-05 03:33:22 UTC (rev 257897)
@@ -143,7 +143,7 @@
 
 void SVGSVGElement::parseAttribute(const QualifiedName& name, const AtomString& value)
 {
-    if (!nearestViewportElement()) {
+    if (!nearestViewportElement() && isConnected()) {
         // For these events, the outermost <svg> element works like a <body> element does,
         // setting certain event handlers directly on the window object.
         if (name == HTMLNames::onunloadAttr) {
@@ -162,20 +162,16 @@
             document().setWindowAttributeEventListener(eventNames().zoomEvent, name, value, mainThreadNormalWorld());
             return;
         }
+        if (name == HTMLNames::onabortAttr) {
+            document().setWindowAttributeEventListener(eventNames().abortEvent, name, value, mainThreadNormalWorld());
+            return;
+        }
+        if (name == HTMLNames::onerrorAttr) {
+            document().setWindowAttributeEventListener(eventNames().errorEvent, name, value, mainThreadNormalWorld());
+            return;
+        }
     }
 
-    // For these events, any <svg> element works like a <body> element does,
-    // setting certain event handlers directly on the window object.
-    // FIXME: Why different from the events above that work only on the outermost <svg> element?
-    if (name == HTMLNames::onabortAttr) {
-        document().setWindowAttributeEventListener(eventNames().abortEvent, name, value, mainThreadNormalWorld());
-        return;
-    }
-    if (name == HTMLNames::onerrorAttr) {
-        document().setWindowAttributeEventListener(eventNames().errorEvent, name, value, mainThreadNormalWorld());
-        return;
-    }
-
     SVGParsingError parseError = NoError;
 
     if (name == SVGNames::xAttr)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to