Title: [229782] trunk
Revision
229782
Author
za...@apple.com
Date
2018-03-20 18:31:09 -0700 (Tue, 20 Mar 2018)

Log Message

RenderTreeNeedsLayoutChecker fails with absolutely positioned svg and <use>
https://bugs.webkit.org/show_bug.cgi?id=183718

Reviewed by Antti Koivisto.

Source/WebCore:

This patch ensures after resolving the style for an SVG element with a corresponding element (<use>),
we adjust this style for the cloned SVG element too.

Test: svg/in-html/path-with-absolute-positioned-svg-and-use-crash.html

* css/StyleResolver.cpp:
(WebCore::StyleResolver::adjustSVGElementStyle):
(WebCore::StyleResolver::adjustRenderStyle):
* css/StyleResolver.h:
* svg/SVGElement.cpp:
(WebCore::SVGElement::resolveCustomStyle):

LayoutTests:

* svg/in-html/path-with-absolute-positioned-svg-and-use-crash-expected.txt: Added.
* svg/in-html/path-with-absolute-positioned-svg-and-use-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (229781 => 229782)


--- trunk/LayoutTests/ChangeLog	2018-03-21 01:09:36 UTC (rev 229781)
+++ trunk/LayoutTests/ChangeLog	2018-03-21 01:31:09 UTC (rev 229782)
@@ -1,3 +1,13 @@
+2018-03-20  Zalan Bujtas  <za...@apple.com>
+
+        RenderTreeNeedsLayoutChecker fails with absolutely positioned svg and <use>
+        https://bugs.webkit.org/show_bug.cgi?id=183718
+
+        Reviewed by Antti Koivisto.
+
+        * svg/in-html/path-with-absolute-positioned-svg-and-use-crash-expected.txt: Added.
+        * svg/in-html/path-with-absolute-positioned-svg-and-use-crash.html: Added.
+
 2018-03-20  Ryan Haddad  <ryanhad...@apple.com>
 
         Mark http/tests/appcache/abort-cache-onprogress.html as flaky.

Added: trunk/LayoutTests/svg/in-html/path-with-absolute-positioned-svg-and-use-crash-expected.txt (0 => 229782)


--- trunk/LayoutTests/svg/in-html/path-with-absolute-positioned-svg-and-use-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/svg/in-html/path-with-absolute-positioned-svg-and-use-crash-expected.txt	2018-03-21 01:31:09 UTC (rev 229782)
@@ -0,0 +1 @@
+PASS if no crash. 

Added: trunk/LayoutTests/svg/in-html/path-with-absolute-positioned-svg-and-use-crash.html (0 => 229782)


--- trunk/LayoutTests/svg/in-html/path-with-absolute-positioned-svg-and-use-crash.html	                        (rev 0)
+++ trunk/LayoutTests/svg/in-html/path-with-absolute-positioned-svg-and-use-crash.html	2018-03-21 01:31:09 UTC (rev 229782)
@@ -0,0 +1,23 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<style>
+.c3 {
+}
+</style>
+<body>
+PASS if no crash.
+<svg id="svg" style="position: absolute;">
+	<path class="c3" d="M 100 100 L 300 100 L 200 300 z" fill="orange" stroke="black" stroke-width="3"></path>
+</svg>
+<svg>
+    <use xlink:href=""
+</svg>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+document.body.offsetTop;
+document.styleSheets[0].insertRule('.c3 { border: 5px solid red; } ', '.c3');
+</script>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (229781 => 229782)


--- trunk/Source/WebCore/ChangeLog	2018-03-21 01:09:36 UTC (rev 229781)
+++ trunk/Source/WebCore/ChangeLog	2018-03-21 01:31:09 UTC (rev 229782)
@@ -1,3 +1,22 @@
+2018-03-20  Zalan Bujtas  <za...@apple.com>
+
+        RenderTreeNeedsLayoutChecker fails with absolutely positioned svg and <use>
+        https://bugs.webkit.org/show_bug.cgi?id=183718
+
+        Reviewed by Antti Koivisto.
+
+        This patch ensures after resolving the style for an SVG element with a corresponding element (<use>),
+        we adjust this style for the cloned SVG element too.
+
+        Test: svg/in-html/path-with-absolute-positioned-svg-and-use-crash.html
+
+        * css/StyleResolver.cpp:
+        (WebCore::StyleResolver::adjustSVGElementStyle):
+        (WebCore::StyleResolver::adjustRenderStyle):
+        * css/StyleResolver.h:
+        * svg/SVGElement.cpp:
+        (WebCore::SVGElement::resolveCustomStyle):
+
 2018-03-20  Brady Eidson  <beid...@apple.com>
 
         First piece of process swapping on navigation.

Modified: trunk/Source/WebCore/css/StyleResolver.cpp (229781 => 229782)


