Do not count extra trailing space for the last word in a range

Do not count extra trailing space for the last word in a range

Sometimes when a text selection ends with the last word on a line
(for instance when just one word is selected) the generated text
for the selection range contains an extra trailing space.
This happens when a text node is split into several InlineTextBoxes
(e.g. a <span> inside a narrow table) and the last selected word
happens to be the last one in such text box.

The TextIterator algorithm restores the spaces between the text boxes
and will add a trailing space for this word. For the SurroundingText,
however, we need the length that corresponds to the visible selection.

This CL introduces a new text iterator behavior that prevents that
extra space to be added. This behavior is used in combination with
DefaultRangeLengthBehavior() for the selection range calculation
in the SurroundingText.

BUG=708329
Review-Url: https://codereview.chromium.org/2832123002
Cr-Commit-Position: refs/heads/master@{#467538}
Committed: https://chromium.googlesource.com/chromium/src/+/9305554380ee3a02c680009b5dee4b9e82cba334

[Tima Vaisburd, 2017-04-21 02:03:51]

The CQ bit was checked by timav@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-21 02:04:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tima Vaisburd, 2017-04-21 02:16:13]

timav@chromium.org changed reviewers:
+ aelias@chromium.org, amaralp@chromium.org, yosin@google.com

[Tima Vaisburd, 2017-04-21 02:16:14]

This fix is needed for the Smart Text selection feature in Android. PTAL.

https://codereview.chromium.org/2832123002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp (right):

