2011-06-30 Julien Chaffraix <jchaffraix@webkit.org>
authorjchaffraix@webkit.org <jchaffraix@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Jun 2011 20:50:16 +0000 (20:50 +0000)
committerjchaffraix@webkit.org <jchaffraix@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 30 Jun 2011 20:50:16 +0000 (20:50 +0000)
        Reviewed by Nikolas Zimmermann.

        Assertion failure in RenderSVGInlineText::characterStartsNewTextChunk
        https://bugs.webkit.org/show_bug.cgi?id=63076

        * svg/custom/crash-text-in-textpath-expected.txt: Added.
        * svg/custom/crash-text-in-textpath.svg: Added.
        Original crashing test case.

        * svg/custom/text-node-in-text-invalidated-expected.txt: Added.
        * svg/custom/text-node-in-text-invalidated.svg: Added.
        This test case was not crashing. However it is good to make sure this change
        did not regress that.
2011-06-30  Julien Chaffraix  <jchaffraix@webkit.org>

        Reviewed by Nikolas Zimmermann.

        Assertion failure in RenderSVGInlineText::characterStartsNewTextChunk
        https://bugs.webkit.org/show_bug.cgi?id=63076

        Tests: svg/custom/crash-text-in-textpath.svg
               svg/custom/text-node-in-text-invalidated.svg

        The problem was that we did not call setNeedsPositionUpdate on RenderSVGText. When
        doing our layout, we would not update the attributes on our SVGRenderInlineText as
        we would not lay it out.

        This was caused by childrenChanged being overridden on SVGTextPositioningElement but
        not on SVGTextPathElement.

        As both classes shared the same mother class, it made sense to move the logic here.
        There should be no other side effects as SVGTextPathElement and SVGTextPositioningElement
        are the only classes deriving from SVGTextContentElement.

        * svg/SVGTextContentElement.cpp:
        (WebCore::SVGTextContentElement::childrenChanged): Moved this method from SVGTextPositioningElement.
        * svg/SVGTextContentElement.h:
        * svg/SVGTextPositioningElement.cpp:
        (WebCore::SVGTextPositioningElement::svgAttributeChanged): Updated after updatePositioningValuesInRenderer
        removal, replaced by RenderSVGText::locateRenderSVGTextAncestor.
        * svg/SVGTextPositioningElement.h:

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

LayoutTests/ChangeLog
LayoutTests/svg/custom/crash-text-in-textpath-expected.txt [new file with mode: 0644]
LayoutTests/svg/custom/crash-text-in-textpath.svg [new file with mode: 0755]
LayoutTests/svg/custom/text-node-in-text-invalidated-expected.txt [new file with mode: 0644]
LayoutTests/svg/custom/text-node-in-text-invalidated.svg [new file with mode: 0755]
Source/WebCore/ChangeLog
Source/WebCore/svg/SVGTextContentElement.cpp
Source/WebCore/svg/SVGTextContentElement.h
Source/WebCore/svg/SVGTextPositioningElement.cpp
Source/WebCore/svg/SVGTextPositioningElement.h

index 1ca33b8..a17d4f0 100644 (file)
@@ -1,3 +1,19 @@
+2011-06-30  Julien Chaffraix  <jchaffraix@webkit.org>
+
+        Reviewed by Nikolas Zimmermann.
+
+        Assertion failure in RenderSVGInlineText::characterStartsNewTextChunk
+        https://bugs.webkit.org/show_bug.cgi?id=63076
+
+        * svg/custom/crash-text-in-textpath-expected.txt: Added.
+        * svg/custom/crash-text-in-textpath.svg: Added.
+        Original crashing test case.
+
+        * svg/custom/text-node-in-text-invalidated-expected.txt: Added.
+        * svg/custom/text-node-in-text-invalidated.svg: Added.
+        This test case was not crashing. However it is good to make sure this change
+        did not regress that.
+
 2011-06-30  Martin Robinson  <mrobinson@igalia.com>
 
         Reviewed by Anders Carlsson.
