- Revision
- 90156
- Author
- [email protected]
- Date
- 2011-06-30 13:50:16 -0700 (Thu, 30 Jun 2011)
Log Message
2011-06-30 Julien Chaffraix <[email protected]>
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 <[email protected]>
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:
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (90155 => 90156)
--- trunk/LayoutTests/ChangeLog 2011-06-30 20:22:22 UTC (rev 90155)
+++ trunk/LayoutTests/ChangeLog 2011-06-30 20:50:16 UTC (rev 90156)
@@ -1,3 +1,19 @@
+2011-06-30 Julien Chaffraix <[email protected]>
+
+ 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 <[email protected]>
Reviewed by Anders Carlsson.
Added: trunk/LayoutTests/svg/custom/crash-text-in-textpath-expected.txt (0 => 90156)
--- trunk/LayoutTests/svg/custom/crash-text-in-textpath-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/custom/crash-text-in-textpath-expected.txt 2011-06-30 20:50:16 UTC (rev 90156)
@@ -0,0 +1,3 @@
+foo
+Test for bug 63076: Assertion failure in RenderSVGInlineText::characterStartsNewTextChunk
+This test passes if it does not crash
Added: trunk/LayoutTests/svg/custom/crash-text-in-textpath.svg (0 => 90156)
--- trunk/LayoutTests/svg/custom/crash-text-in-textpath.svg (rev 0)
+++ trunk/LayoutTests/svg/custom/crash-text-in-textpath.svg 2011-06-30 20:50:16 UTC (rev 90156)
@@ -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="" Assertion failure in RenderSVGInlineText::characterStartsNewTextChunk</text>
+<text x="10" y="100">This test passes if it does not crash</text>
+</svg>
Property changes on: trunk/LayoutTests/svg/custom/crash-text-in-textpath.svg
___________________________________________________________________
Added: svn:executable
Added: trunk/LayoutTests/svg/custom/text-node-in-text-invalidated-expected.txt (0 => 90156)
--- trunk/LayoutTests/svg/custom/text-node-in-text-invalidated-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/custom/text-node-in-text-invalidated-expected.txt 2011-06-30 20:50:16 UTC (rev 90156)
@@ -0,0 +1,3 @@
+foo bar
+Test for bug 63076: Assertion failure in RenderSVGInlineText::characterStartsNewTextChunk
+This test passes if it does not crash
Added: trunk/LayoutTests/svg/custom/text-node-in-text-invalidated.svg (0 => 90156)
--- trunk/LayoutTests/svg/custom/text-node-in-text-invalidated.svg (rev 0)
+++ trunk/LayoutTests/svg/custom/text-node-in-text-invalidated.svg 2011-06-30 20:50:16 UTC (rev 90156)
@@ -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="" Assertion failure in RenderSVGInlineText::characterStartsNewTextChunk</text>
+<text x="10" y="100">This test passes if it does not crash</text>
+</svg>
Property changes on: trunk/LayoutTests/svg/custom/text-node-in-text-invalidated.svg
___________________________________________________________________
Added: svn:executable
Modified: trunk/Source/WebCore/ChangeLog (90155 => 90156)
--- trunk/Source/WebCore/ChangeLog 2011-06-30 20:22:22 UTC (rev 90155)
+++ trunk/Source/WebCore/ChangeLog 2011-06-30 20:50:16 UTC (rev 90156)
@@ -1,3 +1,32 @@
+2011-06-30 Julien Chaffraix <[email protected]>
+
+ 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 <[email protected]>
Unreviewed build fix for !ENABLE(DATABASE) after r84789.
Modified: trunk/Source/WebCore/svg/SVGTextContentElement.cpp (90155 => 90156)
--- trunk/Source/WebCore/svg/SVGTextContentElement.cpp 2011-06-30 20:22:22 UTC (rev 90155)
+++ trunk/Source/WebCore/svg/SVGTextContentElement.cpp 2011-06-30 20:50:16 UTC (rev 90156)
@@ -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 @@
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)
Modified: trunk/Source/WebCore/svg/SVGTextContentElement.h (90155 => 90156)
--- trunk/Source/WebCore/svg/SVGTextContentElement.h 2011-06-30 20:22:22 UTC (rev 90155)
+++ trunk/Source/WebCore/svg/SVGTextContentElement.h 2011-06-30 20:50:16 UTC (rev 90156)
@@ -72,6 +72,7 @@
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; }
Modified: trunk/Source/WebCore/svg/SVGTextPositioningElement.cpp (90155 => 90156)
--- trunk/Source/WebCore/svg/SVGTextPositioningElement.cpp 2011-06-30 20:22:22 UTC (rev 90155)
+++ trunk/Source/WebCore/svg/SVGTextPositioningElement.cpp 2011-06-30 20:50:16 UTC (rev 90156)
@@ -108,30 +108,6 @@
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 @@
return;
if (updateRelativeLengths || attrName == SVGNames::rotateAttr) {
- updatePositioningValuesInRenderer(renderer);
+ if (RenderSVGText* textRenderer = RenderSVGText::locateRenderSVGTextAncestor(renderer))
+ textRenderer->setNeedsPositioningValuesUpdate();
RenderSVGResource::markForLayoutAndParentResourceInvalidation(renderer);
return;
}
@@ -162,17 +139,6 @@
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()) {
Modified: trunk/Source/WebCore/svg/SVGTextPositioningElement.h (90155 => 90156)
--- trunk/Source/WebCore/svg/SVGTextPositioningElement.h 2011-06-30 20:22:22 UTC (rev 90155)
+++ trunk/Source/WebCore/svg/SVGTextPositioningElement.h 2011-06-30 20:50:16 UTC (rev 90156)
@@ -37,7 +37,6 @@
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&);