https://codereview.chromium.org/2832123002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp:788: if
(behavior_.CollapseTrailingSpace()) {
I'm not sure whether CollapseTrailingSpace is the right flag, and in general
whether we need to protect the change in the algorithm like this.

The existing behavior looks like a bug, but the comment on l.800 indicates that
maybe not. Please advise.

[Tima Vaisburd, 2017-04-21 02:16:56]

Description was changed from

==========
Remove artificial tailing space for the last word on a line

Sometimes if the last word on a line is selected with a Range,
the TextIterator on this range generates an extra trailing space.
This happens when one text node is split into several InlineTextBoxes
(e.g. a <span> inside a narrow table) and the selected word happens
to be the last one in such text box.

The CL prevents the extra whitespace generation by comparing the current
position inside a text box to the range end and stopping the iteration
early.

This change in the algorithm is protected by CollapseTrailingSpace() in
TextIteratorBehavior. The CL turns the behavior on for the retrieval
of surrounding text, but not in general case.

BUG=708329
==========

to

==========
Remove artificial trailing space for the last word on a line

Sometimes if the last word on a line is selected with a Range,
the TextIterator on this range generates an extra trailing space.
This happens when one text node is split into several InlineTextBoxes
(e.g. a <span> inside a narrow table) and the selected word happens
to be the last one in such text box.

The CL prevents the extra whitespace generation by comparing the current
position inside a text box to the range end and stopping the iteration
early.

This change in the algorithm is protected by CollapseTrailingSpace() in
TextIteratorBehavior. The CL turns the behavior on for the retrieval
of surrounding text, but not in general case.

BUG=708329
==========

[aelias_OOO_until_Jul13, 2017-04-21 02:18:25]

Description was changed from

==========
Remove artificial trailing space for the last word on a line

Sometimes if the last word on a line is selected with a Range,
the TextIterator on this range generates an extra trailing space.
This happens when one text node is split into several InlineTextBoxes
(e.g. a <span> inside a narrow table) and the selected word happens
to be the last one in such text box.

The CL prevents the extra whitespace generation by comparing the current
position inside a text box to the range end and stopping the iteration
early.

This change in the algorithm is protected by CollapseTrailingSpace() in
TextIteratorBehavior. The CL turns the behavior on for the retrieval
of surrounding text, but not in general case.

BUG=708329
==========

to

==========
Remove artificial tailing space for the last word on a line

Sometimes if the last word on a line is selected with a Range,
the TextIterator on this range generates an extra trailing space.
This happens when one text node is split into several InlineTextBoxes
(e.g. a <span> inside a narrow table) and the selected word happens
to be the last one in such text box.

The CL prevents the extra whitespace generation by comparing the current
position inside a text box to the range end and stopping the iteration
early.

This change in the algorithm is protected by CollapseTrailingSpace() in
TextIteratorBehavior. The CL turns the behavior on for the retrieval
of surrounding text, but not in general case.

BUG=708329
==========

[rlanday, 2017-04-21 03:20:04]

rlanday@chromium.org changed reviewers:
+ rlanday@chromium.org

[rlanday, 2017-04-21 03:20:05]

https://codereview.chromium.org/2832123002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp (right):

https://codereview.chromium.org/2832123002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp:788: if
(behavior_.CollapseTrailingSpace()) {
On 2017/04/21 at 02:16:14, Tima Vaisburd wrote:
> I'm not sure whether CollapseTrailingSpace is the right flag, and in general
whether we need to protect the change in the algorithm like this.
> 
> The existing behavior looks like a bug, but the comment on l.800 indicates
that maybe not. Please advise.

I suspect the problem is that Position doesn't specify whether the cursor is
supposed to be interpreted as being at the end of one line or the beginning of
the next one (you need VisiblePosition for that). So if you're constructing an
EphemeralRange from two positions, TextIterator might be interpreting the
Position immediately after the last character in the word as being at the start
of the next line, and adding a space corresponding to the line break (and then
it would ignore the actual space if the range kept going, since it already added
one for the line break), although that seems incorrect since the range of marked
text didn't actually include a space. Really the space should probably only be
emitted if the range actually includes the space after the word.

I'm not sure what the cleanest fix is, but it seems like this is a bug and
shouldn't have to go behind any sort of behavioral flag.

[commit-bot: I haz the power, 2017-04-21 03:57:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-21 03:57:01]

Dry run: This issue passed the CQ dry run.

[Tima Vaisburd, 2017-04-21 20:03:18]

Patchset #2 (id:20001) has been deleted

[Tima Vaisburd, 2017-04-21 20:15:19]

Description was changed from

==========
Remove artificial tailing space for the last word on a line

Sometimes if the last word on a line is selected with a Range,
the TextIterator on this range generates an extra trailing space.
This happens when one text node is split into several InlineTextBoxes
(e.g. a <span> inside a narrow table) and the selected word happens
to be the last one in such text box.

The CL prevents the extra whitespace generation by comparing the current
position inside a text box to the range end and stopping the iteration
early.

This change in the algorithm is protected by CollapseTrailingSpace() in
TextIteratorBehavior. The CL turns the behavior on for the retrieval
of surrounding text, but not in general case.

BUG=708329
==========

to

==========
Remove extra trailing space for the last word in a range

Sometimes if the selection Range ends with the last word on a line
(for instance when just one word is selected) the generated text
for this range contains an extra trailing space.
This happens when one text node is split into several InlineTextBoxes
(e.g. a <span> inside a narrow table) and the last word happens
to be the last one in such text box.

Currently the TextIterator algorithm restores the spaces between the
text boxes but does not check whether the last word in the text box is
actually the last one in the range, in which case there should be no
extra space.

This CL adds the corresponding check.

BUG=708329
==========

[Tima Vaisburd, 2017-04-21 20:19:29]

On 2017/04/21 03:20:05, rlanday wrote:

> I suspect the problem is that Position doesn't specify whether the cursor is
> supposed to be interpreted as being at the end of one line or the beginning of
> the next one (you need VisiblePosition for that). So if you're constructing an
> EphemeralRange from two positions, TextIterator might be interpreting the
> Position immediately after the last character in the word as being at the
start
> of the next line, and adding a space corresponding to the line break (and then
> it would ignore the actual space if the range kept going, since it already
added
> one for the line break), although that seems incorrect since the range of
marked
> text didn't actually include a space.
>
> I'm not sure what the cleanest fix is, but it seems like this is a bug and
> shouldn't have to go behind any sort of behavioral flag.

This greatly simplifies the CL. I also updated the description after I got a
better
understanding of the problem.

> Really the space should probably only be
> emitted if the range actually includes the space after the word.

I updated the test and included this case. PTAL again?

[aelias_OOO_until_Jul13, 2017-04-21 20:20:52]

The CQ bit was checked by aelias@chromium.org to run a CQ dry run

[Tima Vaisburd, 2017-04-21 20:21:09]

The CQ bit was checked by timav@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-21 20:21:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tima Vaisburd, 2017-04-21 20:21:14]

The CQ bit was checked by timav@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-21 20:21:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-21 21:10:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-21 21:10:53]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Tima Vaisburd, 2017-04-21 23:16:27]

