Make user-triggered SelectAll act as if there is no selection for hidden selection

Make user-triggered SelectAll act as if there is no selection for hidden selection

This patch ensures that a user-triggered SelectAll selects the entire document
when there is a hidden selection. This is for consistency in user experience
because a hidden selection appears as no selection to a user. Behavior of
Javascript-triggered SelectAll remains the same.

BUG=722925
TEST=editing/selection/select_all/select_all_with_unfocused_selection.html

Review-Url: https://codereview.chromium.org/2891203002
Cr-Commit-Position: refs/heads/master@{#473103}
Committed: https://chromium.googlesource.com/chromium/src/+/bd5cd151162312c124b976bc822bbd0e6f4f2d39

[Xiaocheng, 2017-05-18 22:09:06]

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

[commit-bot: I haz the power, 2017-05-18 22:10:59]

Dry run: CQ is trying da patch.

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

[Xiaocheng, 2017-05-19 00:09:21]

Description was changed from

==========
Make SelectAll command act as if there is no selection for hidden selection

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

to

==========
Make user-triggered SelectAll act as if there is no selection for hidden
selection

This patch introduces a new behavior that, user-triggered SelectAll selects the
entire document when there is a hidden selection. This is for consistency in
user experience because a hidden selection appears as no selection to a user.
Behavior of Javascript-triggered SelectAll remains the same.

BUG=722925
TEST=editing/selection/select_all/select_all_with_unfocused_selection.html
==========

[Xiaocheng, 2017-05-19 00:10:12]

Description was changed from

==========
Make user-triggered SelectAll act as if there is no selection for hidden
selection

This patch introduces a new behavior that, user-triggered SelectAll selects the
entire document when there is a hidden selection. This is for consistency in
user experience because a hidden selection appears as no selection to a user.
Behavior of Javascript-triggered SelectAll remains the same.

BUG=722925
TEST=editing/selection/select_all/select_all_with_unfocused_selection.html
==========

to

==========
Make user-triggered SelectAll act as if there is no selection for hidden
selection

This patch ensures that a user-triggered SelectAll selects the entire document
when there is a hidden selection. This is for consistency in user experience
because a hidden selection appears as no selection to a user. Behavior of
Javascript-triggered SelectAll remains the same.

BUG=722925
TEST=editing/selection/select_all/select_all_with_unfocused_selection.html
==========

[Xiaocheng, 2017-05-19 00:10:19]

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

[Xiaocheng, 2017-05-19 00:10:20]

PTAL.

[commit-bot: I haz the power, 2017-05-19 00:57:20]

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

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

Dry run: This issue passed the CQ dry run.

[yosin_UTC9, 2017-05-19 02:19:36]

https://codereview.chromium.org/2891203002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/editing/selection/select_all/select_all_with_unfocused_selection.html
(right):

https://codereview.chromium.org/2891203002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/editing/selection/select_all/select_all_with_unfocused_selection.html:56:
}, 'User-triggered SelectAll selects entire document when there is unfocused
range selection shown in contenteditable div');
I can read this description says "select entire document".
But, expected result specifies selection in cE.

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

https://codereview.chromium.org/2891203002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:711: //
TODO(editing-dev): Should we pass in user_triggered?
Duplicated to L717? Or, do we want to pass |user_triggered| to
|SelectFrameElementInParentIfFullySelected()|?

[Xiaocheng, 2017-05-19 02:25:51]

https://codereview.chromium.org/2891203002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/editing/selection/select_all/select_all_with_unfocused_selection.html
(right):

https://codereview.chromium.org/2891203002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/editing/selection/select_all/select_all_with_unfocused_selection.html:56:
}, 'User-triggered SelectAll selects entire document when there is unfocused
range selection shown in contenteditable div');
On 2017/05/19 at 02:19:36, yosin_UTC9 wrote:
> I can read this description says "select entire document".
> But, expected result specifies selection in cE.

Bad copy-and-paste.

Will fix.

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

https://codereview.chromium.org/2891203002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:711: //
TODO(editing-dev): Should we pass in user_triggered?
On 2017/05/19 at 02:19:36, yosin_UTC9 wrote:
> Duplicated to L717? Or, do we want to pass |user_triggered| to
|SelectFrameElementInParentIfFullySelected()|?

Not duplicate. SetSelection() takes an |option| which allows to specify if the
selection change is user triggered or not.

[yosin_UTC9, 2017-05-19 05:52:29]

The CQ bit was checked by yosin@chromium.org

[yosin_UTC9, 2017-05-19 05:52:30]

lgtm

[commit-bot: I haz the power, 2017-05-19 05:53:03]

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 05:58:38]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1495173149571930, "parent_rev":
"04749717bf2f41bf945b73d9729d24eb262b2843", "commit_rev":
"bd5cd151162312c124b976bc822bbd0e6f4f2d39"}

[commit-bot: I haz the power, 2017-05-19 05:58:50]

Description was changed from

==========
Make user-triggered SelectAll act as if there is no selection for hidden
selection

This patch ensures that a user-triggered SelectAll selects the entire document
when there is a hidden selection. This is for consistency in user experience
because a hidden selection appears as no selection to a user. Behavior of
Javascript-triggered SelectAll remains the same.

BUG=722925
TEST=editing/selection/select_all/select_all_with_unfocused_selection.html
==========

to

==========
Make user-triggered SelectAll act as if there is no selection for hidden
selection

This patch ensures that a user-triggered SelectAll selects the entire document
when there is a hidden selection. This is for consistency in user experience
because a hidden selection appears as no selection to a user. Behavior of
Javascript-triggered SelectAll remains the same.

BUG=722925
TEST=editing/selection/select_all/select_all_with_unfocused_selection.html

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

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

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/bd5cd151162312c124b976bc822b...