Attachment 209848 Details for Bug 108881 – Patch

[patch] Patch

bug-108881-20130828112854.patch (text/plain), 10.69 KB, created by Rashmi Shyamasundar on 2013-08-27 22:53:17 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Rashmi Shyamasundar

Created: 2013-08-27 22:53:17 PDT

Size: 10.69 KB

patch

obsolete

>Subversion Revision: 154674
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 45e5cb4d75310bb0871aa825f24d623dc98defa0..7e7076758732b0ae48e5142c6514691ce56a32e1 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2013-08-27  Rashmi Shyamasundar  <rashmi.s2@samsung.com>
>+
>+        Canvas fillText and measureText handle ideographic spaces differently
>+        https://bugs.webkit.org/show_bug.cgi?id=108881
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        According to spec for text preparation algorithm
>+        http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#text-preparation-algorithm
>+        all the space characters mentioned in the spec
>+        http://www.whatwg.org/specs/web-apps/current-work/multipage/common-microsyntaxes.html#space-character
>+        should be replaced by U+0020 space character.
>+        The canvas functions fillText and measureText should run the text preparation algorithm.
>+        This patch modifies the canvas functions drawTextInternal and measureText to conform to the above spec.
>+
>+        Tests: fast/canvas/canvas-fillText-ideographicSpace.html
>+               fast/canvas/canvas-measureText-ideographicSpace.html
>+
>+        * html/canvas/CanvasRenderingContext2D.cpp:
>+        (WebCore::shouldSpaceBeReplaced):
>+        (WebCore::replaceCharacterInString):
>+        (WebCore::CanvasRenderingContext2D::measureText):
>+        (WebCore::CanvasRenderingContext2D::drawTextInternal):
>+
> 2013-08-27  Allan Sandfeld Jensen  <allan.jensen@digia.com>
> 
>         Font's fast code path doesn't handle partial runs correctly when kerning or ligatures are enabled
>diff --git a/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp b/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp
>old mode 100644
>new mode 100755
>index 0a8bdb12dcd25c7e2ed2ba79ce182f64f4588d36..013d8f10db8a7a7a38f721196f3800a4efc0ba77
>--- a/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp
>+++ b/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp
>@@ -2102,19 +2102,53 @@ void CanvasRenderingContext2D::strokeText(const String& text, float x, float y,
>     drawTextInternal(text, x, y, false, maxWidth, true);
> }
> 
>+static inline bool isSpaceThatNeedsReplacing(UChar c)
>+{
>+    // According to specification all space characters should be replaced with 0x0020 space character.
>+    // http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#text-preparation-algorithm
>+    // The space characters according to specification are : U+0020, U+0009, U+000A, U+000C, and U+000D.
>+    // http://www.whatwg.org/specs/web-apps/current-work/multipage/common-microsyntaxes.html#space-character
>+    // This function returns true for 0x000B also, so that this is backward compatible.
>+    // Otherwise, the test LayoutTests/canvas/philip/tests/2d.text.draw.space.collapse.space.html will fail
>+    return c == 0x0009 || c == 0x000A || c == 0x000B || c == 0x000C || c == 0x000D;
>+}
>+
>+static void normalizeSpaces(String& text)
>+{
>+    size_t i = text.find(isSpaceThatNeedsReplacing);
>+
>+    if (i == notFound)
>+        return;
>+
>+    unsigned textLength = text.length();
>+    Vector<UChar> charVector(textLength);
>+    memcpy(charVector.data(), text.characters(), textLength * sizeof(UChar));
>+
>+    charVector[i++] = ' ';
>+
>+    for (; i < textLength; ++i) {
>+        if (isSpaceThatNeedsReplacing(charVector[i]))
>+            charVector[i] = ' ';
>+    }
>+    text = String::adopt(charVector);
>+}
>+
> PassRefPtr<TextMetrics> CanvasRenderingContext2D::measureText(const String& text)
> {
>     FontCachePurgePreventer fontCachePurgePreventer;
> 
>     RefPtr<TextMetrics> metrics = TextMetrics::create();
> 
>+    String normalizedText = text;
>+    normalizeSpaces(normalizedText);
>+
> #if PLATFORM(QT)
>     // We always use complex text shaping since it can't be turned off for QPainterPath::addText().
>     Font::CodePath oldCodePath = Font::codePath();
>     Font::setCodePath(Font::Complex);
> #endif
> 
>-    metrics->setWidth(accessFont().width(TextRun(text)));
>+    metrics->setWidth(accessFont().width(TextRun(normalizedText)));
> 
> #if PLATFORM(QT)
>     Font::setCodePath(oldCodePath);
>@@ -2123,16 +2157,6 @@ PassRefPtr<TextMetrics> CanvasRenderingContext2D::measureText(const String& text
>     return metrics.release();
> }
> 
>-static void replaceCharacterInString(String& text, WTF::CharacterMatchFunctionPtr matchFunction, const String& replacement)
>-{
>-    const size_t replacementLength = replacement.length();
>-    size_t index = 0;
>-    while ((index = text.find(matchFunction, index)) != notFound) {
>-        text.replace(index, 1, replacement);
>-        index += replacementLength;
>-    }
>-}
>-
> void CanvasRenderingContext2D::drawTextInternal(const String& text, float x, float y, bool fill, float maxWidth, bool useMaxWidth)
> {
>     GraphicsContext* c = drawingContext();
>@@ -2158,9 +2182,9 @@ void CanvasRenderingContext2D::drawTextInternal(const String& text, float x, flo
> 
>     const Font& font = accessFont();
>     const FontMetrics& fontMetrics = font.fontMetrics();
>-    // According to spec, all the space characters must be replaced with U+0020 SPACE characters.
>+
>     String normalizedText = text;
>-    replaceCharacterInString(normalizedText, isSpaceOrNewline, " ");
>+    normalizeSpaces(normalizedText);
> 
>     // FIXME: Need to turn off font smoothing.
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 3314da8ebcb9e00e53370c2091fe80976468f788..b2624160a7db57b0691e7ca3a4d40efe5e7ffc50 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,23 @@
>+2013-08-27  Rashmi Shyamasundar  <rashmi.s2@samsung.com>
>+
>+        Canvas fillText and measureText handle ideographic spaces differently
>+        https://bugs.webkit.org/show_bug.cgi?id=108881
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        According to the text preparation algorithm defined in the spec
>+        http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#text-preparation-algorithm
>+        all the spaces listed in the spec
>+        http://www.whatwg.org/specs/web-apps/current-work/multipage/common-microsyntaxes.html#space-character
>+        should be replaced by +U0020 space character.
>+        The below listed layout tests verify the conformance to above spec.
>+        These tests are passing on Ubuntu-Firefox 23.0.
>+
>+        * fast/canvas/canvas-fillText-ideographicSpace-expected.html: Added.
>+        * fast/canvas/canvas-fillText-ideographicSpace.html: Added.
>+        * fast/canvas/canvas-measureText-ideographicSpace-expected.txt: Added.
>+        * fast/canvas/canvas-measureText-ideographicSpace.html: Added.
>+
> 2013-08-27  Christophe Dumez  <ch.dumez@sisa.samsung.com>
> 
>         Implement DOM3 wheel event
>diff --git a/LayoutTests/fast/canvas/canvas-fillText-ideographicSpace-expected.html b/LayoutTests/fast/canvas/canvas-fillText-ideographicSpace-expected.html
>new file mode 100755
>index 0000000000000000000000000000000000000000..3e6082109b0ce6077d2cbc34a85022b155b45726
>--- /dev/null
>+++ b/LayoutTests/fast/canvas/canvas-fillText-ideographicSpace-expected.html
>@@ -0,0 +1,9 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+  <meta charcode="utf-16">
>+</head>
>+<body>
>+<pre style="padding: 0px; font-size: 12px; font-family: Arial; display: inline">a&#x3000;b&#x3000;c</pre>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/canvas/canvas-fillText-ideographicSpace.html b/LayoutTests/fast/canvas/canvas-fillText-ideographicSpace.html
>new file mode 100755
>index 0000000000000000000000000000000000000000..2be646140efb6db6e709457ec50b630a206141da
>--- /dev/null
>+++ b/LayoutTests/fast/canvas/canvas-fillText-ideographicSpace.html
>@@ -0,0 +1,20 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+  <meta charcode="utf-16">
>+  <script>
>+    document.addEventListener('DOMContentLoaded', function () {
>+      var canvas = document.getElementsByTagName('canvas')[0];
>+      var context = canvas.getContext('2d');
>+      var iSp = '\u3000';
>+      var str = 'a' + iSp + 'b' + iSp + 'c';
>+      context.font = '12px Arial';
>+      context.textBaseline = 'top';
>+      context.fillText(str, 0, 3);
>+    }, false);
>+  </script>
>+</head>
>+<body>
>+<canvas width="320" height="160"></canvas>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/canvas/canvas-measureText-ideographicSpace-expected.txt b/LayoutTests/fast/canvas/canvas-measureText-ideographicSpace-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..ef3ab475b17d06bc39a41943a4be87787936bde9
>--- /dev/null
>+++ b/LayoutTests/fast/canvas/canvas-measureText-ideographicSpace-expected.txt
>@@ -0,0 +1,5 @@
>+PASS spanWidth is canvasWidth
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+ 
>diff --git a/LayoutTests/fast/canvas/canvas-measureText-ideographicSpace.html b/LayoutTests/fast/canvas/canvas-measureText-ideographicSpace.html
>new file mode 100755
>index 0000000000000000000000000000000000000000..bf014a1c809ee0583b9f4e58a64049681bb5085f
>--- /dev/null
>+++ b/LayoutTests/fast/canvas/canvas-measureText-ideographicSpace.html
>@@ -0,0 +1,24 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+  <meta charcode="utf-16">
>+  <script src="../js/resources/js-test-pre.js"></script>
>+</head>
>+<body>
>+<canvas></canvas>
>+<span style="padding: 0px; font-size: 12px; font-family: Arial; display: inline; visibility: hidden">a&#x3000;b&#x3000;c</span>
>+<script>
>+    var canvas = document.getElementsByTagName('canvas')[0];
>+    var context = canvas.getContext('2d');
>+    var iSp = '\u3000';
>+    var str = 'a' + iSp + 'b' + iSp + 'c';
>+    context.font = '12px Arial';
>+    canvasWidth = context.measureText(str).width;
>+  
>+    var span = document.getElementsByTagName('span')[0];
>+    spanWidth = span.offsetWidth;
>+    shouldBe("spanWidth", "canvasWidth");
>+</script>
>+<script src="../js/resources/js-test-post.js"></script>
>+</body>
>+</html>
>diff --git a/LayoutTests/platform/mac/canvas/philip/tests/2d.text.measure.width.space-expected.txt b/LayoutTests/platform/mac/canvas/philip/tests/2d.text.measure.width.space-expected.txt
>index c4f229e6c239db4aa466e6b226320c9174cc84cc..af4ad0bb2ebc079f9f730ea3dcbbc30953e0c181 100644
>--- a/LayoutTests/platform/mac/canvas/philip/tests/2d.text.measure.width.space-expected.txt
>+++ b/LayoutTests/platform/mac/canvas/philip/tests/2d.text.measure.width.space-expected.txt
>@@ -1,4 +1,4 @@
> Failed assertion ctx.measureText('A B').width === 150 (got 200[number], expected 150[number])
>-Failed assertion ctx.measureText('A \x09\x0a\x0c\x0d \x09\x0a\x0c\x0dB').width === 150 (got 450[number], expected 150[number])
>+Failed assertion ctx.measureText('A \x09\x0a\x0c\x0d \x09\x0a\x0c\x0dB').width === 150 (got 650[number], expected 150[number])
> Failed assertion ctx.measureText(' AB').width === 100 (got 150[number], expected 100[number])
> Failed assertion ctx.measureText('AB ').width === 100 (got 150[number], expected 100[number])

Actions: View | Formatted Diff | Diff

Attachments on bug 108881: 186505 | 208243 | 208414 | 208421 | 208423 | 208427 | 208432 | 208514 | 208518 | 208523 | 208908 | 209259 | 209261 | 209284 | 209748 | 209760 | 209765 | 209768 | 209848 | 209852 | 211185