SVGPathSegList.insertItemBefore() should fail if the newItem belongs to an animating...
authorsaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Mar 2019 23:12:51 +0000 (23:12 +0000)
committersaid@apple.com <said@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Mar 2019 23:12:51 +0000 (23:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195333
<rdar://problem/48475802>

Reviewed by Simon Fraser.

Source/WebCore:

Because the SVG1.1 specs states that the newItem should be removed from
its original list before adding it to another list,
SVGPathSegList.insertItemBefore() should fail if the new item belongs to
an animating animPathSegList since it is read-only.

Test: svg/dom/SVGPathSegList-insert-from-animating-animPathSegList.svg

* svg/SVGPathSegList.cpp:
(WebCore::SVGPathSegList::processIncomingListItemValue):

LayoutTests:

* svg/dom/SVGPathSegList-insert-from-animating-animPathSegList-expected.txt: Added.
* svg/dom/SVGPathSegList-insert-from-animating-animPathSegList.svg: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242515 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/svg/dom/SVGPathSegList-insert-from-animating-animPathSegList-expected.txt [new file with mode: 0644]
LayoutTests/svg/dom/SVGPathSegList-insert-from-animating-animPathSegList.svg [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/svg/SVGPathSegList.cpp

index 3b423ee..3273284 100644 (file)
@@ -1,3 +1,14 @@
+2019-03-05  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        SVGPathSegList.insertItemBefore() should fail if the newItem belongs to an animating animPathSegList
+        https://bugs.webkit.org/show_bug.cgi?id=195333
+        <rdar://problem/48475802>
+
+        Reviewed by Simon Fraser.
+
+        * svg/dom/SVGPathSegList-insert-from-animating-animPathSegList-expected.txt: Added.
+        * svg/dom/SVGPathSegList-insert-from-animating-animPathSegList.svg: Added.
+
 2019-03-05  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r242403.
diff --git a/LayoutTests/svg/dom/SVGPathSegList-insert-from-animating-animPathSegList-expected.txt b/LayoutTests/svg/dom/SVGPathSegList-insert-from-animating-animPathSegList-expected.txt
new file mode 100644 (file)
index 0000000..db54f79
--- /dev/null
@@ -0,0 +1 @@
+PASS: did not assert in debug.
diff --git a/LayoutTests/svg/dom/SVGPathSegList-insert-from-animating-animPathSegList.svg b/LayoutTests/svg/dom/SVGPathSegList-insert-from-animating-animPathSegList.svg
new file mode 100644 (file)
index 0000000..03bdcf8
--- /dev/null
@@ -0,0 +1,29 @@
+<svg id="svg" xmlns="http://www.w3.org/2000/svg">
+    <path id="path1"></path>
+    <path id="path2" d="M 1 1 S 2,2 3,3">
+        <animate id="animate" attributeName="d" values="M 1 1" min="1s" max="2s"/>
+    </path>
+    <text>PASS: did not assert in debug.</text>
+    <script>
+        if (window.testRunner) {
+            testRunner.dumpAsText();
+            testRunner.waitUntilDone();
+        }
+
+        var animate = document.getElementById("animate");
+        animate.addEventListener('beginEvent' , function () {
+            var path1 = document.getElementById("path1");
+            var path2 = document.getElementById("path2");
+
+            var path1_pathSegList = path1.pathSegList;
+            var path2_animPathSegList = path2.animatedPathSegList;  
+
+            document.documentElement.setCurrentTime(1);
+
+            var pathseg = path2_animPathSegList.getItem(0); 
+            path1_pathSegList.insertItemBefore(pathseg, 0);
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }, { once: true});
+    </script>
+</svg>
index edbf5e4..804c02d 100644 (file)
@@ -1,3 +1,21 @@
+2019-03-05  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        SVGPathSegList.insertItemBefore() should fail if the newItem belongs to an animating animPathSegList
+        https://bugs.webkit.org/show_bug.cgi?id=195333
+        <rdar://problem/48475802>
+
+        Reviewed by Simon Fraser.
+
+        Because the SVG1.1 specs states that the newItem should be removed from
+        its original list before adding it to another list,
+        SVGPathSegList.insertItemBefore() should fail if the new item belongs to
+        an animating animPathSegList since it is read-only.
+
+        Test: svg/dom/SVGPathSegList-insert-from-animating-animPathSegList.svg
+
+        * svg/SVGPathSegList.cpp:
+        (WebCore::SVGPathSegList::processIncomingListItemValue):
+
 2019-03-05  Zalan Bujtas  <zalan@apple.com>
 
         [ContentChangeObserver] Send content change notification through adjustObservedState
index 6a0ce90..8237ad4 100644 (file)
@@ -85,7 +85,14 @@ bool SVGPathSegList::processIncomingListItemValue(const ListItemType& newItem, u
     bool livesInOtherList = animatedPropertyOfItem != m_animatedProperty;
     RefPtr<SVGAnimatedPathSegListPropertyTearOff> propertyTearOff = static_pointer_cast<SVGAnimatedPathSegListPropertyTearOff>(animatedPropertyOfItem);
     int indexToRemove = propertyTearOff->findItem(newItem.get());
-    ASSERT(indexToRemove != -1);
+
+    // If newItem does not exist in the propertyTearOff baseVal() list, it has to be
+    // in its animVal() list and it has to be animating.
+    if (indexToRemove == -1) {
+        ASSERT(propertyTearOff->isAnimating());
+        ASSERT(propertyTearOff->animVal()->findItem(newItem.get()) != -1);
+        return false;
+    }
 
     // Do not remove newItem if already in this list at the target index.
     if (!livesInOtherList && indexToModify && static_cast<unsigned>(indexToRemove) == *indexToModify)