2011-01-24 Ryosuke Niwa <rniwa@webkit.org>

        Reviewed by Ojan Vafai.

        Inserting multiple whitespace using text composition (IME) should insert interleaved nbsp and whitespace.
        https://bugs.webkit.org/show_bug.cgi?id=52781

        The test was rebaselined to have interleaved space and non-breaking space.

        * editing/inserting/insert-composition-whitespace-expected.txt:
        * editing/inserting/insert-composition-whitespace.html:
2011-01-24  Ryosuke Niwa  <rniwa@webkit.org>

        Reviewed by Ojan Vafai.

        Inserting multiple whitespace using text composition (IME) should insert interleaved nbsp and whitespace.
        https://bugs.webkit.org/show_bug.cgi?id=52781

        The bug was caused by stringWithRebalancedWhitespace's replacing the space at the beginning of a paragraph
        and the end of a paragraph by a non-breaking space after it replaced two consecutive spaces by a space and
        non-breaking space pattern, thereby replacing more spaces by non-breaking spaces than needed.

        Rewrote the function using Vector<UChar> to fix the bug. New function no longer calls String::replace
        multiple times but instead it traverses through the string and replaces a space that immediately follows
        another space or appears at the beginning of a paragraph or at the end of a paragraph by a non-break space.

        * editing/CompositeEditCommand.cpp:
        * editing/htmlediting.cpp:
        (WebCore::stringWithRebalancedWhitespace): Written.
        * editing/htmlediting.h:
        (WebCore::isWhitespace): Removed from CompositeEditCommand.cpp

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index e9662f7..6095d29 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2011-01-24  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Ojan Vafai.
+
+        Inserting multiple whitespace using text composition (IME) should insert interleaved nbsp and whitespace.
+        https://bugs.webkit.org/show_bug.cgi?id=52781
+
+        The test was rebaselined to have interleaved space and non-breaking space.
+
+        * editing/inserting/insert-composition-whitespace-expected.txt:
+        * editing/inserting/insert-composition-whitespace.html:
+
 2011-01-24  Martin Robinson  <mrobinson@igalia.com>
 
         Skip one more crashing test.

diff --git a/LayoutTests/editing/inserting/insert-composition-whitespace-expected.txt b/LayoutTests/editing/inserting/insert-composition-whitespace-expected.txt
index ade0725..103e18b 100644 (file)
--- a/LayoutTests/editing/inserting/insert-composition-whitespace-expected.txt
+++ b/LayoutTests/editing/inserting/insert-composition-whitespace-expected.txt
@@ -10,18 +10,18 @@ PASS compositingText is 'A    B'
 PASS confirmedText is 'A    B'
 PASS compositingText is ' AB'
 PASS confirmedText is ' AB'
-PASS compositingText is '  AB'
-PASS confirmedText is '  AB'
-PASS compositingText is '   AB'
-PASS confirmedText is '   AB'
-PASS compositingText is '    AB'
-PASS confirmedText is '    AB'
-PASS compositingText is '     AB'
-PASS confirmedText is '     AB'
-PASS compositingText is '      AB'
-PASS confirmedText is '      AB'
-PASS compositingText is '       AB'
-PASS confirmedText is '       AB'
+PASS compositingText is '  AB'
+PASS confirmedText is '  AB'
+PASS compositingText is '   AB'
+PASS confirmedText is '   AB'
+PASS compositingText is '    AB'
+PASS confirmedText is '    AB'
+PASS compositingText is '     AB'
+PASS confirmedText is '     AB'
+PASS compositingText is '      AB'
+PASS confirmedText is '      AB'
+PASS compositingText is '       AB'
+PASS confirmedText is '       AB'
 PASS compositingText is 'AB  '
 PASS confirmedText is 'AB  '
 PASS compositingText is 'AB   '
@@ -34,16 +34,16 @@ PASS compositingText is 'AB      '
 PASS confirmedText is 'AB      '
 PASS compositingText is 'AB       '
 PASS confirmedText is 'AB       '
-PASS compositingText is '  A  B  '
-PASS confirmedText is '  A  B  '
-PASS compositingText is '  A  B  '
-PASS confirmedText is '  A  B  '
+PASS compositingText is '  A  B  '
+PASS confirmedText is '  A  B  '
+PASS compositingText is '  A  B  '
+PASS confirmedText is '  A  B  '
 PASS compositingText is ' '
 PASS confirmedText is ' '
 PASS compositingText is '  '
 PASS confirmedText is '  '
