[InputEvent] Fill |data| field for 'input' event InsertText

[InputEvent] Fill |data| field for 'input' event InsertText

According to spec only InsertText and InsertNonText has |data| field.

This CL fills |data| field for 'input' event InsertText ('beforeinput' already has it), InsertNonText is not supported yet.

The |data| text is retrieved from the last |InsertTextCommand| of current |TypingCommand|, return empty string if the last command is not |InsertTextCommand|. (e.g. |InsertLineBreakCommand|)

Note:
|TypingCommand| has member |m_textToInsert| but it's only recording the first text of current open command.

SPEC=http://w3c.github.io/editing/input-events.html#h-interface-inputevent-attributes

BUG=585875
Committed: https://crrev.com/bfd2e5aec10ceceecda09fe7440e4a5e3e2bc648
Cr-Commit-Position: refs/heads/master@{#404192}

[chongz, 2016-07-04 17:28:25]

Patchset #1 (id:1) has been deleted

[chongz, 2016-07-04 21:42:20]

Description was changed from

==========
[InputEvent] Fill |data| field for 'input' event InsertText

BUG=585875
==========

to

==========
[InputEvent] Fill |data| field for 'input' event InsertText

According to spec only InsertText and InsertNonText has |data| field.

This CL fills |data| field for 'input' event InsertText ('beforeinput' already
has it), InsertNonText is not supported yet.

The |data| text is retrieved from the last |InsertTextCommand| of current
|TypingCommand|, return empty string if the last command is not
|InsertTextCommand|. (e.g. |InsertLineBreakCommand|)

Note:
|TypingCommand| has member |m_textToInsert| but it's only recording the first
text of current open command.

SPEC=http://w3c.github.io/editing/input-events.html#h-interface-inputevent-attributes

BUG=585875
==========

[chongz, 2016-07-04 21:47:20]

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

[chongz, 2016-07-04 21:47:21]

yosin@ PTAL at this 'input' event CL for |data| field, thanks!

[yosin_UTC9, 2016-07-05 01:22:08]

It seems we have virtual member function EditCommand::textData(), we don't need
to have |isInsertTextCommand()|. virtual member function may be simpler than
type dispatching.

https://codereview.chromium.org/2117543003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/events/inputevents/inputevent-data-keyboard.html
(right):

https://codereview.chromium.org/2117543003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/events/inputevents/inputevent-data-keyboard.html:12:
if (!window.eventSender) {
Let's use |assert_not_equal(window.eventSender, undefined, 'this test requires
eventSender)|,
rather than wipe out.

https://codereview.chromium.org/2117543003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/Editor.cpp (right):

https://codereview.chromium.org/2117543003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/Editor.cpp:147: if
(command->isTypingCommand()) {
nit: To reduce indentation, we could write:

if (!command->isTypingCommand())
    return emptyString();
....

https://codereview.chromium.org/2117543003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/Editor.cpp:152: return
typingCommand->textOfLastCommand();
Can we make |textOfLastCommand()| as virtual function of |EditCommand|?
If so, we don't need to dispatching by type here.

https://codereview.chromium.org/2117543003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/commands/TypingCommand.h (right):

https://codereview.chromium.org/2117543003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/TypingCommand.h:82: //
TyingCommand may contain multiple InsertTextCommand, |textOfLastCommand()|
returns the text of the last one.
s/Tying/Typing/

[chongz, 2016-07-05 20:33:28]

yosin@ I've changed |textOfLastCommand()| to |virtual EditCommand::textData()|
and removed type dispatching. 
PTAL again, thanks!

https://codereview.chromium.org/2117543003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/events/inputevents/inputevent-data-keyboard.html
(right):

https://codereview.chromium.org/2117543003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/events/inputevents/inputevent-data-keyboard.html:12:
if (!window.eventSender) {
On 2016/07/05 01:22:07, Yosi_UTC9 wrote:
> Let's use |assert_not_equal(window.eventSender, undefined, 'this test requires
> eventSender)|,
> rather than wipe out.

Done.

https://codereview.chromium.org/2117543003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/Editor.cpp (right):

https://codereview.chromium.org/2117543003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/Editor.cpp:147: if
(command->isTypingCommand()) {
On 2016/07/05 01:22:07, Yosi_UTC9 wrote:
> nit: To reduce indentation, we could write:
> 
> if (!command->isTypingCommand())
>     return emptyString();
> ....

Done.

https://codereview.chromium.org/2117543003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/Editor.cpp:152: return
typingCommand->textOfLastCommand();
On 2016/07/05 01:22:07, Yosi_UTC9 wrote:
> Can we make |textOfLastCommand()| as virtual function of |EditCommand|?
> If so, we don't need to dispatching by type here.

Changed to |textData()|.

https://codereview.chromium.org/2117543003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/commands/TypingCommand.h (right):

https://codereview.chromium.org/2117543003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/TypingCommand.h:82: //
TyingCommand may contain multiple InsertTextCommand, |textOfLastCommand()|
returns the text of the last one.
On 2016/07/05 01:22:07, Yosi_UTC9 wrote:
> s/Tying/Typing/

Done.

[yosin_UTC9, 2016-07-06 01:20:56]

https://codereview.chromium.org/2117543003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/Editor.cpp (right):

https://codereview.chromium.org/2117543003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/Editor.cpp:145: String
dataFromCommand(const CompositeEditCommand* command)
It seems having |EditCommand::textDataForInputEvent()| makes our code simpler
and more specific.
Since at this time, we have only one client of |EditCommand::textData()|, which
is dispatching input event.

https://codereview.chromium.org/2117543003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/commands/EditCommand.h (right):

https://codereview.chromium.org/2117543003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/EditCommand.h:58: virtual String
textData() const { return emptyString(); }
Please move implementation of virtual member function in .cpp file:
http://dev.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Stop-in...

https://codereview.chromium.org/2117543003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/commands/InsertTextCommand.h
(right):

https://codereview.chromium.org/2117543003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/InsertTextCommand.h:46: String
textData() const override { return m_text; };
nit: s/override/final/

Please move implementation of virtual member function in .cpp file:
http://dev.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Stop-in...

[chongz, 2016-07-06 22:56:41]

Patchset #3 (id:60001) has been deleted

[chongz, 2016-07-06 23:34:32]

yosin@ I've changed |EditCommand::textData()| to
|EditCommand::textDataForInputEvent()| and moved implementation to .cpp files,
PTAL again, thanks!

https://codereview.chromium.org/2117543003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/Editor.cpp (right):

https://codereview.chromium.org/2117543003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/Editor.cpp:145: String
dataFromCommand(const CompositeEditCommand* command)
On 2016/07/06 01:20:55, Yosi_UTC9 wrote:
> It seems having |EditCommand::textDataForInputEvent()| makes our code simpler
> and more specific.
> Since at this time, we have only one client of |EditCommand::textData()|,
which
> is dispatching input event.

Done.

https://codereview.chromium.org/2117543003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/commands/EditCommand.h (right):

https://codereview.chromium.org/2117543003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/EditCommand.h:58: virtual String
textData() const { return emptyString(); }
On 2016/07/06 01:20:55, Yosi_UTC9 wrote:
> Please move implementation of virtual member function in .cpp file:
>
http://dev.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Stop-in...

Done.

https://codereview.chromium.org/2117543003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/commands/InsertTextCommand.h
(right):

https://codereview.chromium.org/2117543003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/commands/InsertTextCommand.h:46: String
textData() const override { return m_text; };
On 2016/07/06 01:20:55, Yosi_UTC9 wrote:
> nit: s/override/final/
> 
> Please move implementation of virtual member function in .cpp file:
>
http://dev.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Stop-in...
> 

Done.

[yosin_UTC9, 2016-07-07 00:44:32]

lgtm w/ small nit

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

https://codereview.chromium.org/2117543003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/Editor.cpp:146: String
dataFromCommand(const CompositeEditCommand* command)
We don't need to have this function at this time.
When we need to do more for data, we'll introduce this function again with more
functionality.
Let's make changes smaller.

[chongz, 2016-07-07 18:36:16]

The CQ bit was checked by chongz@chromium.org

[chongz, 2016-07-07 18:36:16]

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/2117543003/#ps100001
(title: "Removed |dataFromCommand()| helper function")

[commit-bot: I haz the power, 2016-07-07 18:37:21]

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, 2016-07-07 18:50:27]

Description was changed from

==========
[InputEvent] Fill |data| field for 'input' event InsertText

According to spec only InsertText and InsertNonText has |data| field.

This CL fills |data| field for 'input' event InsertText ('beforeinput' already
has it), InsertNonText is not supported yet.

The |data| text is retrieved from the last |InsertTextCommand| of current
|TypingCommand|, return empty string if the last command is not
|InsertTextCommand|. (e.g. |InsertLineBreakCommand|)

Note:
|TypingCommand| has member |m_textToInsert| but it's only recording the first
text of current open command.

SPEC=http://w3c.github.io/editing/input-events.html#h-interface-inputevent-attributes

BUG=585875
==========

to

==========
[InputEvent] Fill |data| field for 'input' event InsertText

According to spec only InsertText and InsertNonText has |data| field.

This CL fills |data| field for 'input' event InsertText ('beforeinput' already
has it), InsertNonText is not supported yet.

The |data| text is retrieved from the last |InsertTextCommand| of current
|TypingCommand|, return empty string if the last command is not
|InsertTextCommand|. (e.g. |InsertLineBreakCommand|)

Note:
|TypingCommand| has member |m_textToInsert| but it's only recording the first
text of current open command.

SPEC=http://w3c.github.io/editing/input-events.html#h-interface-inputevent-attributes

BUG=585875
==========

[commit-bot: I haz the power, 2016-07-07 18:50:28]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2016-07-07 18:50:33]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-07 18:54:09]

