REGRESSION (Safari 5.0.5 - 5.1): No animation on svg-wow.org/text-effects/text-effect...
authorzimmermann@webkit.org <zimmermann@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Feb 2012 12:58:42 +0000 (12:58 +0000)
committerzimmermann@webkit.org <zimmermann@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Feb 2012 12:58:42 +0000 (12:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=65072

Reviewed by Zoltan Herczeg.

Source/WebCore:

Fix EMS/EXS length resolving, when the target context has no renderer, eg.
<text display="none" dy="1em">ABC</text>, myText.dy.baseVal.getItem(0).value()
currently throws, even if <text> has a parent, we could use to resolve the length.

Always fall-back to parent context, to resolve EMS/EXS units, instead of ignoring it.
The current behaviour stays the same, if the target element is not in the document,
then we really can't resolve lengths like this.

Tests: svg/text/ems-display-none.svg
       svg/text/exs-display-none.svg

* svg/SVGLengthContext.cpp:
(WebCore::renderStyleForLengthResolving):
(WebCore::SVGLengthContext::convertValueFromUserUnitsToEMS):
(WebCore::SVGLengthContext::convertValueFromEMSToUserUnits):
(WebCore::SVGLengthContext::convertValueFromUserUnitsToEXS):
(WebCore::SVGLengthContext::convertValueFromEXSToUserUnits):

LayoutTests:

* platform/chromium/test_expectations.txt:
* platform/mac/svg/text/ems-display-none-expected.png: Added.
* platform/mac/svg/text/ems-display-none-expected.txt: Added.
* platform/mac/svg/text/exs-display-none-expected.png: Added.
* platform/mac/svg/text/exs-display-none-expected.txt: Added.
* svg/text/ems-display-none.svg: Added.
* svg/text/exs-display-none.svg: Added.

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

LayoutTests/ChangeLog
LayoutTests/platform/chromium/test_expectations.txt
LayoutTests/platform/mac/svg/text/ems-display-none-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/svg/text/ems-display-none-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac/svg/text/exs-display-none-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/svg/text/exs-display-none-expected.txt [new file with mode: 0644]
LayoutTests/svg/text/ems-display-none.svg [new file with mode: 0644]
LayoutTests/svg/text/exs-display-none.svg [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/svg/SVGLengthContext.cpp [changed mode: 0755->0644]

index 77dbd89..5a293f8 100644 (file)
@@ -1,3 +1,18 @@
+2012-02-15  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        REGRESSION (Safari 5.0.5 - 5.1): No animation on svg-wow.org/text-effects/text-effects.xhtml
+        https://bugs.webkit.org/show_bug.cgi?id=65072
+
+        Reviewed by Zoltan Herczeg.
+
+        * platform/chromium/test_expectations.txt:
+        * platform/mac/svg/text/ems-display-none-expected.png: Added.
+        * platform/mac/svg/text/ems-display-none-expected.txt: Added.
+        * platform/mac/svg/text/exs-display-none-expected.png: Added.
+        * platform/mac/svg/text/exs-display-none-expected.txt: Added.
+        * svg/text/ems-display-none.svg: Added.
+        * svg/text/exs-display-none.svg: Added.
+
 2012-02-16  Csaba Osztrogonác  <ossy@webkit.org>
 
         [Qt] Unreviewed gardening.
index b03ea3c..4b709fb 100644 (file)
@@ -4230,6 +4230,9 @@ BUGWK78219 : svg/dynamic-updates/SVGTextElement-svgdom-y-prop.html = IMAGE
 BUGWK78219 : svg/dynamic-updates/SVGUseElement-dom-requiredFeatures.html = IMAGE
 BUGWK78219 : svg/dynamic-updates/SVGUseElement-svgdom-requiredFeatures.html = IMAGE
 
+// Needs new baselines.
+BUGWK65072 : svg/text/exs-display-none.svg = IMAGE IMAGE+TEXT
+BUGWK65072 : svg/text/ems-display-none.svg = IMAGE IMAGE+TEXT
+
 // Started failing http://trac.webkit.org/log/?verbose=on&rev=107837&stop_rev=107836
 BUGWK78755 : compositing/culling/scrolled-within-boxshadow.html = IMAGE
-
diff --git a/LayoutTests/platform/mac/svg/text/ems-display-none-expected.png b/LayoutTests/platform/mac/svg/text/ems-display-none-expected.png
new file mode 100644 (file)
index 0000000..e807ded
Binary files /dev/null and b/LayoutTests/platform/mac/svg/text/ems-display-none-expected.png differ
diff --git a/LayoutTests/platform/mac/svg/text/ems-display-none-expected.txt b/LayoutTests/platform/mac/svg/text/ems-display-none-expected.txt
new file mode 100644 (file)
index 0000000..5e4cc79
--- /dev/null
@@ -0,0 +1,14 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderSVGRoot {svg} at (50,54) size 574x86
+    RenderSVGContainer {g} at (50,54) size 574x86
+      RenderSVGText {text} at (50,54) size 574x86 contains 1 chunk(s)
+        RenderSVGTSpan {tspan} at (0,0) size 564x46
+          RenderSVGInlineText {#text} at (0,0) size 564x46
+            chunk 1 text run 1 at (50.00,90.00) startOffset 0 endOffset 36 width 564.00: "Two lines of text should be visible."
+        RenderSVGInlineText {#text} at (564,0) size 10x46
+          chunk 1 text run 1 at (614.00,90.00) startOffset 0 endOffset 1 width 10.00: " "
+        RenderSVGTSpan {tspan} at (0,0) size 564x46
+          RenderSVGInlineText {#text} at (0,40) size 564x46
+            chunk 1 text run 1 at (50.00,130.00) startOffset 0 endOffset 36 width 564.00: "Two lines of text should be visible."
diff --git a/LayoutTests/platform/mac/svg/text/exs-display-none-expected.png b/LayoutTests/platform/mac/svg/text/exs-display-none-expected.png
new file mode 100644 (file)
index 0000000..479a94b
Binary files /dev/null and b/LayoutTests/platform/mac/svg/text/exs-display-none-expected.png differ
diff --git a/LayoutTests/platform/mac/svg/text/exs-display-none-expected.txt b/LayoutTests/platform/mac/svg/text/exs-display-none-expected.txt
new file mode 100644 (file)
index 0000000..3a9152c
--- /dev/null
@@ -0,0 +1,14 @@
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderSVGRoot {svg} at (50,50) size 574x82
+    RenderSVGContainer {g} at (50,50) size 574x82
+      RenderSVGText {text} at (50,50) size 574x82 contains 1 chunk(s)
+        RenderSVGTSpan {tspan} at (0,0) size 564x46
+          RenderSVGInlineText {#text} at (0,0) size 564x46
+            chunk 1 text run 1 at (50.00,86.00) startOffset 0 endOffset 36 width 564.00: "Two lines of text should be visible."
+        RenderSVGInlineText {#text} at (564,0) size 10x46
+          chunk 1 text run 1 at (614.00,86.00) startOffset 0 endOffset 1 width 10.00: " "
+        RenderSVGTSpan {tspan} at (0,0) size 564x46
+          RenderSVGInlineText {#text} at (0,36) size 564x46
+            chunk 1 text run 1 at (50.00,122.00) startOffset 0 endOffset 36 width 564.00: "Two lines of text should be visible."
diff --git a/LayoutTests/svg/text/ems-display-none.svg b/LayoutTests/svg/text/ems-display-none.svg
new file mode 100644 (file)
index 0000000..91cd50c
--- /dev/null
@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" onload="runRepaintTest()">
+    <script xlink:href="../../fast/repaint/resources/repaint.js"/>
+    <g font-size="40">
+        <text id="text" y="50" display="none">
+            <tspan id="tspan" x="50" dy="1em">Two lines of text should be visible.</tspan>
+        </text>
+    </g>
+    <script>
+        function repaintTest() {
+            var tspan1 = document.getElementById("tspan");
+            var result = 0;
+            try {
+                result = tspan1.dy.baseVal.getItem(0).value;
+            } catch(e) { }
+
+            var tspan2 = document.createElementNS("http://www.w3.org/2000/svg", "tspan");
+            tspan2.setAttribute("x", "50");
+            tspan2.setAttribute("dy", result);
+            tspan2.textContent = "Two lines of text should be visible.";
+            document.getElementById("text").appendChild(tspan2);
+            document.getElementById("text").setAttribute("display", "inline");
+        }
+    </script>
+</svg>
diff --git a/LayoutTests/svg/text/exs-display-none.svg b/LayoutTests/svg/text/exs-display-none.svg
new file mode 100644 (file)
index 0000000..5c49405
--- /dev/null
@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" onload="runRepaintTest()">
+    <script xlink:href="../../fast/repaint/resources/repaint.js"/>
+    <g font-size="40">
+        <text id="text" y="50" display="none">
+            <tspan id="tspan" x="50" dy="2ex">Two lines of text should be visible.</tspan>
+        </text>
+    </g>
+    <script>
+        function repaintTest() {
+            var tspan1 = document.getElementById("tspan");
+            var result = 0;
+            try {
+                result = tspan1.dy.baseVal.getItem(0).value;
+            } catch(e) { }
+
+            var tspan2 = document.createElementNS("http://www.w3.org/2000/svg", "tspan");
+            tspan2.setAttribute("x", "50");
+            tspan2.setAttribute("dy", result);
+            tspan2.textContent = "Two lines of text should be visible.";
+            document.getElementById("text").appendChild(tspan2);
+            document.getElementById("text").setAttribute("display", "inline");
+        }
+    </script>
+</svg>
index 0e245e9..4280ce4 100644 (file)
@@ -1,3 +1,28 @@
+2012-02-15  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        REGRESSION (Safari 5.0.5 - 5.1): No animation on svg-wow.org/text-effects/text-effects.xhtml
+        https://bugs.webkit.org/show_bug.cgi?id=65072
+
+        Reviewed by Zoltan Herczeg.
+
+        Fix EMS/EXS length resolving, when the target context has no renderer, eg.
+        <text display="none" dy="1em">ABC</text>, myText.dy.baseVal.getItem(0).value()
+        currently throws, even if <text> has a parent, we could use to resolve the length.
+
+        Always fall-back to parent context, to resolve EMS/EXS units, instead of ignoring it.
+        The current behaviour stays the same, if the target element is not in the document,
+        then we really can't resolve lengths like this.
+
+        Tests: svg/text/ems-display-none.svg
+               svg/text/exs-display-none.svg
+
+        * svg/SVGLengthContext.cpp:
+        (WebCore::renderStyleForLengthResolving):
+        (WebCore::SVGLengthContext::convertValueFromUserUnitsToEMS):
+        (WebCore::SVGLengthContext::convertValueFromEMSToUserUnits):
+        (WebCore::SVGLengthContext::convertValueFromUserUnitsToEXS):
+        (WebCore::SVGLengthContext::convertValueFromEXSToUserUnits):
+
 2012-02-16  Simon Hausmann  <simon.hausmann@nokia.com>
 
         Build fix for Qt 5 without QtWidgets.
old mode 100755 (executable)
new mode 100644 (file)
index 36deeff..d33de34
@@ -203,14 +203,31 @@ float SVGLengthContext::convertValueFromPercentageToUserUnits(float value, SVGLe
     return 0;
 }
 
+static inline RenderStyle* renderStyleForLengthResolving(const SVGElement* context)
+{
+    if (!context)
+        return 0;
+
+    const ContainerNode* currentContext = context;
+    while (currentContext) {
+        if (currentContext->renderer())
+            return currentContext->renderer()->style();
+        currentContext = currentContext->parentNode();
+    }
+
+    // There must be at least a RenderSVGRoot renderer, carrying a style.
+    ASSERT_NOT_REACHED();
+    return 0;
+}
+
 float SVGLengthContext::convertValueFromUserUnitsToEMS(float value, ExceptionCode& ec) const
 {
-    if (!m_context || !m_context->renderer() || !m_context->renderer()->style()) {
+    RenderStyle* style = renderStyleForLengthResolving(m_context);
+    if (!style) {
         ec = NOT_SUPPORTED_ERR;
         return 0;
     }
 
-    RenderStyle* style = m_context->renderer()->style();
     float fontSize = style->fontSize();
     if (!fontSize) {
         ec = NOT_SUPPORTED_ERR;
@@ -222,24 +239,23 @@ float SVGLengthContext::convertValueFromUserUnitsToEMS(float value, ExceptionCod
 
 float SVGLengthContext::convertValueFromEMSToUserUnits(float value, ExceptionCode& ec) const
 {
-    if (!m_context || !m_context->renderer() || !m_context->renderer()->style()) {
+    RenderStyle* style = renderStyleForLengthResolving(m_context);
+    if (!style) {
         ec = NOT_SUPPORTED_ERR;
         return 0;
     }
 
-    RenderStyle* style = m_context->renderer()->style();
     return value * style->fontSize();
 }
 
 float SVGLengthContext::convertValueFromUserUnitsToEXS(float value, ExceptionCode& ec) const
 {
-    if (!m_context || !m_context->renderer() || !m_context->renderer()->style()) {
+    RenderStyle* style = renderStyleForLengthResolving(m_context);
+    if (!style) {
         ec = NOT_SUPPORTED_ERR;
         return 0;
     }
 
-    RenderStyle* style = m_context->renderer()->style();
-
     // Use of ceil allows a pixel match to the W3Cs expected output of coords-units-03-b.svg
     // if this causes problems in real world cases maybe it would be best to remove this
     float xHeight = ceilf(style->fontMetrics().xHeight());
@@ -253,12 +269,12 @@ float SVGLengthContext::convertValueFromUserUnitsToEXS(float value, ExceptionCod
 
 float SVGLengthContext::convertValueFromEXSToUserUnits(float value, ExceptionCode& ec) const
 {
-    if (!m_context || !m_context->renderer() || !m_context->renderer()->style()) {
+    RenderStyle* style = renderStyleForLengthResolving(m_context);
+    if (!style) {
         ec = NOT_SUPPORTED_ERR;
         return 0;
     }
 
-    RenderStyle* style = m_context->renderer()->style();
     // Use of ceil allows a pixel match to the W3Cs expected output of coords-units-03-b.svg
     // if this causes problems in real world cases maybe it would be best to remove this
     return value * ceilf(style->fontMetrics().xHeight());