diff --git a/LayoutTests/svg/custom/crash-text-in-textpath-expected.txt b/LayoutTests/svg/custom/crash-text-in-textpath-expected.txt
new file mode 100644 (file)
index 0000000..c245c19
--- /dev/null
@@ -0,0 +1,3 @@
+foo
+Test for bug 63076: Assertion failure in RenderSVGInlineText::characterStartsNewTextChunk
+This test passes if it does not crash
diff --git a/LayoutTests/svg/custom/crash-text-in-textpath.svg b/LayoutTests/svg/custom/crash-text-in-textpath.svg
new file mode 100755 (executable)
index 0000000..4fc2841
--- /dev/null
@@ -0,0 +1,20 @@
+<svg version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+<text>
+    <textPath id="textPath">
+        <script>
+            if (window.layoutTestController)
+                layoutTestController.dumpAsText();
+
+            // This triggers a layout before adding the #text node.
+            document.getElementById('textPath').scrollIntoView('foo');
+        </script>
+        foo    
+        <script>
+            // This triggers a layout after adding the #text node to fire the ASSERT.
+            document.getElementById('textPath').scrollIntoView('foo');
+        </script>
+    </textPath>
+</text>
+<text x="10" y="50">Test for bug <a xlink:href="https://bugs.webkit.org/show_bug.cgi?id=63076">63076</a>: Assertion failure in RenderSVGInlineText::characterStartsNewTextChunk</text>
+<text x="10" y="100">This test passes if it does not crash</text>
+</svg>
diff --git a/LayoutTests/svg/custom/text-node-in-text-invalidated-expected.txt b/LayoutTests/svg/custom/text-node-in-text-invalidated-expected.txt
new file mode 100644 (file)
index 0000000..6ab6ee4
--- /dev/null
@@ -0,0 +1,3 @@
+foo bar
+Test for bug 63076: Assertion failure in RenderSVGInlineText::characterStartsNewTextChunk
+This test passes if it does not crash
diff --git a/LayoutTests/svg/custom/text-node-in-text-invalidated.svg b/LayoutTests/svg/custom/text-node-in-text-invalidated.svg
new file mode 100755 (executable)
index 0000000..9301346
--- /dev/null
@@ -0,0 +1,20 @@
+<svg version="1.1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+<text id="text">
+    bar
+    <script>
+        if (window.layoutTestController)
+            layoutTestController.dumpAsText();
+
+        // This triggers a layout before adding the #text node.
+        document.getElementById('text').scrollIntoView('foo');
+
+        var text = document.getElementById('text');
+        text.insertBefore(document.createTextNode('foo'), text.firstChild);
+
+        // This triggers a layout after adding the #text node to fire the ASSERT.
+        document.getElementById('text').scrollIntoView('foo');
+    </script>
+</text>
+<text x="10" y="50">Test for bug <a xlink:href="https://bugs.webkit.org/show_bug.cgi?id=63076">63076</a>: Assertion failure in RenderSVGInlineText::characterStartsNewTextChunk</text>
+<text x="10" y="100">This test passes if it does not crash</text>
+</svg>
index 4c7b37c..a47d6fc 100644 (file)
@@ -1,3 +1,32 @@
+2011-06-30  Julien Chaffraix  <jchaffraix@webkit.org>
+
+        Reviewed by Nikolas Zimmermann.
+
+        Assertion failure in RenderSVGInlineText::characterStartsNewTextChunk
+        https://bugs.webkit.org/show_bug.cgi?id=63076
+
+        Tests: svg/custom/crash-text-in-textpath.svg
+               svg/custom/text-node-in-text-invalidated.svg
+
+        The problem was that we did not call setNeedsPositionUpdate on RenderSVGText. When
+        doing our layout, we would not update the attributes on our SVGRenderInlineText as
+        we would not lay it out.
+
+        This was caused by childrenChanged being overridden on SVGTextPositioningElement but
+        not on SVGTextPathElement.
+
+        As both classes shared the same mother class, it made sense to move the logic here.
+        There should be no other side effects as SVGTextPathElement and SVGTextPositioningElement
+        are the only classes deriving from SVGTextContentElement.
+
+        * svg/SVGTextContentElement.cpp:
+        (WebCore::SVGTextContentElement::childrenChanged): Moved this method from SVGTextPositioningElement.
+        * svg/SVGTextContentElement.h:
+        * svg/SVGTextPositioningElement.cpp:
+        (WebCore::SVGTextPositioningElement::svgAttributeChanged): Updated after updatePositioningValuesInRenderer
+        removal, replaced by RenderSVGText::locateRenderSVGTextAncestor.
+        * svg/SVGTextPositioningElement.h:
+
 2011-06-30  Patrick Gansterer  <paroga@webkit.org>
 
         Unreviewed build fix for !ENABLE(DATABASE) after r84789.