--- trunk/Source/WebCore/css/StyleResolver.cpp	2018-03-21 01:09:36 UTC (rev 229781)
+++ trunk/Source/WebCore/css/StyleResolver.cpp	2018-03-21 01:31:09 UTC (rev 229782)
@@ -810,6 +810,23 @@
         style.setDisplay(NONE);
 }
 
+void StyleResolver::adjustSVGElementStyle(const SVGElement& svgElement, RenderStyle& style)
+{
+    // Only the root <svg> element in an SVG document fragment tree honors css position
+    auto isPositioningAllowed = svgElement.hasTagName(SVGNames::svgTag) && svgElement.parentNode() && !svgElement.parentNode()->isSVGElement() && !svgElement.correspondingElement();
+    if (!isPositioningAllowed)
+        style.setPosition(RenderStyle::initialPosition());
+
+    // RenderSVGRoot handles zooming for the whole SVG subtree, so foreignObject content should
+    // not be scaled again.
+    if (svgElement.hasTagName(SVGNames::foreignObjectTag))
+        style.setEffectiveZoom(RenderStyle::initialZoom());
+
+    // SVG text layout code expects us to be a block-level style element.
+    if ((svgElement.hasTagName(SVGNames::foreignObjectTag) || svgElement.hasTagName(SVGNames::textTag)) && style.isDisplayInlineType())
+        style.setDisplay(BLOCK);
+}
+
 void StyleResolver::adjustRenderStyle(RenderStyle& style, const RenderStyle& parentStyle, const RenderStyle* parentBoxStyle, const Element* element)
 {
     // If the composed tree parent has display:contents, the parent box style will be different from the parent style.
@@ -1048,21 +1065,9 @@
         || style.hasBlendMode()))
         style.setTransformStyle3D(TransformStyle3DFlat);
 
-    if (is<SVGElement>(element)) {
-        // Only the root <svg> element in an SVG document fragment tree honors css position
-        if (!(element->hasTagName(SVGNames::svgTag) && element->parentNode() && !element->parentNode()->isSVGElement()))
-            style.setPosition(RenderStyle::initialPosition());
+    if (is<SVGElement>(element))
+        adjustSVGElementStyle(downcast<SVGElement>(*element), style);
 
-        // RenderSVGRoot handles zooming for the whole SVG subtree, so foreignObject content should
-        // not be scaled again.
-        if (element->hasTagName(SVGNames::foreignObjectTag))
-            style.setEffectiveZoom(RenderStyle::initialZoom());
-
-        // SVG text layout code expects us to be a block-level style element.
-        if ((element->hasTagName(SVGNames::foreignObjectTag) || element->hasTagName(SVGNames::textTag)) && style.isDisplayInlineType())
-            style.setDisplay(BLOCK);
-    }
-
     // If the inherited value of justify-items includes the 'legacy' keyword (plus 'left', 'right' or
     // 'center'), 'legacy' computes to the the inherited value. Otherwise, 'auto' computes to 'normal'.
     if (parentBoxStyle->justifyItems().positionType() == LegacyPosition && style.justifyItems().position() == ItemPositionLegacy)

Modified: trunk/Source/WebCore/css/StyleResolver.h (229781 => 229782)


--- trunk/Source/WebCore/css/StyleResolver.h	2018-03-21 01:09:36 UTC (rev 229781)
+++ trunk/Source/WebCore/css/StyleResolver.h	2018-03-21 01:31:09 UTC (rev 229782)
@@ -157,6 +157,7 @@
     void setOverrideDocumentElementStyle(RenderStyle* style) { m_overrideDocumentElementStyle = style; }
 
     void addCurrentSVGFontFaceRules();
+    static void adjustSVGElementStyle(const SVGElement&, RenderStyle&);
 
 private:
     std::unique_ptr<RenderStyle> styleForKeyframe(const RenderStyle*, const StyleRuleKeyframe*, KeyframeValue&);

Modified: trunk/Source/WebCore/svg/SVGElement.cpp (229781 => 229782)


--- trunk/Source/WebCore/svg/SVGElement.cpp	2018-03-21 01:09:36 UTC (rev 229781)
+++ trunk/Source/WebCore/svg/SVGElement.cpp	2018-03-21 01:31:09 UTC (rev 229782)
@@ -732,8 +732,11 @@
 std::optional<ElementStyle> SVGElement::resolveCustomStyle(const RenderStyle& parentStyle, const RenderStyle*)
 {
     // If the element is in a <use> tree we get the style from the definition tree.
-    if (auto styleElement = makeRefPtr(this->correspondingElement()))
-        return styleElement->resolveStyle(&parentStyle);
+    if (auto styleElement = makeRefPtr(this->correspondingElement())) {
+        std::optional<ElementStyle> style = styleElement->resolveStyle(&parentStyle);
+        StyleResolver::adjustSVGElementStyle(*this, *style->renderStyle);
+        return style;
+    }
 
     return resolveStyle(&parentStyle);
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to