TEXTAREA: Cutting last line without EOL should not remove the remaining EOL in the previous line.

TEXTAREA: Cutting last line without EOL should not remove the remaining EOL in the previous line.

Our TEXTAREA element assumes there is an extra trailing <BR> or \n if the last
character of the value is \n.  So we remove the last \n when we generate value
from innerEditor content.
However, the assumption was broken if we cut the last line content without EOL.
Editing code doesn't add an extra <BR> in such case.  We removed the last \n in
the value mistakenly.

Solution:
We should make sure there no <BR> elements in innerEditor except the extra trailing
one so that we can ignore it safely on generating TEXTAREA value.
Also, we should make sure editing code doesn't add \n for placeholder newline.

 - HTMLTextAreaElement simply ignore the last <BR> on generating value.

 - HTMLTextAreaElement makes sure that innerEditor has the last <BR> after
  every editing operations.

 - InsertLineBreakCommand always inserts \n for plain text editing field, however
  it produces <BR> as an extra line break for TEXTAREA.

 - ReplaceSelectionCommand appends a <BR> for an InterchangeNewline at the end.

 - InsertTextCommand should not produce TabSpans for plain text editing.

BUG=522144, 593184
Committed: https://crrev.com/37f57d892222dc0ed816ae004074d7c80600ad46
Cr-Commit-Position: refs/heads/master@{#380385}

[tkent, 2016-03-10 03:37:14]

Description was changed from

==========
TEXTAREA: Cutting last line without EOL should not remove the remaining EOL in
the previous line.

BUG=522144,593184
==========

to

==========
TEXTAREA: Cutting last line without EOL should not remove the remaining EOL in
the previous line.

Our TEXTAREA element assumes there is an extra trailing <BR> or \n if the last
character of the value is \n.  So we remove the last \n when we generate value
from innerEditor content.
However, the assumption was broken if we cut the last line content without EOL.
Editing code doesn't add an extra <BR> in such case.  We removed the last \n in
the value mistakenly.

Solution:
We should make sure there no <BR> elements in innerEditor except the extra
trailing
one so that we can ignore it safely on generating TEXTAREA value.
 - HTMLTextAreaElement simply ignore the last <BR> on generating value.
 - HTMLTextAreaElement makes sure that innerEditor has the last <BR> after
  every editing operations.
 - InsertLineBreakCommand always inserts \n for plain text editing field,
however
  it produces <BR> as an extra line break for TEXTAREA.


BUG=522144,593184
==========

[tkent, 2016-03-10 05:35:31]

Description was changed from

==========
TEXTAREA: Cutting last line without EOL should not remove the remaining EOL in
the previous line.

Our TEXTAREA element assumes there is an extra trailing <BR> or \n if the last
character of the value is \n.  So we remove the last \n when we generate value
from innerEditor content.
However, the assumption was broken if we cut the last line content without EOL.
Editing code doesn't add an extra <BR> in such case.  We removed the last \n in
the value mistakenly.

Solution:
We should make sure there no <BR> elements in innerEditor except the extra
trailing
one so that we can ignore it safely on generating TEXTAREA value.
 - HTMLTextAreaElement simply ignore the last <BR> on generating value.
 - HTMLTextAreaElement makes sure that innerEditor has the last <BR> after
  every editing operations.
 - InsertLineBreakCommand always inserts \n for plain text editing field,
however
  it produces <BR> as an extra line break for TEXTAREA.


BUG=522144,593184
==========

to

==========
TEXTAREA: Cutting last line without EOL should not remove the remaining EOL in
the previous line.

Our TEXTAREA element assumes there is an extra trailing <BR> or \n if the last
character of the value is \n.  So we remove the last \n when we generate value
from innerEditor content.
However, the assumption was broken if we cut the last line content without EOL.
Editing code doesn't add an extra <BR> in such case.  We removed the last \n in
the value mistakenly.

Solution:
We should make sure there no <BR> elements in innerEditor except the extra
trailing
one so that we can ignore it safely on generating TEXTAREA value.
Also, we should make sure editing code doesn't add \n for placeholder newline.

 - HTMLTextAreaElement simply ignore the last <BR> on generating value.

 - HTMLTextAreaElement makes sure that innerEditor has the last <BR> after
  every editing operations.

 - InsertLineBreakCommand always inserts \n for plain text editing field,
however
  it produces <BR> as an extra line break for TEXTAREA.

 - ReplaceSelectionCommand appends a <BR> for an InterchangeNewline at the end.

 - InsertTextCommand should not produce TabSpans for plain text editing.


BUG=522144,593184
==========

[tkent, 2016-03-10 05:41:25]

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

[tkent, 2016-03-10 05:41:25]

yosin@, would you review this please?

This CL will be the last one for crbug.com/522144.

The changes in InsertLineBreakCommand and ReplaceSelectionCommand are ugly. 
Your advice are welcome.

[tkent, 2016-03-10 05:47:18]

Patchset #1 (id:1) has been deleted

[yosin_UTC9, 2016-03-10 08:33:15]

lgtm w/ nits

https://codereview.chromium.org/1785603002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLTextAreaElement.cpp (right):

https://codereview.chromium.org/1785603002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLTextAreaElement.cpp:278: // element.  If
we see other nodes, we need to fix editing code.
nit: one space after "."?

https://codereview.chromium.org/1785603002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLTextFormControlElement.cpp (right):

https://codereview.chromium.org/1785603002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLTextFormControlElement.cpp:631:
ASSERT(innerEditor);
Q: do we need to have this assert? following line de-reference |innerEditor|.

[tkent, 2016-03-10 08:42:38]

https://codereview.chromium.org/1785603002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLTextAreaElement.cpp (right):

https://codereview.chromium.org/1785603002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLTextAreaElement.cpp:278: // element.  If
we see other nodes, we need to fix editing code.
On 2016/03/10 at 08:33:15, Yosi_UTC9 wrote:
> nit: one space after "."?

Done.

https://codereview.chromium.org/1785603002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/HTMLTextFormControlElement.cpp (right):

https://codereview.chromium.org/1785603002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/HTMLTextFormControlElement.cpp:631:
ASSERT(innerEditor);
On 2016/03/10 at 08:33:15, Yosi_UTC9 wrote:
> Q: do we need to have this assert? following line de-reference |innerEditor|.

We may remove it.  Removed.

[tkent, 2016-03-10 09:42:25]

The CQ bit was checked by tkent@chromium.org

[tkent, 2016-03-10 09:42:25]

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/1785603002/#ps40001
(title: "remove an ASSERT, update a comment")

[commit-bot: I haz the power, 2016-03-10 09:42:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1785603002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1785603002/40001

[commit-bot: I haz the power, 2016-03-10 10:53:10]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-03-10 10:54:10]

Description was changed from

==========
TEXTAREA: Cutting last line without EOL should not remove the remaining EOL in
the previous line.

Our TEXTAREA element assumes there is an extra trailing <BR> or \n if the last
character of the value is \n.  So we remove the last \n when we generate value
from innerEditor content.
However, the assumption was broken if we cut the last line content without EOL.
Editing code doesn't add an extra <BR> in such case.  We removed the last \n in
the value mistakenly.

Solution:
We should make sure there no <BR> elements in innerEditor except the extra
trailing
one so that we can ignore it safely on generating TEXTAREA value.
Also, we should make sure editing code doesn't add \n for placeholder newline.

 - HTMLTextAreaElement simply ignore the last <BR> on generating value.

 - HTMLTextAreaElement makes sure that innerEditor has the last <BR> after
  every editing operations.

 - InsertLineBreakCommand always inserts \n for plain text editing field,
however
  it produces <BR> as an extra line break for TEXTAREA.

 - ReplaceSelectionCommand appends a <BR> for an InterchangeNewline at the end.

 - InsertTextCommand should not produce TabSpans for plain text editing.


BUG=522144,593184
==========

to

==========
TEXTAREA: Cutting last line without EOL should not remove the remaining EOL in
the previous line.

Our TEXTAREA element assumes there is an extra trailing <BR> or \n if the last
character of the value is \n.  So we remove the last \n when we generate value
from innerEditor content.
However, the assumption was broken if we cut the last line content without EOL.
Editing code doesn't add an extra <BR> in such case.  We removed the last \n in
the value mistakenly.

Solution:
We should make sure there no <BR> elements in innerEditor except the extra
trailing
one so that we can ignore it safely on generating TEXTAREA value.
Also, we should make sure editing code doesn't add \n for placeholder newline.

 - HTMLTextAreaElement simply ignore the last <BR> on generating value.

 - HTMLTextAreaElement makes sure that innerEditor has the last <BR> after
  every editing operations.

 - InsertLineBreakCommand always inserts \n for plain text editing field,
however
  it produces <BR> as an extra line break for TEXTAREA.

 - ReplaceSelectionCommand appends a <BR> for an InterchangeNewline at the end.

 - InsertTextCommand should not produce TabSpans for plain text editing.

BUG=522144,593184

Committed: https://crrev.com/37f57d892222dc0ed816ae004074d7c80600ad46
Cr-Commit-Position: refs/heads/master@{#380385}
==========

[commit-bot: I haz the power, 2016-03-10 10:54:11]

Patchset 2 (id:??) landed as
https://crrev.com/37f57d892222dc0ed816ae004074d7c80600ad46
Cr-Commit-Position: refs/heads/master@{#380385}