-PASS compositingText is '   '
-PASS confirmedText is '   '
+PASS compositingText is '   '
+PASS confirmedText is '   '
 PASS compositingText is 'AB'
 PASS confirmedText is 'AB'
 PASS compositingText is 'A B'

diff --git a/LayoutTests/editing/inserting/insert-composition-whitespace.html b/LayoutTests/editing/inserting/insert-composition-whitespace.html
index 81215dd..9dd147a 100644 (file)
--- a/LayoutTests/editing/inserting/insert-composition-whitespace.html
+++ b/LayoutTests/editing/inserting/insert-composition-whitespace.html
@@ -45,12 +45,12 @@ test("div", "A   B", "A \xA0 B");
 test("div", "A    B", "A \xA0 \xA0B"); 
 
 test("div", " AB", "\xA0AB");
-test("div", "  AB", "\xA0\xA0AB");
-test("div", "   AB", "\xA0\xA0 AB");
-test("div", "    AB", "\xA0\xA0 \xA0AB");
-test("div", "     AB", "\xA0\xA0 \xA0 AB");
-test("div", "      AB", "\xA0\xA0 \xA0 \xA0AB");
-test("div", "       AB", "\xA0\xA0 \xA0 \xA0 AB");
+test("div", "  AB", "\xA0 AB");
+test("div", "   AB", "\xA0 \xA0AB");
+test("div", "    AB", "\xA0 \xA0 AB");
+test("div", "     AB", "\xA0 \xA0 \xA0AB");
+test("div", "      AB", "\xA0 \xA0 \xA0 AB");
+test("div", "       AB", "\xA0 \xA0 \xA0 \xA0AB");
 test("div", "AB  ", "AB \xA0");
 test("div", "AB   ", "AB \xA0\xA0");
 test("div", "AB    ", "AB \xA0 \xA0");
@@ -58,13 +58,12 @@ test("div", "AB     ", "AB \xA0 \xA0\xA0");
 test("div", "AB      ", "AB \xA0 \xA0 \xA0");
 test("div", "AB       ", "AB \xA0 \xA0 \xA0\xA0");
 
-test("div", "  A  B  ", "\xA0\xA0A \xA0B \xA0");
-test("div", "\t\tA\t\tB\t\t", "\xA0\xA0A \xA0B \xA0");
+test("div", "  A  B  ", "\xA0 A \xA0B \xA0");
+test("div", "\t\tA\t\tB\t\t", "\xA0 A \xA0B \xA0");
 
 test("div", " ", "\xA0");
 test("div", "  ", "\xA0\xA0");
-// This is wrong. This should insert interleaved nbsp and whitespace. See https://bugs.webkit.org/show_bug.cgi?id=52781.
-test("div", "   ", "\xA0\xA0\xA0");
+test("div", "   ", "\xA0 \xA0");
 
 test("pre", "AB", "AB");
 test("pre", "A B", "A B");

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 053094c..5f3fb6c 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,24 @@
+2011-01-24  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Ojan Vafai.
+
+        Inserting multiple whitespace using text composition (IME) should insert interleaved nbsp and whitespace.
+        https://bugs.webkit.org/show_bug.cgi?id=52781
+
+        The bug was caused by stringWithRebalancedWhitespace's replacing the space at the beginning of a paragraph
+        and the end of a paragraph by a non-breaking space after it replaced two consecutive spaces by a space and
+        non-breaking space pattern, thereby replacing more spaces by non-breaking spaces than needed.
+
+        Rewrote the function using Vector<UChar> to fix the bug. New function no longer calls String::replace
+        multiple times but instead it traverses through the string and replaces a space that immediately follows
+        another space or appears at the beginning of a paragraph or at the end of a paragraph by a non-break space.
+
+        * editing/CompositeEditCommand.cpp:
+        * editing/htmlediting.cpp:
+        (WebCore::stringWithRebalancedWhitespace): Written.
+        * editing/htmlediting.h:
+        (WebCore::isWhitespace): Removed from CompositeEditCommand.cpp
+
 2011-01-24  Kenneth Russell  <kbr@google.com>
 
         Reviewed by James Robinson.