Description was changed from

==========
[InputEvent] Fill |data| field for 'input' event InsertText

According to spec only InsertText and InsertNonText has |data| field.

This CL fills |data| field for 'input' event InsertText ('beforeinput' already
has it), InsertNonText is not supported yet.

The |data| text is retrieved from the last |InsertTextCommand| of current
|TypingCommand|, return empty string if the last command is not
|InsertTextCommand|. (e.g. |InsertLineBreakCommand|)

Note:
|TypingCommand| has member |m_textToInsert| but it's only recording the first
text of current open command.

SPEC=http://w3c.github.io/editing/input-events.html#h-interface-inputevent-attributes

BUG=585875
==========

to

==========
[InputEvent] Fill |data| field for 'input' event InsertText

According to spec only InsertText and InsertNonText has |data| field.

This CL fills |data| field for 'input' event InsertText ('beforeinput' already
has it), InsertNonText is not supported yet.

The |data| text is retrieved from the last |InsertTextCommand| of current
|TypingCommand|, return empty string if the last command is not
|InsertTextCommand|. (e.g. |InsertLineBreakCommand|)

Note:
|TypingCommand| has member |m_textToInsert| but it's only recording the first
text of current open command.

SPEC=http://w3c.github.io/editing/input-events.html#h-interface-inputevent-attributes

BUG=585875

Committed: https://crrev.com/bfd2e5aec10ceceecda09fe7440e4a5e3e2bc648
Cr-Commit-Position: refs/heads/master@{#404192}
==========

[commit-bot: I haz the power, 2016-07-07 18:54:11]

Patchset 4 (id:??) landed as
https://crrev.com/bfd2e5aec10ceceecda09fe7440e4a5e3e2bc648
Cr-Commit-Position: refs/heads/master@{#404192}