Copy and paste sometimes removes spaces between words.

Copy and paste sometimes removes spaces between words.

The problem is StyledMarkupAccumulator uses renderedText and the space at the end of the text has been stripped when the tag after the text was wrapped.

BUG=318925
In the DOM tree, the copied content is represented by two nodes, one is "Copy this text " and the 2nd one is the anchor. When the code tries to convert the 1st node to plain text, it uses a text iterator to scan through the text. However, the spaces at the ending of the 1st node were lost because they were collapsed during rendering. This patch restores the missing space if it detects there is a non-text node following the last text node. This information has to be passed into the textIterator when it was constructed in StyledMarkupAccumulator::renderedText.

[c.shu, 2014-12-09 00:07:15]

c.shu@samsung.com changed reviewers:
+ yutak@chromium.org

[c.shu, 2014-12-09 00:07:16]

I come up with a new patch that fixes the bug 318925.
Comparing to my 1st attempt,
https://codereview.chromium.org/165083002/,
this patch utilizes the m_lastTextNodeEndedWithCollapsedSpace better and lets
the text iterator emit collapsed spaces automatically in the next call to
advance().

[c.shu, 2014-12-09 15:17:50]

c.shu@samsung.com changed reviewers:
+ tkent@chromium.org

[c.shu, 2014-12-11 20:48:46]

ping reviewers.

[tkent, 2014-12-11 21:17:14]

tkent@chromium.org changed reviewers:
- tkent@chromium.org

[tkent, 2014-12-11 21:17:40]

tkent@chromium.org changed reviewers:
+ yosin@chromium.org
- tkent@chromium.org

[tkent, 2014-12-11 21:18:37]

tkent@chromium.org changed reviewers:
+ yosin@chromium.org
- tkent@chromium.org

[tkent, 2014-12-12 00:23:03]

tkent@chromium.org changed reviewers:
+ yosin@chromium.org
- tkent@chromium.org

[tkent, 2014-12-12 00:23:03]

+yosin

[yosin_UTC9, 2015-01-14 01:46:20]

https://codereview.chromium.org/753573003/diff/1/Source/core/editing/TextIter...
File Source/core/editing/TextIterator.cpp (right):

https://codereview.chromium.org/753573003/diff/1/Source/core/editing/TextIter...
Source/core/editing/TextIterator.cpp:271: , m_hasNodesFollowing(behavior &
TextIteratorBehavesAsIfNodesFollowing)
Do we need to have new behavior? It seems the issue is caused by word wrapping
and all call sites want to have a space at wrapped position.

https://codereview.chromium.org/753573003/diff/1/Source/core/editing/TextIter...
Source/core/editing/TextIterator.cpp:749: if
(m_lastTextNodeEndedWithCollapsedSpace && !nextTextBox && m_hasNodesFollowing) {
Sorry, I'm not sure why newline character is a special case.

[c.shu, 2015-01-14 21:13:41]

https://codereview.chromium.org/753573003/diff/1/Source/core/editing/TextIter...
File Source/core/editing/TextIterator.cpp (right):

https://codereview.chromium.org/753573003/diff/1/Source/core/editing/TextIter...
Source/core/editing/TextIterator.cpp:271: , m_hasNodesFollowing(behavior &
TextIteratorBehavesAsIfNodesFollowing)
On 2015/01/14 01:46:20, Yosi_UTC9 wrote:
> Do we need to have new behavior? It seems the issue is caused by word wrapping
> and all call sites want to have a space at wrapped position.

The bug is not a general issue. It only shows up when the text node is followed
by a non-text node. See function renderedText(...) in mark.cpp. The new behavior
is set when the textNode is not range->endContainer(). This info is lost when
constructing a new Range using textNode with startOffset and endOffset. So we
have to pass it through the constructor.

https://codereview.chromium.org/753573003/diff/1/Source/core/editing/TextIter...
Source/core/editing/TextIterator.cpp:749: if
(m_lastTextNodeEndedWithCollapsedSpace && !nextTextBox && m_hasNodesFollowing) {
On 2015/01/14 01:46:20, Yosi_UTC9 wrote:
> Sorry, I'm not sure why newline character is a special case.

We don't want to restore the space for newline as it's taken care of already in
the exiting code. Otherwise, we see extra space in the paste.