On 2017/04/21 20:19:29, Tima Vaisburd wrote:
> On 2017/04/21 03:20:05, rlanday wrote:
> 
> > I suspect the problem is that Position doesn't specify whether the cursor is
> > supposed to be interpreted as being at the end of one line or the beginning
of
> > the next one (you need VisiblePosition for that). So if you're constructing
an
> > EphemeralRange from two positions, TextIterator might be interpreting the
> > Position immediately after the last character in the word as being at the
> start
> > of the next line, and adding a space corresponding to the line break (and
then
> > it would ignore the actual space if the range kept going, since it already
> added
> > one for the line break), although that seems incorrect since the range of
> marked
> > text didn't actually include a space.
> >
> > I'm not sure what the cleanest fix is, but it seems like this is a bug and
> > shouldn't have to go behind any sort of behavioral flag.
> 
> This greatly simplifies the CL. I also updated the description after I got a
> better
> understanding of the problem.
> 
> > Really the space should probably only be
> > emitted if the range actually includes the space after the word.
> 
> I updated the test and included this case. PTAL again?

It did not work, apparently the content of each text box is visible to JS and
the tests
concatenate them and expect to reassemble the original text.

Would VisiblePosition help me in calculation of the proper selected text length?

[Tima Vaisburd, 2017-04-22 01:57:18]

The CQ bit was checked by timav@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-22 01:57:53]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tima Vaisburd, 2017-04-22 02:16:35]

Description was changed from

==========
Remove extra trailing space for the last word in a range

Sometimes if the selection Range ends with the last word on a line
(for instance when just one word is selected) the generated text
for this range contains an extra trailing space.
This happens when one text node is split into several InlineTextBoxes
(e.g. a <span> inside a narrow table) and the last word happens
to be the last one in such text box.

Currently the TextIterator algorithm restores the spaces between the
text boxes but does not check whether the last word in the text box is
actually the last one in the range, in which case there should be no
extra space.

This CL adds the corresponding check.

BUG=708329
==========

to

==========
Do not count extra trailing space for the last word in a range

Sometimes when a text selection ends with the last word on a line
(for instance when just one word is selected) the generated text
for the selection range contains an extra trailing space.
This happens when a text node is split into several InlineTextBoxes
(e.g. a <span> inside a narrow table) and the last selected word
happens to be the last one in such text box.

The TextIterator algorithm restores the spaces between the text boxes
and will add a trailing space for this word. For the SurroundingText,
however, we need the length that corresponds to the visible selection.

This CL introduces a new text iterator behavior that prevents that
extra space to be added. This behavior may be turned on in the method
TextIterator::RangeLength() which is now used for surrounding text.

BUG=708329
==========

[Tima Vaisburd, 2017-04-22 02:41:08]

timav@chromium.org changed reviewers:
+ yosin@chromium.org
- yosin@google.com

[Tima Vaisburd, 2017-04-22 02:41:09]

I fixed the e-mail address for Yoshifumi, please accept my apology in case you
did not see this CL when it was first submitted.

[Tima Vaisburd, 2017-04-22 02:47:35]

> Would VisiblePosition help me in calculation of the proper
> selected text length?

Answering to myself: as far as can get after reading the code and doing some
experiments the VisiblePosition participates in algorithms that modify the
range, but in this case the range was correct from the start, it is the
generated text/length was calculated not as expected for the case.