index 33f1017..85274a6 100644 (file)
@@ -29,6 +29,7 @@
 #include "FrameSelection.h"
 #include "RenderObject.h"
 #include "RenderSVGResource.h"
+#include "RenderSVGText.h"
 #include "SVGDocumentExtensions.h"
 #include "SVGElementInstance.h"
 #include "SVGNames.h"
@@ -333,6 +334,17 @@ SVGTextContentElement* SVGTextContentElement::elementFromRenderer(RenderObject*
     return static_cast<SVGTextContentElement*>(node);
 }
 
+void SVGTextContentElement::childrenChanged(bool changedByParser, Node* beforeChange, Node* afterChange, int childCountDelta)
+{
+    SVGStyledElement::childrenChanged(changedByParser, beforeChange, afterChange, childCountDelta);
+
+    if (changedByParser || !renderer())
+        return;
+
+    if (RenderSVGText* textRenderer = RenderSVGText::locateRenderSVGTextAncestor(renderer()))
+        textRenderer->setNeedsPositioningValuesUpdate();
+}
+
 }
 
 #endif // ENABLE(SVG)
index 6969d5b..8deeee8 100644 (file)
@@ -72,6 +72,7 @@ protected:
     void fillPassedAttributeToPropertyTypeMap(AttributeToPropertyTypeMap&);
 
     virtual bool selfHasRelativeLengths() const;
+    virtual void childrenChanged(bool changedByParser = false, Node* beforeChange = 0, Node* afterChange = 0, int childCountDelta = 0);
 
 private:
     virtual bool isTextContent() const { return true; }
index 89f03cb..cd1855b 100644 (file)
@@ -108,30 +108,6 @@ void SVGTextPositioningElement::parseMappedAttribute(Attribute* attr)
     ASSERT_NOT_REACHED();
 }
 
-static inline void updatePositioningValuesInRenderer(RenderObject* renderer)
-{
-    RenderSVGText* textRenderer = 0;
-
-    if (renderer->isSVGText())
-        textRenderer = toRenderSVGText(renderer);
-    else {
-        // Locate RenderSVGText parent renderer.
-        RenderObject* parent = renderer->parent();
-        while (parent && !parent->isSVGText())
-            parent = parent->parent();
-
-        if (parent) {
-            ASSERT(parent->isSVGText());
-            textRenderer = toRenderSVGText(parent);
-        }
-    }
-
-    if (!textRenderer)
-        return;
-
-    textRenderer->setNeedsPositioningValuesUpdate();
-}
-
 void SVGTextPositioningElement::svgAttributeChanged(const QualifiedName& attrName)
 {
     if (!isSupportedAttribute(attrName)) {
@@ -154,7 +130,8 @@ void SVGTextPositioningElement::svgAttributeChanged(const QualifiedName& attrNam
         return;
 
     if (updateRelativeLengths || attrName == SVGNames::rotateAttr) {
-        updatePositioningValuesInRenderer(renderer);
+        if (RenderSVGText* textRenderer = RenderSVGText::locateRenderSVGTextAncestor(renderer))
+            textRenderer->setNeedsPositioningValuesUpdate();
         RenderSVGResource::markForLayoutAndParentResourceInvalidation(renderer);
         return;
     }
@@ -162,17 +139,6 @@ void SVGTextPositioningElement::svgAttributeChanged(const QualifiedName& attrNam
     ASSERT_NOT_REACHED();
 }
 
-void SVGTextPositioningElement::childrenChanged(bool changedByParser, Node* beforeChange, Node* afterChange, int childCountDelta)
-{
-    SVGTextContentElement::childrenChanged(changedByParser, beforeChange, afterChange, childCountDelta);
-
-    if (changedByParser)
-        return;
-
-    if (RenderObject* object = renderer())
-        updatePositioningValuesInRenderer(object);
-}
-
 void SVGTextPositioningElement::synchronizeProperty(const QualifiedName& attrName)
 {
     if (attrName == anyQName()) {
index c0a1888..0cff1dc 100644 (file)
@@ -37,7 +37,6 @@ protected:
 
     bool isSupportedAttribute(const QualifiedName&);
     virtual void parseMappedAttribute(Attribute*);
-    virtual void childrenChanged(bool changedByParser = false, Node* beforeChange = 0, Node* afterChange = 0, int childCountDelta = 0);
     virtual void svgAttributeChanged(const QualifiedName&);
     virtual void synchronizeProperty(const QualifiedName&);
     void fillPassedAttributeToPropertyTypeMap(AttributeToPropertyTypeMap&);