Fix text positioning with non-bmp characters.

https://bugs.webkit.org/show_bug.cgi?id=87681

Reviewed by Nikolas Zimmermann.

Source/WebCore:

Previously when constructing metrics for tspans with non-bmp characters,
each non-bmp character treated as a skipped character in the same way that
spaces are ignored.
This made sense because the initial SVGCharacterDataMap for <text> is
indexed by character index (not string length) so the high portion of a
non-bmp character was treated as a skipped space. Unfortunately, this
led to a bug because skipped spaces lead to an offset in the positioning
values list but non-bmp characters do not.

This change switches the code to use a new offset for non-bmp characters,
surrogatePairCharacters, which does not affect the positioning values list.

Tests: svg/text/non-bmp-tspans-expected.svg
       svg/text/non-bmp-tspans.svg

* rendering/svg/SVGTextMetricsBuilder.cpp:
(WebCore::SVGTextMetricsBuilder::measureTextRenderer):

LayoutTests:

* svg/text/non-bmp-tspans-expected.svg: Added.
* svg/text/non-bmp-tspans.svg: Added.

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 6c59540..b779270 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2012-07-03  Philip Rogers  <pdr@google.com>
+
+        Fix text positioning with non-bmp characters.
+        https://bugs.webkit.org/show_bug.cgi?id=87681
+
+        Reviewed by Nikolas Zimmermann.
+
+        * svg/text/non-bmp-tspans-expected.svg: Added.
+        * svg/text/non-bmp-tspans.svg: Added.
+
 2012-07-03  Gyuyoung Kim  <gyuyoung.kim@samsung.com>
 
         Improve test cases for network information APIs

diff --git a/LayoutTests/svg/text/non-bmp-tspans-expected.svg b/LayoutTests/svg/text/non-bmp-tspans-expected.svg
new file mode 100644 (file)
index 0000000..1a9a108
--- /dev/null
+++ b/LayoutTests/svg/text/non-bmp-tspans-expected.svg
@@ -0,0 +1,14 @@
+<svg xmlns="http://www.w3.org/2000/svg" width="800">
+    <!-- Test for WK87681 where non-BMP characters broke positioning in sibling tspans. -->
+    <text x="30" y="30">This test passes if there are three rows of 2 green Cs each with</text>
+    <text x="30" y="50">the first row cursive, the second plain, and the third cursive.</text>
+    <text fill="green">
+        <tspan x="100" y="100">&#x1d49e;&#x1d49e;</tspan>
+    </text>
+    <text fill="green">
+        <tspan x="100" y="150">CC</tspan>
+    </text>
+    <text fill="green">
+        <tspan x="100" y="200">&#x1d49e;&#x1d49e;</tspan>
+    </text>
+</svg>

diff --git a/LayoutTests/svg/text/non-bmp-tspans.svg b/LayoutTests/svg/text/non-bmp-tspans.svg
new file mode 100644 (file)
index 0000000..736fe0e
--- /dev/null
+++ b/LayoutTests/svg/text/non-bmp-tspans.svg
@@ -0,0 +1,10 @@
+<svg xmlns="http://www.w3.org/2000/svg" width="800">
+    <!-- Test for WK87681 where non-BMP characters broke positioning in sibling tspans. -->
+    <text x="30" y="30">This test passes if there are three rows of 2 green Cs each with</text>
+    <text x="30" y="50">the first row cursive, the second plain, and the third cursive.</text>
+    <text fill="green">
+        <tspan x="100" y="100">&#x1d49e;&#x1d49e;</tspan>
+        <tspan x="100" y="150">CC</tspan>
+        <tspan x="100" y="200">&#x1d49e;&#x1d49e;</tspan>
+    </text>
+</svg>

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 6aacf48..d2dbbe4 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+2012-07-03  Philip Rogers  <pdr@google.com>
+
+        Fix text positioning with non-bmp characters.
+        https://bugs.webkit.org/show_bug.cgi?id=87681
+
+        Reviewed by Nikolas Zimmermann.
+
+        Previously when constructing metrics for tspans with non-bmp characters,
+        each non-bmp character treated as a skipped character in the same way that
+        spaces are ignored.
+        This made sense because the initial SVGCharacterDataMap for <text> is
+        indexed by character index (not string length) so the high portion of a
+        non-bmp character was treated as a skipped space. Unfortunately, this
+        led to a bug because skipped spaces lead to an offset in the positioning
+        values list but non-bmp characters do not.
+
+        This change switches the code to use a new offset for non-bmp characters,
+        surrogatePairCharacters, which does not affect the positioning values list.
+
+        Tests: svg/text/non-bmp-tspans-expected.svg
+               svg/text/non-bmp-tspans.svg
+
+        * rendering/svg/SVGTextMetricsBuilder.cpp:
+        (WebCore::SVGTextMetricsBuilder::measureTextRenderer):
+
 2012-07-03  Gyuyoung Kim  <gyuyoung.kim@samsung.com>
 
         Improve test cases for network information APIs

diff --git a/Source/WebCore/rendering/svg/SVGTextMetricsBuilder.cpp b/Source/WebCore/rendering/svg/SVGTextMetricsBuilder.cpp
index 81fc325..685e515 100644 (file)
--- a/Source/WebCore/rendering/svg/SVGTextMetricsBuilder.cpp
+++ b/Source/WebCore/rendering/svg/SVGTextMetricsBuilder.cpp
@@ -155,6 +155,7 @@ void SVGTextMetricsBuilder::measureTextRenderer(RenderSVGInlineText* text, Measu
 
     initializeMeasurementWithTextRenderer(text);
     bool preserveWhiteSpace = text->style()->whiteSpace() == PRE;
+    int surrogatePairCharacters = 0;
 
     while (advance()) {
         const UChar* currentCharacter = m_run.data(m_textPosition);
@@ -168,7 +169,7 @@ void SVGTextMetricsBuilder::measureTextRenderer(RenderSVGInlineText* text, Measu
 
         if (data->processRenderer) {
             if (data->allCharactersMap) {
-                const SVGCharacterDataMap::const_iterator it = data->allCharactersMap->find(data->valueListPosition + m_textPosition - data->skippedCharacters + 1);
+                const SVGCharacterDataMap::const_iterator it = data->allCharactersMap->find(data->valueListPosition + m_textPosition - data->skippedCharacters - surrogatePairCharacters + 1);
                 if (it != data->allCharactersMap->end())
                     attributes->characterDataMap().set(m_textPosition + 1, it->second);
             }
@@ -176,7 +177,7 @@ void SVGTextMetricsBuilder::measureTextRenderer(RenderSVGInlineText* text, Measu
         }
 
         if (data->allCharactersMap && currentCharacterStartsSurrogatePair())
-            data->skippedCharacters += m_currentMetrics.length() - 1;
+            surrogatePairCharacters++;
 
         data->lastCharacter = currentCharacter;
     }