I'm also not sure that the current behavior is a bug because several layout
tests seem to rely on it.

Therefore I added a new behavior and modified TextIterator::RangeLength().

[commit-bot: I haz the power, 2017-04-22 03:22:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-22 03:22:20]

Dry run: This issue passed the CQ dry run.

[yosin_UTC9, 2017-04-24 03:42:48]

https://codereview.chromium.org/2832123002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp (right):

https://codereview.chromium.org/2832123002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp:787: if
(behavior_.ForVisibleSelection()) {
Could you pick up another name? Sorry, I don't have good option.
But "ForVisibleSelection" doesn't reflect what this flag controls.

https://codereview.chromium.org/2832123002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/iterators/TextIterator.h (right):

https://codereview.chromium.org/2832123002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/iterators/TextIterator.h:113: bool
for_visible_selection = false);
I think we don't need to rename this parameter, since this parameter isn't for
VisibleSeleciton.

https://codereview.chromium.org/2832123002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp
(right):

https://codereview.chromium.org/2832123002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp:478:
ASSERT_NE(span_node, nullptr);
nit: We don't need to have this ASSERT, since |span_node->firstChild()| causes
null reference.

https://codereview.chromium.org/2832123002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp:480:
ASSERT_NE(text_node, nullptr);
nit: We don't need to have this ASSERT, |Position| ctor has DCHECK for |nullptr|
+ non-zero offset.

https://codereview.chromium.org/2832123002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp:483:
Position start = Position(text_node, 4);
nit: s/Position start/const Position start(text_node, 4)/

https://codereview.chromium.org/2832123002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp:484:
Position end = Position(text_node, 7);
nit: s/Position end/const Position end(text_node, 4)/

[Tima Vaisburd, 2017-04-24 20:13:50]

The CQ bit was checked by timav@chromium.org to run a CQ dry run

[Tima Vaisburd, 2017-04-24 20:14:55]

https://codereview.chromium.org/2832123002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp (right):

https://codereview.chromium.org/2832123002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp:787: if
(behavior_.ForVisibleSelection()) {
On 2017/04/24 03:42:48, yosin_UTC9 wrote:
> Could you pick up another name? Sorry, I don't have good option.
> But "ForVisibleSelection" doesn't reflect what this flag controls.

I'm not very good at naming. Changed to DoesNotEmitSpaceBeyondRangeEnd().

https://codereview.chromium.org/2832123002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp
(right):

https://codereview.chromium.org/2832123002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp:478:
ASSERT_NE(span_node, nullptr);
On 2017/04/24 03:42:48, yosin_UTC9 wrote:
> nit: We don't need to have this ASSERT, since |span_node->firstChild()| causes
> null reference.

Done.

https://codereview.chromium.org/2832123002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp:480:
ASSERT_NE(text_node, nullptr);
On 2017/04/24 03:42:48, yosin_UTC9 wrote:
> nit: We don't need to have this ASSERT, |Position| ctor has DCHECK for
|nullptr|
> + non-zero offset.

Done.

https://codereview.chromium.org/2832123002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp:483:
Position start = Position(text_node, 4);
On 2017/04/24 03:42:48, yosin_UTC9 wrote:
> nit: s/Position start/const Position start(text_node, 4)/

Done.

https://codereview.chromium.org/2832123002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/iterators/TextIteratorTest.cpp:484:
Position end = Position(text_node, 7);
On 2017/04/24 03:42:48, yosin_UTC9 wrote:
> nit: s/Position end/const Position end(text_node, 4)/

Done.

[commit-bot: I haz the power, 2017-04-24 20:14:59]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-24 21:41:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-24 21:41:02]

Dry run: This issue passed the CQ dry run.

[yosin_UTC9, 2017-04-25 02:09:12]

https://codereview.chromium.org/2832123002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/editing/iterators/TextIterator.h (right):

