SVGElement should detach itself from all its properties before it is deleted
https://bugs.webkit.org/show_bug.cgi?id=197954
Reviewed by Simon Fraser.
Source/WebCore:
Before deleting the SVGElement node, SVGElement::detachAllProperties()
needs to be called. This will make the properties be detached objects
which means no change will be committed unless these properties are
attached to another owner.
Test: svg/dom/svg-properties-detach-change.html
* dom/Node.cpp:
(WebCore::Node::removedLastRef):
* svg/SVGElement.h:
(WebCore::SVGElement::detachAllProperties):
LayoutTests:
* svg/dom/svg-properties-detach-change-expected.txt: Added.
* svg/dom/svg-properties-detach-change.html : Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@245469 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index bc65398..26a7c71 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2019-05-16 Said Abou-Hallawa <sabouhallawa@apple.com>
+
+ SVGElement should detach itself from all its properties before it is deleted
+ https://bugs.webkit.org/show_bug.cgi?id=197954
+
+ Reviewed by Simon Fraser.
+
+ * svg/dom/svg-properties-detach-change-expected.txt: Added.
+ * svg/dom/svg-properties-detach-change.html : Added.
+
2019-05-17 Eric Carlson <eric.carlson@apple.com>
Allow sequential playback of media files when initial playback started with a user gesture
diff --git a/LayoutTests/svg/dom/svg-properties-detach-change-expected.txt b/LayoutTests/svg/dom/svg-properties-detach-change-expected.txt
new file mode 100644
index 0000000..ae626f2
--- /dev/null
+++ b/LayoutTests/svg/dom/svg-properties-detach-change-expected.txt
@@ -0,0 +1,38 @@
+This test verifying changing the values of detached properties - utilizing the properties of SVGRectElement and SVGTextElement
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Check the SVGLength properties
+PASS x.value is 10
+PASS y.value is 20
+PASS width.value is 100
+PASS height.value is 200
+
+Check the items of the SVGLengthList properties
+PASS xList.getItem(0).value is 10
+PASS yList.getItem(0).value is 20
+PASS dxList.getItem(0).value is 100
+PASS dyList.getItem(0).value is 200
+
+Check the items of the SVGLengthList properties of the SVGTextElement
+PASS text.x.baseVal.getItem(0).value is 100
+PASS text.y.baseVal.getItem(0).value is 200
+PASS text.dx.baseVal.getItem(0).value is 1000
+PASS text.dy.baseVal.getItem(0).value is 2000
+
+Check the SVGLength properties after removing the SVGTextElement
+PASS x.value is 100
+PASS y.value is 200
+PASS width.value is 1000
+PASS height.value is 2000
+
+Check the SVGLength properties after resetting their values
+PASS x.value is 10
+PASS y.value is 20
+PASS width.value is 100
+PASS height.value is 200
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/svg/dom/svg-properties-detach-change.html b/LayoutTests/svg/dom/svg-properties-detach-change.html
new file mode 100644
index 0000000..efaaeba
--- /dev/null
+++ b/LayoutTests/svg/dom/svg-properties-detach-change.html
@@ -0,0 +1,94 @@
+<html>
+<head>
+<script src="../../resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script>
+description("This test verifying changing the values of detached properties - utilizing the properties of SVGRectElement and SVGTextElement");
+
+var x = document.createElementNS("http://www.w3.org/2000/svg", "rect").x.baseVal;
+var y = document.createElementNS("http://www.w3.org/2000/svg", "rect").y.baseVal;
+var width = document.createElementNS("http://www.w3.org/2000/svg", "rect").width.baseVal;
+var height = document.createElementNS("http://www.w3.org/2000/svg", "rect").height.baseVal;
+
+var xList = document.createElementNS("http://www.w3.org/2000/svg", "text").x.baseVal;
+var yList = document.createElementNS("http://www.w3.org/2000/svg", "text").y.baseVal;
+var dxList = document.createElementNS("http://www.w3.org/2000/svg", "text").dx.baseVal;
+var dyList = document.createElementNS("http://www.w3.org/2000/svg", "text").dy.baseVal;
+
+var text = document.createElementNS("http://www.w3.org/2000/svg", "text");
+
+x.newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 10);
+y.newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 20);
+width.newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 100);
+height.newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 200);
+
+xList.initialize(x);
+yList.initialize(y);
+dxList.initialize(width);
+dyList.initialize(height);
+
+text.x.baseVal.initialize(x);
+text.y.baseVal.initialize(y);
+text.dx.baseVal.initialize(width);
+text.dy.baseVal.initialize(height);
+
+text.x.baseVal.getItem(0).newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 100);
+text.y.baseVal.getItem(0).newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 200);
+text.dx.baseVal.getItem(0).newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 1000);
+text.dy.baseVal.getItem(0).newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 2000);
+
+debug("Check the SVGLength properties");
+shouldBe("x.value", "10");
+shouldBe("y.value", "20");
+shouldBe("width.value", "100");
+shouldBe("height.value", "200");
+
+debug("");
+debug("Check the items of the SVGLengthList properties");
+shouldBe("xList.getItem(0).value", "10");
+shouldBe("yList.getItem(0).value", "20");
+shouldBe("dxList.getItem(0).value", "100");
+shouldBe("dyList.getItem(0).value", "200");
+
+debug("");
+debug("Check the items of the SVGLengthList properties of the SVGTextElement");
+shouldBe("text.x.baseVal.getItem(0).value", "100");
+shouldBe("text.y.baseVal.getItem(0).value", "200");
+shouldBe("text.dx.baseVal.getItem(0).value", "1000");
+shouldBe("text.dy.baseVal.getItem(0).value", "2000");
+
+x = text.x.baseVal.getItem(0);
+y = text.y.baseVal.getItem(0);
+width = text.dx.baseVal.getItem(0);
+height = text.dy.baseVal.getItem(0);
+
+text.remove();
+text = null;
+
+debug("");
+debug("Check the SVGLength properties after removing the SVGTextElement");
+shouldBe("x.value", "100");
+shouldBe("y.value", "200");
+shouldBe("width.value", "1000");
+shouldBe("height.value", "2000");
+
+x.newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 10);
+y.newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 20);
+width.newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 100);
+height.newValueSpecifiedUnits(SVGLength.SVG_LENGTHTYPE_NUMBER, 200);
+
+debug("");
+debug("Check the SVGLength properties after resetting their values");
+shouldBe("x.value", "10");
+shouldBe("y.value", "20");
+shouldBe("width.value", "100");
+shouldBe("height.value", "200");
+
+successfullyParsed = true;
+</script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index dbdffce..fef9a73 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,22 @@
+2019-05-16 Said Abou-Hallawa <sabouhallawa@apple.com>
+
+ SVGElement should detach itself from all its properties before it is deleted
+ https://bugs.webkit.org/show_bug.cgi?id=197954
+
+ Reviewed by Simon Fraser.
+
+ Before deleting the SVGElement node, SVGElement::detachAllProperties()
+ needs to be called. This will make the properties be detached objects
+ which means no change will be committed unless these properties are
+ attached to another owner.
+
+ Test: svg/dom/svg-properties-detach-change.html
+
+ * dom/Node.cpp:
+ (WebCore::Node::removedLastRef):
+ * svg/SVGElement.h:
+ (WebCore::SVGElement::detachAllProperties):
+
2019-05-17 Eric Carlson <eric.carlson@apple.com>
Allow sequential playback of media files when initial playback started with a user gesture
diff --git a/Source/WebCore/dom/Node.cpp b/Source/WebCore/dom/Node.cpp
index d6f434b..15041e9 100644
--- a/Source/WebCore/dom/Node.cpp
+++ b/Source/WebCore/dom/Node.cpp
@@ -62,6 +62,7 @@
#include "RenderBox.h"
#include "RenderTextControl.h"
#include "RenderView.h"
+#include "SVGElement.h"
#include "ScopedEventQueue.h"
#include "ScriptDisallowedScope.h"
#include "StorageEvent.h"
@@ -2509,6 +2510,12 @@
return;
}
+ // Now it is time to detach the SVGElement from all its properties. These properties
+ // may outlive the SVGElement. The only difference after the detach is no commit will
+ // be carried out unless these properties are attached to another owner.
+ if (is<SVGElement>(*this))
+ downcast<SVGElement>(*this).detachAllProperties();
+
#ifndef NDEBUG
m_deletionHasBegun = true;
#endif
diff --git a/Source/WebCore/svg/SVGElement.h b/Source/WebCore/svg/SVGElement.h
index a4beeea..b4a701d 100644
--- a/Source/WebCore/svg/SVGElement.h
+++ b/Source/WebCore/svg/SVGElement.h
@@ -128,6 +128,7 @@
using PropertyRegistry = SVGPropertyOwnerRegistry<SVGElement>;
virtual const SVGPropertyRegistry& propertyRegistry() const { return m_propertyRegistry; }
+ void detachAllProperties() { propertyRegistry().detachAllProperties(); }
bool isAnimatedPropertyAttribute(const QualifiedName&) const;
bool isAnimatedAttribute(const QualifiedName&) const;
