Insert a line break when space is inserted at the top of wrapped lines

Insert a line break when space is inserted at the top of wrapped lines

This patch changes to insert a line break when user inserts a space at
the top of wrapped lines, so that the inserted space appear at the
caret position.

The change is for plaintext only for now, so that we can collect
feedback and discuss with editor developers for what to do with
contenteditable.

BUG=76113
Review-Url: https://codereview.chromium.org/2618613004
Cr-Commit-Position: refs/heads/master@{#441848}
Committed: https://chromium.googlesource.com/chromium/src/+/a3b600981286b135632371477f902214c55a1724

[kojii, 2017-01-05 08:07:25]

Description was changed from

==========
smartspace

BUG=
==========

to

==========
Insert a line break when space is inserted at the top of wrapped lines

This patch changes to insert a line break when user inserts a space at
the top of wrapped lines.

BUG=76113
==========

[kojii, 2017-01-05 08:08:36]

kojii@chromium.org changed reviewers:
+ xing.xu@intel.com, yosin@chromium.org

[kojii, 2017-01-05 08:08:36]

yosin@, could you advice me what is the proper way to check if the caret is at
the beginning of a wrapped line?

[yosin_UTC9, 2017-01-05 08:36:50]

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

https://codereview.chromium.org/2618613004/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/Editor.cpp:212: // TODO(kojii): how to
know if a Position is at the top of wrapped line?
I think "start" is better than "top".

We can check this by

const Position& position = selection.start();

bool isStartOfWrappedLine = !InSameLine(
  PositionWithAffinity(position, TextAffinity::Upstream), 
  PositionWithAffinity(position, TextAffinity::Downstream));

If |position| is at start of wrapped line, VP(pos, Up) and VP(pos, Down) are in
different line.
VisiblePosition:create() does similar check to return VP(pos, Down) if VP(pos,
Down) and VP(pos, Up) are in same line.

[kojii, 2017-01-05 16:28:07]

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

[commit-bot: I haz the power, 2017-01-05 16:28:29]

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

[kojii, 2017-01-05 16:31:46]

Description was changed from

==========
Insert a line break when space is inserted at the top of wrapped lines

This patch changes to insert a line break when user inserts a space at
the top of wrapped lines.

BUG=76113
==========

to

==========
Insert a line break when space is inserted at the top of wrapped lines

This patch changes to insert a line break when user inserts a space at
the top of wrapped lines, so that the inserted space appear at the
caret position.

The change is for plaintext only for now, so that we can collect
feedback and discuss with editor developers for what to do with
contenteditable.

BUG=76113
==========

[kojii, 2017-01-05 16:34:55]

Perfect, thanks.

PTAL, added tests.

[commit-bot: I haz the power, 2017-01-05 18:36:32]

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

[commit-bot: I haz the power, 2017-01-05 18:36:33]

Dry run: 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_...)

[kojii, 2017-01-05 18:41:58]

...which didn't run on Mac, sorry, I'll check.

[kojii, 2017-01-05 19:44:03]

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

[commit-bot: I haz the power, 2017-01-05 19:44:25]

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-01-05 21:19:01]

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

[commit-bot: I haz the power, 2017-01-05 21:19:01]

Dry run: This issue passed the CQ dry run.

[kojii, 2017-01-06 00:56:38]

PTAL.

[yosin_UTC9, 2017-01-06 01:22:22]

lgtm

https://codereview.chromium.org/2618613004/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/editing/inserting/insert-space-at-start-of-wrapped-line.html
(right):

https://codereview.chromium.org/2618613004/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/inserting/insert-space-at-start-of-wrapped-line.html:19:
if (!window.eventSender) {
Good pattern!

[kojii, 2017-01-06 01:36:41]

The CQ bit was checked by kojii@chromium.org

[commit-bot: I haz the power, 2017-01-06 01:37:33]

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-01-06 02:47:43]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1483666601871110,
"parent_rev": "0fbe54c5a50804735b8407a0703515a1a39f78e8", "commit_rev":
"a3b600981286b135632371477f902214c55a1724"}

[commit-bot: I haz the power, 2017-01-06 02:48:14]

Description was changed from

==========
Insert a line break when space is inserted at the top of wrapped lines

This patch changes to insert a line break when user inserts a space at
the top of wrapped lines, so that the inserted space appear at the
caret position.

The change is for plaintext only for now, so that we can collect
feedback and discuss with editor developers for what to do with
contenteditable.

BUG=76113
==========

to

==========
Insert a line break when space is inserted at the top of wrapped lines

This patch changes to insert a line break when user inserts a space at
the top of wrapped lines, so that the inserted space appear at the
caret position.

The change is for plaintext only for now, so that we can collect
feedback and discuss with editor developers for what to do with
contenteditable.

BUG=76113

Review-Url: https://codereview.chromium.org/2618613004
Cr-Commit-Position: refs/heads/master@{#441848}
Committed:
https://chromium.googlesource.com/chromium/src/+/a3b600981286b135632371477f90...
==========

[commit-bot: I haz the power, 2017-01-06 02:48:15]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/a3b600981286b135632371477f90...