https://codereview.chromium.org/2832123002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/editing/iterators/TextIterator.h:113: bool
for_visible_selection = false);
Sorry, I misunderstanding this change.
You want to add new |bool| parameter.
Since we don't want to use |bool| parameter.
Let's make |RangeLength()| to |TextIteratorBehavior| within two patches:
1. Introduce static member |TextIteratorBehavior::DefaultRangeLengthBehavior|
with .SetEmitsObjectReplacementCharacter(true)
as default parameter of |RangeLength()|
2. Introduce DoesNotEmitSpaceBeyondRange to |TextIteratorBehavior|
 almost this patch except for step 1.

[Tima Vaisburd, 2017-04-25 02:53:16]

On 2017/04/25 02:09:12, yosin_UTC9 wrote:
>
https://codereview.chromium.org/2832123002/diff/100001/third_party/WebKit/Sou...
> File third_party/WebKit/Source/core/editing/iterators/TextIterator.h (right):
> 
>
https://codereview.chromium.org/2832123002/diff/100001/third_party/WebKit/Sou...
> third_party/WebKit/Source/core/editing/iterators/TextIterator.h:113: bool
> for_visible_selection = false);
> Sorry, I misunderstanding this change.
> You want to add new |bool| parameter.
> Since we don't want to use |bool| parameter.
> Let's make |RangeLength()| to |TextIteratorBehavior| within two patches:
> 1. Introduce static member |TextIteratorBehavior::DefaultRangeLengthBehavior|
> with .SetEmitsObjectReplacementCharacter(true)
> as default parameter of |RangeLength()|
> 2. Introduce DoesNotEmitSpaceBeyondRange to |TextIteratorBehavior|
>  almost this patch except for step 1.

Sorry, two patches: would you like to see two different CLs or just two steps in
this one?

[Tima Vaisburd, 2017-04-25 02:55:25]

On 2017/04/25 02:53:16, Tima Vaisburd wrote:
> Sorry, two patches: would you like to see two different CLs or just two steps
in
> this one?

Never mind, responded too fast

[yosin_UTC9, 2017-04-25 06:28:09]

On 2017/04/25 at 02:55:25, timav wrote:
> On 2017/04/25 02:53:16, Tima Vaisburd wrote:
> > Sorry, two patches: would you like to see two different CLs or just two
steps in
> > this one?
> 
> Never mind, responded too fast

Re-phase:

Patch 1: Replace RangeLength()'s bool parameter to TextIteratorBehavior; no
behavior changes
 - introduce static member |TextIteratorBehavior::DefaultRangeLengthBehavior()|
sets EmitsObjectReplacementCharacter

Patch 2: Add DoesNotEmitSpaceBeyondRange to |TextIteratorBehavior| and utilize
it

[Tima Vaisburd, 2017-04-26 05:40:41]

Description was changed from

==========
Do not count extra trailing space for the last word in a range

Sometimes when a text selection ends with the last word on a line
(for instance when just one word is selected) the generated text
for the selection range contains an extra trailing space.
This happens when a text node is split into several InlineTextBoxes
(e.g. a <span> inside a narrow table) and the last selected word
happens to be the last one in such text box.

The TextIterator algorithm restores the spaces between the text boxes
and will add a trailing space for this word. For the SurroundingText,
however, we need the length that corresponds to the visible selection.

This CL introduces a new text iterator behavior that prevents that
extra space to be added. This behavior may be turned on in the method
TextIterator::RangeLength() which is now used for surrounding text.

BUG=708329
==========

to

==========
Do not count extra trailing space for the last word in a range

Sometimes when a text selection ends with the last word on a line
(for instance when just one word is selected) the generated text
for the selection range contains an extra trailing space.
This happens when a text node is split into several InlineTextBoxes
(e.g. a <span> inside a narrow table) and the last selected word
happens to be the last one in such text box.

The TextIterator algorithm restores the spaces between the text boxes
and will add a trailing space for this word. For the SurroundingText,
however, we need the length that corresponds to the visible selection.

This CL introduces a new text iterator behavior that prevents that
extra space to be added. This behavior is used in combination with
DefaultRangeLengthBehavior() for the selection range calculation
in the SurroundingText.

BUG=708329
==========

[Tima Vaisburd, 2017-04-26 05:44:42]

