RenderTreeNeedsLayoutChecker fails with absolutely positioned svg and <use>
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 21 Mar 2018 01:31:09 +0000 (01:31 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 21 Mar 2018 01:31:09 +0000 (01:31 +0000)
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.

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

LayoutTests/ChangeLog
LayoutTests/svg/in-html/path-with-absolute-positioned-svg-and-use-crash-expected.txt [new file with mode: 0644]
LayoutTests/svg/in-html/path-with-absolute-positioned-svg-and-use-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/StyleResolver.cpp
Source/WebCore/css/StyleResolver.h
Source/WebCore/svg/SVGElement.cpp

index 570dd4c..6829afc 100644 (file)
@@ -1,3 +1,13 @@
+2018-03-20  Zalan Bujtas  <zalan@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  <ryanhaddad@apple.com>
 
         Mark http/tests/appcache/abort-cache-onprogress.html as flaky.
diff --git a/LayoutTests/svg/in-html/path-with-absolute-positioned-svg-and-use-crash-expected.txt b/LayoutTests/svg/in-html/path-with-absolute-positioned-svg-and-use-crash-expected.txt
new file mode 100644 (file)
index 0000000..4c5caa9
--- /dev/null
@@ -0,0 +1 @@
+PASS if no crash. 
diff --git a/LayoutTests/svg/in-html/path-with-absolute-positioned-svg-and-use-crash.html b/LayoutTests/svg/in-html/path-with-absolute-positioned-svg-and-use-crash.html
new file mode 100644 (file)
index 0000000..e96382c
--- /dev/null
@@ -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"></use>
+</svg>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+document.body.offsetTop;
+document.styleSheets[0].insertRule('.c3 { border: 5px solid red; } ', '.c3');
+</script>
+</body>
+</html>
index 1d6a295..8bf30f7 100644 (file)
@@ -1,3 +1,22 @@
+2018-03-20  Zalan Bujtas  <zalan@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  <beidson@apple.com>
 
         First piece of process swapping on navigation.
index 14ffe99..159f681 100644 (file)
@@ -810,6 +810,23 @@ static void adjustDisplayContentsStyle(RenderStyle& style, const Element* elemen
         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,20 +1065,8 @@ void StyleResolver::adjustRenderStyle(RenderStyle& style, const RenderStyle& par
         || 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());
-
-        // 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 (is<SVGElement>(element))
+        adjustSVGElementStyle(downcast<SVGElement>(*element), style);
 
     // 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'.
index c46c9c8..310b2e6 100644 (file)
@@ -157,6 +157,7 @@ public:
     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&);
index d78a64d..4b59b1f 100644 (file)
@@ -732,8 +732,11 @@ void SVGElement::synchronizeSystemLanguage(SVGElement* contextElement)
 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);
 }