Skip to content
This repository was archived by the owner on Jun 24, 2022. It is now read-only.

Commit bc5c3ab

Browse files
author
[email protected]
committed
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. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@165836 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 43a4900 commit bc5c3ab

File tree

10 files changed

+100
-6
lines changed

10 files changed

+100
-6
lines changed

‎LayoutTests/ChangeLog

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
2014-03-18 Daniel Bates <[email protected]>
2+
3+
REGRESSION (r163560): ASSERTION FAILED: childrenInline() in WebCore::RenderSVGText::layout
4+
https://bugs.webkit.org/show_bug.cgi?id=130346
5+
6+
Reviewed by Andreas Kling.
7+
8+
Added tests to ensure that SVG <a> and <tref> are always treated as inline-level elements.
9+
10+
* svg/text/a-display-block-expected.txt: Added.
11+
* svg/text/a-display-block.html: Added.
12+
* svg/text/tref-display-inherit-expected.txt: Added.
13+
* svg/text/tref-display-inherit.html: Added.
14+
115
2014-03-18 Hans Muller <[email protected]>
216

317
[CSS Shapes] shape-outside: ellipse(50% 50% at) causes crash

‎LayoutTests/svg/text/a-display-block-expected.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Tests that an <a> with display block doesn't cause an assertion failure.
2+
3+
PASS
4+

‎LayoutTests/svg/text/a-display-block.html

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<!DOCTYPE html>
2+
<head>
3+
<style>
4+
a {
5+
display: block;
6+
}
7+
</style>
8+
<script>
9+
if (window.testRunner)
10+
testRunner.dumpAsText();
11+
</script>
12+
</head>
13+
<body>
14+
<p>Tests that an &lt;a&gt; with display block doesn't cause an assertion failure.</p>
15+
<svg>
16+
<text x="0" y="20">
17+
<a xlink:href="https://bugs.webkit.org/show_bug.cgi?id=130346">PASS</a>
18+
</text>
19+
</svg>
20+
</body>
21+
</html>

‎LayoutTests/svg/text/tref-display-inherit-expected.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Tests that a <tref> with display inherit doesn't cause an assertion failure.
2+
3+
PASS
4+

‎LayoutTests/svg/text/tref-display-inherit.html

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<!DOCTYPE html>
2+
<head>
3+
<script>
4+
if (window.testRunner)
5+
testRunner.dumpAsText();
6+
</script>
7+
</head>
8+
<body>
9+
<p>Tests that a &lt;tref&gt; with display inherit doesn't cause an assertion failure.</p>
10+
<svg>
11+
<text x="0" y="20">
12+
<tref display="inherit"></tref>PASS
13+
</text>
14+
</svg>
15+
</body>
16+
</html>

‎Source/WebCore/ChangeLog

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,32 @@
1+
2014-03-18 Daniel Bates <[email protected]>
2+
3+
REGRESSION (r163560): ASSERTION FAILED: childrenInline() in WebCore::RenderSVGText::layout
4+
https://bugs.webkit.org/show_bug.cgi?id=130346
5+
6+
Reviewed by Andreas Kling.
7+
8+
Following <http://trac.webkit.org/changeset/163560>, SVG inline elements may be treated as block-
9+
level elements depending on their CSS styles (e.g. display: block). But such elements should always
10+
be treated as inline-level elements.
11+
12+
Partially revert <http://trac.webkit.org/changeset/164368> as it addressed a similar issue for
13+
<tspan> and <tref>. Instead we should implement RenderSVGInline::updateFromStyle() to ensure that
14+
RenderSVGInline and any derived classes (e.g. RenderSVGTSpan) are always treated as inline elements
15+
regardless of their CSS style because the SVG text layout code depends on this assumption as part
16+
of a performance optimization. We may want to revaluate the benefits of this optimization with respect
17+
to code clarity and ensuring the code is less error prone.
18+
19+
Test: svg/text/a-display-block.html
20+
svg/text/tref-display-inherit.html
21+
22+
* css/StyleResolver.cpp:
23+
(WebCore::StyleResolver::adjustRenderStyle): Revert changes from <http://trac.webkit.org/changeset/164368>.
24+
* rendering/RenderInline.h:
25+
* rendering/svg/RenderSVGInline.cpp:
26+
(WebCore::RenderSVGInline::updateFromStyle): Added; ensure that RenderSVGInline and any derived
27+
classes are treated as inline elements because the SVG text layout code depends on this assumption.
28+
* rendering/svg/RenderSVGInline.h:
29+
130
2014-03-18 Hans Muller <[email protected]>
231

332
[CSS Shapes] shape-outside: ellipse(50% 50% at) causes crash

‎Source/WebCore/css/StyleResolver.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1363,10 +1363,6 @@ void StyleResolver::adjustRenderStyle(RenderStyle& style, const RenderStyle& par
13631363
// SVG text layout code expects us to be a block-level style element.
13641364
if ((e->hasTagName(SVGNames::foreignObjectTag) || e->hasTagName(SVGNames::textTag)) && style.isDisplayInlineType())
13651365
style.setDisplay(BLOCK);
1366-
1367-
// SVG text layout code expects us to be an inline-level style element.
1368-
if ((e->hasTagName(SVGNames::tspanTag) || e->hasTagName(SVGNames::textPathTag)) && style.display() != NONE && !style.isDisplayInlineType())
1369-
style.setDisplay(INLINE);
13701366
}
13711367
}
13721368

‎Source/WebCore/rendering/RenderInline.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ class RenderInline : public RenderBoxModelObject {
100100

101101
virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle) override;
102102

103+
virtual void updateFromStyle() override;
104+
103105
private:
104106
virtual const char* renderName() const override;
105107

@@ -169,8 +171,6 @@ class RenderInline : public RenderBoxModelObject {
169171
virtual void addAnnotatedRegions(Vector<AnnotatedRegionValue>&) override final;
170172
#endif
171173

172-
virtual void updateFromStyle() override final;
173-
174174
RenderPtr<RenderInline> clone() const;
175175

176176
void paintOutlineForLine(GraphicsContext*, const LayoutPoint&, const LayoutRect& prevLine, const LayoutRect& thisLine,

‎Source/WebCore/rendering/svg/RenderSVGInline.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,14 @@ void RenderSVGInline::styleDidChange(StyleDifference diff, const RenderStyle* ol
112112
SVGResourcesCache::clientStyleChanged(*this, diff, style());
113113
}
114114

115+
void RenderSVGInline::updateFromStyle()
116+
{
117+
RenderInline::updateFromStyle();
118+
119+
// SVG text layout code expects us to be an inline-level element.
120+
setInline(true);
121+
}
122+
115123
void RenderSVGInline::addChild(RenderObject* child, RenderObject* beforeChild)
116124
{
117125
RenderInline::addChild(child, beforeChild);

‎Source/WebCore/rendering/svg/RenderSVGInline.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ class RenderSVGInline : public RenderInline {
3939
virtual bool requiresLayer() const override final { return false; }
4040
virtual bool isSVGInline() const override final { return true; }
4141

42+
virtual void updateFromStyle() override final;
43+
4244
// Chapter 10.4 of the SVG Specification say that we should use the
4345
// object bounding box of the parent text element.
4446
// We search for the root text element and take its bounding box.

0 commit comments

Comments
 (0)