diff --git a/Source/WebCore/editing/CompositeEditCommand.cpp b/Source/WebCore/editing/CompositeEditCommand.cpp
index 39280d4..bcac20e 100644 (file)
--- a/Source/WebCore/editing/CompositeEditCommand.cpp
+++ b/Source/WebCore/editing/CompositeEditCommand.cpp
@@ -390,11 +390,6 @@ void CompositeEditCommand::setNodeAttribute(PassRefPtr<Element> element, const Q
     applyCommandToComposite(SetNodeAttributeCommand::create(element, attribute, value));
 }
 
-static inline bool isWhitespace(UChar c)
-{
-    return c == noBreakSpace || c == ' ' || c == '\n' || c == '\t';
-}
-
 static inline bool containsOnlyWhitespace(const String& text)
 {
     for (unsigned i = 0; i < text.length(); ++i) {

diff --git a/Source/WebCore/editing/htmlediting.cpp b/Source/WebCore/editing/htmlediting.cpp
index 90db3ef..fba560a 100644 (file)
--- a/Source/WebCore/editing/htmlediting.cpp
+++ b/Source/WebCore/editing/htmlediting.cpp
@@ -356,25 +356,27 @@ int lastOffsetForEditing(const Node* node)
 
 String stringWithRebalancedWhitespace(const String& string, bool startIsStartOfParagraph, bool endIsEndOfParagraph)
 {
-    DEFINE_STATIC_LOCAL(String, twoSpaces, ("  "));
-    DEFINE_STATIC_LOCAL(String, nbsp, ("\xa0"));
-    DEFINE_STATIC_LOCAL(String, pattern, (" \xa0"));
+    Vector<UChar> rebalancedString;
+    append(rebalancedString, string);
 
-    String rebalancedString = string;
+    bool previousCharacterWasSpace = false;
+    for (size_t i = 0; i < rebalancedString.size(); i++) {
+        if (!isWhitespace(rebalancedString[i])) {
+            previousCharacterWasSpace = false;
+            continue;
+        }
 
-    rebalancedString.replace(noBreakSpace, ' ');
-    rebalancedString.replace('\n', ' ');
-    rebalancedString.replace('\t', ' ');
-    
-    rebalancedString.replace(twoSpaces, pattern);
-    
-    if (startIsStartOfParagraph && rebalancedString[0] == ' ')
-        rebalancedString.replace(0, 1, nbsp);
-    int end = rebalancedString.length() - 1;
-    if (endIsEndOfParagraph && rebalancedString[end] == ' ')
-        rebalancedString.replace(end, 1, nbsp);    
+        if (previousCharacterWasSpace || (!i && startIsStartOfParagraph) || (i + 1 == rebalancedString.size() && endIsEndOfParagraph)) {
+            rebalancedString[i] = noBreakSpace;
+            previousCharacterWasSpace = false;
+        } else {
+            rebalancedString[i] = ' ';
+            previousCharacterWasSpace = true;
+        }
+            
+    }
 
-    return rebalancedString;
+    return String::adopt(rebalancedString);
 }
 
 bool isTableStructureNode(const Node *node)

diff --git a/Source/WebCore/editing/htmlediting.h b/Source/WebCore/editing/htmlediting.h
index 0208dfb..98fdf66 100644 (file)
--- a/Source/WebCore/editing/htmlediting.h
+++ b/Source/WebCore/editing/htmlediting.h
@@ -26,10 +26,11 @@
 #ifndef htmlediting_h
 #define htmlediting_h
 
-#include <wtf/Forward.h>
-#include "HTMLNames.h"
+#include "CharacterNames.h"
 #include "ExceptionCode.h"
+#include "HTMLNames.h"
 #include "Position.h"
+#include <wtf/Forward.h>
 
 namespace WebCore {
 
@@ -231,8 +232,11 @@ VisibleSelection avoidIntersectionWithNode(const VisibleSelection&, Node*);
 VisibleSelection selectionForParagraphIteration(const VisibleSelection&);
     
 
-// Miscellaneous functions on String
-    
+// Miscellaneous functions on Text
+inline bool isWhitespace(UChar c)
+{
+    return c == noBreakSpace || c == ' ' || c == '\n' || c == '\t';
+}
 String stringWithRebalancedWhitespace(const String&, bool, bool);
 const String& nonBreakingSpaceString();