Commands preserve handles

Editing Commands should preserve touch handles

This CL makes it so editing commands will keep touch selections if they were
present before the command.

BUG=719890
Review-Url: https://codereview.chromium.org/2890313003
Cr-Commit-Position: refs/heads/master@{#475778}
Committed: https://chromium.googlesource.com/chromium/src/+/dcc8b89e2e6e8e058f54c3169c796bc47b27e95b

[amaralp, 2017-05-19 01:06:02]

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

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

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-05-19 06:05:51]

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

[commit-bot: I haz the power, 2017-05-19 06:05:52]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[amaralp, 2017-05-20 00:47:40]

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

[commit-bot: I haz the power, 2017-05-20 00:48:12]

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-05-20 04:13:29]

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

[commit-bot: I haz the power, 2017-05-20 04:13:30]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[amaralp, 2017-05-22 18:29:37]

Description was changed from

==========
Commands preserve handles

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

to

==========
Editing Commands should preserve touch handles

This CL makes it so editing commands will keep touch selections if they were
present before the command.

BUG=719890
==========

[amaralp, 2017-05-22 18:29:38]

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

[amaralp, 2017-05-22 18:30:02]

PTAL

[yosin_UTC9, 2017-05-23 01:46:37]

It seems Editor::{Applied,Unapplied,Reapplied}Editing() calls 
 Editor::ChangeSelectionAfterCommand()
to update FrameSelection after computing SelecitonInDOMtree
by CorrectedSelectionAfterCommand().

To fix the issue 719890, we can change ChangeSelectionAfterCommand() to
call FS::SetSelection() with current handle visibility.

This is just one function change.

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

https://codereview.chromium.org/2890313003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/Editor.cpp:920: static SelectionInDOMTree
CorrectedSelectionAfterCommand(
Is it better to pass FrameSelection instead of Document and
bool/HandleVisibility to this function?

SelectionInDOMTree CorrectedSelectionAfterCommand(const VS& passed, const
FrameSelection& frameSelection) {
  if (!passed.IsValidFor(frameSeleciton.GetDocument())
    return SelectionInDOMTree();
  return
SelectionInDOMTree::Builder(passd.AsSelection()).SetIsHandleVisible(frameSeleciton.IsHandleVisible());
}

xiaochengh@, WDYT?

https://codereview.chromium.org/2890313003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/Editor.cpp:923: bool is_handle_visible) {
We should use |HandleVisibility|.

https://codereview.chromium.org/2890313003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/Editor.cpp:930: return
passed_selection.AsSelection(is_handle_visible);
Please use SelectionInDOMTree::Builder as

return SelectionInDOMTree::Builder(passed_selection.AsSelection())
  .SetIsHandleVisbile(is_handle_visible)
  .Build();

[amaralp, 2017-05-30 23:54:22]

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

[commit-bot: I haz the power, 2017-05-30 23:55:15]

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-05-31 01:57:06]

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

[commit-bot: I haz the power, 2017-05-31 01:57:07]

Dry run: This issue passed the CQ dry run.

[amaralp, 2017-05-31 02:12:06]

On 2017/05/23 at 01:46:37, yosin wrote:
> It seems Editor::{Applied,Unapplied,Reapplied}Editing() calls 
>  Editor::ChangeSelectionAfterCommand()
> to update FrameSelection after computing SelecitonInDOMtree
> by CorrectedSelectionAfterCommand().
> 
> To fix the issue 719890, we can change ChangeSelectionAfterCommand() to
> call FS::SetSelection() with current handle visibility.
> 
> This is just one function change.

This greatly simplifies things. Thanks for the suggestion.

[yosin_UTC9, 2017-05-31 03:39:25]

The CQ bit was checked by yosin@chromium.org

[yosin_UTC9, 2017-05-31 03:39:26]

lgtm

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

https://codereview.chromium.org/2890313003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/SelectionEditor.cpp:197:
.SetIsHandleVisible(selection_.IsHandleVisible())
Good catch! (^_^)b

[commit-bot: I haz the power, 2017-05-31 03:39:39]

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-05-31 03:45:10]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496201965847430,
"parent_rev": "24cb0c7bbb151c77cba4b6fa43b79f6bd030a618", "commit_rev":
"dcc8b89e2e6e8e058f54c3169c796bc47b27e95b"}

[commit-bot: I haz the power, 2017-05-31 03:45:22]

Description was changed from

==========
Editing Commands should preserve touch handles

This CL makes it so editing commands will keep touch selections if they were
present before the command.

BUG=719890
==========

to

==========
Editing Commands should preserve touch handles

This CL makes it so editing commands will keep touch selections if they were
present before the command.

BUG=719890

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

[commit-bot: I haz the power, 2017-05-31 03:45:23]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/dcc8b89e2e6e8e058f54c3169c79...