Changeset 76565 in webkit


Ignore:
Timestamp:
Jan 24, 2011 7:27:14 PM (13 years ago)
Author:
rniwa@webkit.org
Message:

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
Location:
trunk
Files:
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r76564 r76565  
     12011-01-24  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Ojan Vafai.
     4
     5        Inserting multiple whitespace using text composition (IME) should insert interleaved nbsp and whitespace.
     6        https://bugs.webkit.org/show_bug.cgi?id=52781
     7
     8        The test was rebaselined to have interleaved space and non-breaking space.
     9
     10        * editing/inserting/insert-composition-whitespace-expected.txt:
     11        * editing/inserting/insert-composition-whitespace.html:
     12
    1132011-01-24  Martin Robinson  <mrobinson@igalia.com>
    214
  • trunk/LayoutTests/editing/inserting/insert-composition-whitespace-expected.txt

    r76482 r76565  
    1111PASS compositingText is ' AB'
    1212PASS confirmedText is ' AB'
    13 PASS compositingText is '  AB'
    14 PASS confirmedText is '  AB'
    15 PASS compositingText is '   AB'
    16 PASS confirmedText is '   AB'
    17 PASS compositingText is '    AB'
    18 PASS confirmedText is '    AB'
    19 PASS compositingText is '     AB'
    20 PASS confirmedText is '     AB'
    21 PASS compositingText is '      AB'
    22 PASS confirmedText is '      AB'
    23 PASS compositingText is '       AB'
    24 PASS confirmedText is '       AB'
     13PASS compositingText is '  AB'
     14PASS confirmedText is '  AB'
     15PASS compositingText is '   AB'
     16PASS confirmedText is '   AB'
     17PASS compositingText is '    AB'
     18PASS confirmedText is '    AB'
     19PASS compositingText is '     AB'
     20PASS confirmedText is '     AB'
     21PASS compositingText is '      AB'
     22PASS confirmedText is '      AB'
     23PASS compositingText is '       AB'
     24PASS confirmedText is '       AB'
    2525PASS compositingText is 'AB  '
    2626PASS confirmedText is 'AB  '
     
    3535PASS compositingText is 'AB       '
    3636PASS confirmedText is 'AB       '
    37 PASS compositingText is '  A  B  '
    38 PASS confirmedText is '  A  B  '
    39 PASS compositingText is '  A  B  '
    40 PASS confirmedText is '  A  B  '
     37PASS compositingText is '  A  B  '
     38PASS confirmedText is '  A  B  '
     39PASS compositingText is '  A  B  '
     40PASS confirmedText is '  A  B  '
    4141PASS compositingText is ' '
    4242PASS confirmedText is ' '
    4343PASS compositingText is '  '
    4444PASS confirmedText is '  '
    45 PASS compositingText is '   '
    46 PASS confirmedText is '   '
     45PASS compositingText is '   '
     46PASS confirmedText is '   '
    4747PASS compositingText is 'AB'
    4848PASS confirmedText is 'AB'
  • trunk/LayoutTests/editing/inserting/insert-composition-whitespace.html

    r76482 r76565  
    4646
    4747test("div", " AB", "\xA0AB");
    48 test("div", "  AB", "\xA0\xA0AB");
    49 test("div", "   AB", "\xA0\xA0 AB");
    50 test("div", "    AB", "\xA0\xA0 \xA0AB");
    51 test("div", "     AB", "\xA0\xA0 \xA0 AB");
    52 test("div", "      AB", "\xA0\xA0 \xA0 \xA0AB");
    53 test("div", "       AB", "\xA0\xA0 \xA0 \xA0 AB");
     48test("div", "  AB", "\xA0 AB");
     49test("div", "   AB", "\xA0 \xA0AB");
     50test("div", "    AB", "\xA0 \xA0 AB");
     51test("div", "     AB", "\xA0 \xA0 \xA0AB");
     52test("div", "      AB", "\xA0 \xA0 \xA0 AB");
     53test("div", "       AB", "\xA0 \xA0 \xA0 \xA0AB");
    5454test("div", "AB  ", "AB \xA0");
    5555test("div", "AB   ", "AB \xA0\xA0");
     
    5959test("div", "AB       ", "AB \xA0 \xA0 \xA0\xA0");
    6060
    61 test("div", "  A  B  ", "\xA0\xA0A \xA0B \xA0");
    62 test("div", "\t\tA\t\tB\t\t", "\xA0\xA0A \xA0B \xA0");
     61test("div", "  A  B  ", "\xA0 A \xA0B \xA0");
     62test("div", "\t\tA\t\tB\t\t", "\xA0 A \xA0B \xA0");
    6363
    6464test("div", " ", "\xA0");
    6565test("div", "  ", "\xA0\xA0");
    66 // This is wrong. This should insert interleaved nbsp and whitespace. See https://bugs.webkit.org/show_bug.cgi?id=52781.
    67 test("div", "   ", "\xA0\xA0\xA0");
     66test("div", "   ", "\xA0 \xA0");
    6867
    6968test("pre", "AB", "AB");
  • trunk/Source/WebCore/ChangeLog

    r76562 r76565  
     12011-01-24  Ryosuke Niwa  <rniwa@webkit.org>
     2
     3        Reviewed by Ojan Vafai.
     4
     5        Inserting multiple whitespace using text composition (IME) should insert interleaved nbsp and whitespace.
     6        https://bugs.webkit.org/show_bug.cgi?id=52781
     7
     8        The bug was caused by stringWithRebalancedWhitespace's replacing the space at the beginning of a paragraph
     9        and the end of a paragraph by a non-breaking space after it replaced two consecutive spaces by a space and
     10        non-breaking space pattern, thereby replacing more spaces by non-breaking spaces than needed.
     11
     12        Rewrote the function using Vector<UChar> to fix the bug. New function no longer calls String::replace
     13        multiple times but instead it traverses through the string and replaces a space that immediately follows
     14        another space or appears at the beginning of a paragraph or at the end of a paragraph by a non-break space.
     15
     16        * editing/CompositeEditCommand.cpp:
     17        * editing/htmlediting.cpp:
     18        (WebCore::stringWithRebalancedWhitespace): Written.
     19        * editing/htmlediting.h:
     20        (WebCore::isWhitespace): Removed from CompositeEditCommand.cpp
     21
    1222011-01-24  Kenneth Russell  <kbr@google.com>
    223
  • trunk/Source/WebCore/editing/CompositeEditCommand.cpp

    r76482 r76565  
    391391}
    392392
    393 static inline bool isWhitespace(UChar c)
    394 {
    395     return c == noBreakSpace || c == ' ' || c == '\n' || c == '\t';
    396 }
    397 
    398393static inline bool containsOnlyWhitespace(const String& text)
    399394{
  • trunk/Source/WebCore/editing/htmlediting.cpp

    r76107 r76565  
    357357String stringWithRebalancedWhitespace(const String& string, bool startIsStartOfParagraph, bool endIsEndOfParagraph)
    358358{
    359     DEFINE_STATIC_LOCAL(String, twoSpaces, ("  "));
    360     DEFINE_STATIC_LOCAL(String, nbsp, ("\xa0"));
    361     DEFINE_STATIC_LOCAL(String, pattern, (" \xa0"));
    362 
    363     String rebalancedString = string;
    364 
    365     rebalancedString.replace(noBreakSpace, ' ');
    366     rebalancedString.replace('\n', ' ');
    367     rebalancedString.replace('\t', ' ');
    368    
    369     rebalancedString.replace(twoSpaces, pattern);
    370    
    371     if (startIsStartOfParagraph && rebalancedString[0] == ' ')
    372         rebalancedString.replace(0, 1, nbsp);
    373     int end = rebalancedString.length() - 1;
    374     if (endIsEndOfParagraph && rebalancedString[end] == ' ')
    375         rebalancedString.replace(end, 1, nbsp);   
    376 
    377     return rebalancedString;
     359    Vector<UChar> rebalancedString;
     360    append(rebalancedString, string);
     361
     362    bool previousCharacterWasSpace = false;
     363    for (size_t i = 0; i < rebalancedString.size(); i++) {
     364        if (!isWhitespace(rebalancedString[i])) {
     365            previousCharacterWasSpace = false;
     366            continue;
     367        }
     368
     369        if (previousCharacterWasSpace || (!i && startIsStartOfParagraph) || (i + 1 == rebalancedString.size() && endIsEndOfParagraph)) {
     370            rebalancedString[i] = noBreakSpace;
     371            previousCharacterWasSpace = false;
     372        } else {
     373            rebalancedString[i] = ' ';
     374            previousCharacterWasSpace = true;
     375        }
     376           
     377    }
     378
     379    return String::adopt(rebalancedString);
    378380}
    379381
  • trunk/Source/WebCore/editing/htmlediting.h

    r76107 r76565  
    2727#define htmlediting_h
    2828
     29#include "CharacterNames.h"
     30#include "ExceptionCode.h"
     31#include "HTMLNames.h"
     32#include "Position.h"
    2933#include <wtf/Forward.h>
    30 #include "HTMLNames.h"
    31 #include "ExceptionCode.h"
    32 #include "Position.h"
    3334
    3435namespace WebCore {
     
    232233   
    233234
    234 // Miscellaneous functions on String
    235    
     235// Miscellaneous functions on Text
     236inline bool isWhitespace(UChar c)
     237{
     238    return c == noBreakSpace || c == ' ' || c == '\n' || c == '\t';
     239}
    236240String stringWithRebalancedWhitespace(const String&, bool, bool);
    237241const String& nonBreakingSpaceString();
Note: See TracChangeset for help on using the changeset viewer.