On 2017/04/25 06:28:09, yosin_UTC9 wrote:
> On 2017/04/25 at 02:55:25, timav wrote:
> > On 2017/04/25 02:53:16, Tima Vaisburd wrote:
> > > Sorry, two patches: would you like to see two different CLs or just two
> steps in
> > > this one?
> > 
> > Never mind, responded too fast
> 
> Re-phase:
> 
> Patch 1: Replace RangeLength()'s bool parameter to TextIteratorBehavior; no
> behavior changes
>  - introduce static member
|TextIteratorBehavior::DefaultRangeLengthBehavior()|
> sets EmitsObjectReplacementCharacter
> 
> Patch 2: Add DoesNotEmitSpaceBeyondRange to |TextIteratorBehavior| and utilize
> it

This CL has now become the Patch 2. PTAL again.

[yosin_UTC9, 2017-04-26 06:34:40]

lgtm

[Tima Vaisburd, 2017-04-26 21:49:03]

The CQ bit was checked by timav@chromium.org

[Tima Vaisburd, 2017-04-26 21:49:04]

The patchset sent to the CQ was uploaded after l-g-t-m from yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/2832123002/#ps140001
(title: "Rebased")

[commit-bot: I haz the power, 2017-04-26 21:49:49]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-26 22:07:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-26 22:07:59]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Tima Vaisburd, 2017-04-26 22:17:02]

The CQ bit was checked by timav@chromium.org

[Tima Vaisburd, 2017-04-26 22:17:03]

The patchset sent to the CQ was uploaded after l-g-t-m from yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/2832123002/#ps160001
(title: "getElementById")

[commit-bot: I haz the power, 2017-04-26 22:17:46]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-27 00:09:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-27 00:09:20]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Tima Vaisburd, 2017-04-27 00:22:21]

The CQ bit was checked by timav@chromium.org

[commit-bot: I haz the power, 2017-04-27 00:22:55]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-27 01:19:17]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1493252541254930,
"parent_rev": "c54fcea42957f1effa94fedcb2d985f11488888c", "commit_rev":
"9305554380ee3a02c680009b5dee4b9e82cba334"}

[commit-bot: I haz the power, 2017-04-27 01:20:21]

Description was changed from

==========
Do not count extra trailing space for the last word in a range

Sometimes when a text selection ends with the last word on a line
(for instance when just one word is selected) the generated text
for the selection range contains an extra trailing space.
This happens when a text node is split into several InlineTextBoxes
(e.g. a <span> inside a narrow table) and the last selected word
happens to be the last one in such text box.

The TextIterator algorithm restores the spaces between the text boxes
and will add a trailing space for this word. For the SurroundingText,
however, we need the length that corresponds to the visible selection.

This CL introduces a new text iterator behavior that prevents that
extra space to be added. This behavior is used in combination with
DefaultRangeLengthBehavior() for the selection range calculation
in the SurroundingText.

BUG=708329
==========

to

==========
Do not count extra trailing space for the last word in a range

Sometimes when a text selection ends with the last word on a line
(for instance when just one word is selected) the generated text
for the selection range contains an extra trailing space.
This happens when a text node is split into several InlineTextBoxes
(e.g. a <span> inside a narrow table) and the last selected word
happens to be the last one in such text box.

The TextIterator algorithm restores the spaces between the text boxes
and will add a trailing space for this word. For the SurroundingText,
however, we need the length that corresponds to the visible selection.

This CL introduces a new text iterator behavior that prevents that
extra space to be added. This behavior is used in combination with
DefaultRangeLengthBehavior() for the selection range calculation
in the SurroundingText.

BUG=708329

Review-Url: https://codereview.chromium.org/2832123002
Cr-Commit-Position: refs/heads/master@{#467538}
Committed:
https://chromium.googlesource.com/chromium/src/+/9305554380ee3a02c680009b5dee...
==========

[commit-bot: I haz the power, 2017-04-27 01:20:25]

Committed patchset #8 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/9305554380ee3a02c680009b5dee...