- Revision
- 165836
- Author
- [email protected]
- Date
- 2014-03-18 12:59:05 -0700 (Tue, 18 Mar 2014)
Log Message
REGRESSION (r163560): ASSERTION FAILED: childrenInline() in WebCore::RenderSVGText::layout
https://bugs.webkit.org/show_bug.cgi?id=130346
Reviewed by Andreas Kling.
Source/WebCore:
Following <http://trac.webkit.org/changeset/163560>, SVG inline elements may be treated as block-
level elements depending on their CSS styles (e.g. display: block). But such elements should always
be treated as inline-level elements.
Partially revert <http://trac.webkit.org/changeset/164368> as it addressed a similar issue for
<tspan> and <tref>. Instead we should implement RenderSVGInline::updateFromStyle() to ensure that
RenderSVGInline and any derived classes (e.g. RenderSVGTSpan) are always treated as inline elements
regardless of their CSS style because the SVG text layout code depends on this assumption as part
of a performance optimization. We may want to revaluate the benefits of this optimization with respect
to code clarity and ensuring the code is less error prone.
Test: svg/text/a-display-block.html
svg/text/tref-display-inherit.html
* css/StyleResolver.cpp:
(WebCore::StyleResolver::adjustRenderStyle): Revert changes from <http://trac.webkit.org/changeset/164368>.
* rendering/RenderInline.h:
* rendering/svg/RenderSVGInline.cpp:
(WebCore::RenderSVGInline::updateFromStyle): Added; ensure that RenderSVGInline and any derived
classes are treated as inline elements because the SVG text layout code depends on this assumption.
* rendering/svg/RenderSVGInline.h:
LayoutTests:
Added tests to ensure that SVG <a> and <tref> are always treated as inline-level elements.
* svg/text/a-display-block-expected.txt: Added.
* svg/text/a-display-block.html: Added.
* svg/text/tref-display-inherit-expected.txt: Added.
* svg/text/tref-display-inherit.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (165835 => 165836)
--- trunk/LayoutTests/ChangeLog 2014-03-18 19:50:57 UTC (rev 165835)
+++ trunk/LayoutTests/ChangeLog 2014-03-18 19:59:05 UTC (rev 165836)
@@ -1,3 +1,17 @@
+2014-03-18 Daniel Bates <[email protected]>
+
+ REGRESSION (r163560): ASSERTION FAILED: childrenInline() in WebCore::RenderSVGText::layout
+ https://bugs.webkit.org/show_bug.cgi?id=130346
+
+ Reviewed by Andreas Kling.
+
+ Added tests to ensure that SVG <a> and <tref> are always treated as inline-level elements.
+
+ * svg/text/a-display-block-expected.txt: Added.
+ * svg/text/a-display-block.html: Added.
+ * svg/text/tref-display-inherit-expected.txt: Added.
+ * svg/text/tref-display-inherit.html: Added.
+
2014-03-18 Hans Muller <[email protected]>
[CSS Shapes] shape-outside: ellipse(50% 50% at) causes crash
Added: trunk/LayoutTests/svg/text/a-display-block-expected.txt (0 => 165836)
--- trunk/LayoutTests/svg/text/a-display-block-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/text/a-display-block-expected.txt 2014-03-18 19:59:05 UTC (rev 165836)
@@ -0,0 +1,4 @@
+Tests that an <a> with display block doesn't cause an assertion failure.
+
+PASS
+
Added: trunk/LayoutTests/svg/text/a-display-block.html (0 => 165836)
--- trunk/LayoutTests/svg/text/a-display-block.html (rev 0)
+++ trunk/LayoutTests/svg/text/a-display-block.html 2014-03-18 19:59:05 UTC (rev 165836)
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<head>
+<style>
+a {
+ display: block;
+}
+</style>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+</script>
+</head>
+<body>
+<p>Tests that an <a> with display block doesn't cause an assertion failure.</p>
+<svg>
+ <text x="0" y="20">
+ <a xlink:href=""
+ </text>
+</svg>
+</body>
+</html>
Added: trunk/LayoutTests/svg/text/tref-display-inherit-expected.txt (0 => 165836)
--- trunk/LayoutTests/svg/text/tref-display-inherit-expected.txt (rev 0)
+++ trunk/LayoutTests/svg/text/tref-display-inherit-expected.txt 2014-03-18 19:59:05 UTC (rev 165836)
@@ -0,0 +1,4 @@
+Tests that a <tref> with display inherit doesn't cause an assertion failure.
+
+PASS
+
Added: trunk/LayoutTests/svg/text/tref-display-inherit.html (0 => 165836)
--- trunk/LayoutTests/svg/text/tref-display-inherit.html (rev 0)
+++ trunk/LayoutTests/svg/text/tref-display-inherit.html 2014-03-18 19:59:05 UTC (rev 165836)
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<head>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+</script>
+</head>
+<body>
+<p>Tests that a <tref> with display inherit doesn't cause an assertion failure.</p>
+<svg>
+ <text x="0" y="20">
+ <tref display="inherit"></tref>PASS
+ </text>
+</svg>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (165835 => 165836)
--- trunk/Source/WebCore/ChangeLog 2014-03-18 19:50:57 UTC (rev 165835)
+++ trunk/Source/WebCore/ChangeLog 2014-03-18 19:59:05 UTC (rev 165836)
@@ -1,3 +1,32 @@
+2014-03-18 Daniel Bates <[email protected]>
+
+ REGRESSION (r163560): ASSERTION FAILED: childrenInline() in WebCore::RenderSVGText::layout
+ https://bugs.webkit.org/show_bug.cgi?id=130346
+
+ Reviewed by Andreas Kling.
+
+ Following <http://trac.webkit.org/changeset/163560>, SVG inline elements may be treated as block-
+ level elements depending on their CSS styles (e.g. display: block). But such elements should always
+ be treated as inline-level elements.
+
+ Partially revert <http://trac.webkit.org/changeset/164368> as it addressed a similar issue for
+ <tspan> and <tref>. Instead we should implement RenderSVGInline::updateFromStyle() to ensure that
+ RenderSVGInline and any derived classes (e.g. RenderSVGTSpan) are always treated as inline elements
+ regardless of their CSS style because the SVG text layout code depends on this assumption as part
+ of a performance optimization. We may want to revaluate the benefits of this optimization with respect
+ to code clarity and ensuring the code is less error prone.
+
+ Test: svg/text/a-display-block.html
+ svg/text/tref-display-inherit.html
+
+ * css/StyleResolver.cpp:
+ (WebCore::StyleResolver::adjustRenderStyle): Revert changes from <http://trac.webkit.org/changeset/164368>.
+ * rendering/RenderInline.h:
+ * rendering/svg/RenderSVGInline.cpp:
+ (WebCore::RenderSVGInline::updateFromStyle): Added; ensure that RenderSVGInline and any derived
+ classes are treated as inline elements because the SVG text layout code depends on this assumption.
+ * rendering/svg/RenderSVGInline.h:
+
2014-03-18 Hans Muller <[email protected]>
[CSS Shapes] shape-outside: ellipse(50% 50% at) causes crash
Modified: trunk/Source/WebCore/css/StyleResolver.cpp (165835 => 165836)
--- trunk/Source/WebCore/css/StyleResolver.cpp 2014-03-18 19:50:57 UTC (rev 165835)
+++ trunk/Source/WebCore/css/StyleResolver.cpp 2014-03-18 19:59:05 UTC (rev 165836)
@@ -1363,10 +1363,6 @@
// SVG text layout code expects us to be a block-level style element.
if ((e->hasTagName(SVGNames::foreignObjectTag) || e->hasTagName(SVGNames::textTag)) && style.isDisplayInlineType())
style.setDisplay(BLOCK);
-
- // SVG text layout code expects us to be an inline-level style element.
- if ((e->hasTagName(SVGNames::tspanTag) || e->hasTagName(SVGNames::textPathTag)) && style.display() != NONE && !style.isDisplayInlineType())
- style.setDisplay(INLINE);
}
}
Modified: trunk/Source/WebCore/rendering/RenderInline.h (165835 => 165836)
--- trunk/Source/WebCore/rendering/RenderInline.h 2014-03-18 19:50:57 UTC (rev 165835)
+++ trunk/Source/WebCore/rendering/RenderInline.h 2014-03-18 19:59:05 UTC (rev 165836)
@@ -100,6 +100,8 @@
virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle) override;
+ virtual void updateFromStyle() override;
+
private:
virtual const char* renderName() const override;
@@ -169,8 +171,6 @@
virtual void addAnnotatedRegions(Vector<AnnotatedRegionValue>&) override final;
#endif
- virtual void updateFromStyle() override final;
-
RenderPtr<RenderInline> clone() const;
void paintOutlineForLine(GraphicsContext*, const LayoutPoint&, const LayoutRect& prevLine, const LayoutRect& thisLine,
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGInline.cpp (165835 => 165836)
--- trunk/Source/WebCore/rendering/svg/RenderSVGInline.cpp 2014-03-18 19:50:57 UTC (rev 165835)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGInline.cpp 2014-03-18 19:59:05 UTC (rev 165836)
@@ -112,6 +112,14 @@
SVGResourcesCache::clientStyleChanged(*this, diff, style());
}
+void RenderSVGInline::updateFromStyle()
+{
+ RenderInline::updateFromStyle();
+
+ // SVG text layout code expects us to be an inline-level element.
+ setInline(true);
+}
+
void RenderSVGInline::addChild(RenderObject* child, RenderObject* beforeChild)
{
RenderInline::addChild(child, beforeChild);
Modified: trunk/Source/WebCore/rendering/svg/RenderSVGInline.h (165835 => 165836)
--- trunk/Source/WebCore/rendering/svg/RenderSVGInline.h 2014-03-18 19:50:57 UTC (rev 165835)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGInline.h 2014-03-18 19:59:05 UTC (rev 165836)
@@ -39,6 +39,8 @@
virtual bool requiresLayer() const override final { return false; }
virtual bool isSVGInline() const override final { return true; }
+ virtual void updateFromStyle() override final;
+
// Chapter 10.4 of the SVG Specification say that we should use the
// object bounding box of the parent text element.
// We search for the root text element and take its bounding box.
