Bug 51918 – contentEditable cannot handle multiple whitespace which is set as a composition text.

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: HTML Editing

[Hajime Morrita]

How to reproduce:
1. Open attached repro URL (it is a plain contentEditable div)
2. Place caret between "[" and "]"
3. Enable IME
4. input "AB" (by typing Shift-A, Shift-B)
5. move caret between A and B (by typing left-arrow)
6. input whitespace multiple times (by typing whitespace)

What is expected:
The string "A", followed by multiple whitespace, followed by "B" should be appeared as 
a composition text.

What happens instead:
The string "A", followed by single whitespace, followed by "B" is appeared.

Other browsers:
- Firefox accepts multiple whitespace characters
- On Windows, composition text is typically rendered by IME itself, (thus multiple whitespace is accepted)

Note: 
You can use Google Japanese Input (http://www.google.com/intl/ja/ime/index-mac.html) to 
reproduce this problem. You can input multiple whitespace between compositing text.
Kotoeri doesn't allow users to input such type of sequence.

[Ryosuke Niwa]

I'm not too familiar with how IME works on Mac but in other places in editing, we use interleaving sp and nbsp to render line-breakable multiple whitespace.

[Hajime Morrita]

Created attachment 78211 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 78211 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=78211&action=review

> LayoutTests/ChangeLog:7
> +

You probably want to explain what kind of test you're adding.

> WebCore/editing/CompositeEditCommand.cpp:433
> +String CompositeEditCommand::stringWithBalancedWhitespaceAt(const Position& position, const String& text)

Nit: the original function name "rebalanceWhitespaceAt" is better IMO.

> WebCore/editing/CompositeEditCommand.cpp:438
> +    String ret;

Nit: please don't use abbreviation.

> WebCore/editing/CompositeEditCommand.cpp:469
> +    for (unsigned i = 0; i < text.length(); ++i) {
> +        bool isWhitespace = text[i] == ' ';
> +        if (isWhitespace) {
> +            whiteCount++;
> +
> +            if (i + 1 < text.length())
> +                continue;
> +            // Preserves trailing whitespace, which will be balanced by rebalanceWhitespaceAt()
> +            ret.append(text.characters() + text.length() - whiteCount, whiteCount);
> +            break;
> +        }
> +
> +        // Preseves leading whitespace, which will be balanced by rebalanceWhitespaceAt()
> +        if (whiteCount == i)
> +            ret.append(text.characters(), whiteCount);
> +        else {
> +            // Interleaves nbsp and whitespace to preserve
> +            // space characters and line-breakability.
> +            if (1 == whiteCount)
> +                ret.append(' ');
> +            else {
> +                for (unsigned whiteIndex = 0; whiteIndex < whiteCount; ++whiteIndex)
> +                    ret.append(static_cast<UChar>(whiteIndex%2 ? noBreakSpace : ' '));
> +            }
> +        }
> +
> +        ret.append(text.characters()[i]);
> +        whiteCount = 0;
> +    }

Why aren't we calling stringWithRebalancedWhitespace in htmlediting.cpp?

> WebCore/editing/Editor.cpp:1604
> +    // Some IME pass sequence of whitespace ' ' instead of nbsp even they expect
> +    // multiple whitespaces inserted into the redering result. 
> +    // So we mix some nbsp characters for preserving the whitespace.

Nit: I don't think this comment is necessary.  Anyone who knows about whitespace rebalancing would understand what this code does.

> WebCore/editing/Editor.cpp:1696
> +        // Some IME pass sequence of whitespace ' ' instead of nbsp even they expect
> +        // multiple whitespaces inserted into the redering result. 
> +        // So we mix some nbsp characters for preserving the whitespace.

Ditto.

[Alexey Proskuryakov]

Duplicate of bug 5241?

[Ryosuke Niwa]

(In reply to comment #4)
> Duplicate of bug 5241?

So it seems.

[Hajime Morrita]

> > Duplicate of bug 5241?
> 
> So it seems.
Thanks to pointing this out. I'm closing this to mark as duplicated.
Update patch will come shortly to Bug 5241.

*** This bug has been marked as a duplicate of bug 5241 ***