New issue
Advanced search Search tips

Issue 867563 link

Starred by 1 user
Status: Available
Owner: ----
Cc:
editing-dev@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Clean up call sites of FrameSelection::SetSelectionAndEndTyping()

Project Member Reported by xiaoche...@chromium.org, Jul 25 
This function used to be the default FrameSelection::SetSelection(), which by itself sets SetSelectionOptions that ends typing.

To reduce confusion, we renamed this function into SetSelectionAndEndTyping().

However, not all of its callers care about ending typing. Most of them, especially those in unit tests, should be passing an empty SetSelectionOptions instead.

We should check the callers, change them to call SetSelection() with empty SetSelectionOptions if they don't care about typing.
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 31 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/97d22fa5048cb74be87b863cb232f13793339851

commit 97d22fa5048cb74be87b863cb232f13793339851
Author: tanvir.rizvi <tanvir.rizvi@samsung.com>
Date: Tue Jul 31 06:37:38 2018

Clean up call sites of FrameSelection::SetSelectionAndEndTyping

Many call sites which use this function do not
bother about the clearing the typing but they use
this function which is confusing.
Rather we will use FrameSelection::SetSelection
and pass default options now.
This CL does it from the EditorCommand.

Bug: 867563
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Ie25813f1cd76b00001a0447118c9a3e8cd42cc52
Reviewed-on: https://chromium-review.googlesource.com/1154861
Commit-Queue: Tanvir Rizvi <tanvir.rizvi@samsung.com>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579329}
[modify] https://crrev.com/97d22fa5048cb74be87b863cb232f13793339851/third_party/blink/renderer/core/editing/commands/apply_block_element_command_test.cc
[modify] https://crrev.com/97d22fa5048cb74be87b863cb232f13793339851/third_party/blink/renderer/core/editing/commands/apply_style_command_test.cc
[modify] https://crrev.com/97d22fa5048cb74be87b863cb232f13793339851/third_party/blink/renderer/core/editing/commands/delete_selection_command_test.cc
[modify] https://crrev.com/97d22fa5048cb74be87b863cb232f13793339851/third_party/blink/renderer/core/editing/commands/insert_incremental_text_command_test.cc
[modify] https://crrev.com/97d22fa5048cb74be87b863cb232f13793339851/third_party/blink/renderer/core/editing/commands/insert_list_command_test.cc
[modify] https://crrev.com/97d22fa5048cb74be87b863cb232f13793339851/third_party/blink/renderer/core/editing/commands/insert_paragraph_separator_command_test.cc
[modify] https://crrev.com/97d22fa5048cb74be87b863cb232f13793339851/third_party/blink/renderer/core/editing/commands/insert_text_command_test.cc
[modify] https://crrev.com/97d22fa5048cb74be87b863cb232f13793339851/third_party/blink/renderer/core/editing/commands/replace_selection_command_test.cc
[modify] https://crrev.com/97d22fa5048cb74be87b863cb232f13793339851/third_party/blink/renderer/core/editing/commands/typing_command_test.cc
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 1 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ae237f463630828a33682c9799a54e2d4d1a5f2f

commit ae237f463630828a33682c9799a54e2d4d1a5f2f
Author: tanvir.rizvi <tanvir.rizvi@samsung.com>
Date: Wed Aug 01 09:43:11 2018

Cleanup unused member of GranularityStrategyTest

Member function SetSelectionAndEndTyping()
was never used, instead FrameSelection
SetSelectionAndEndTyping was used directly.
So this is removed as a DeadCode

Bug: 867563
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I7d45dd28597e66cfb4ce35db28474136b5283356
Reviewed-on: https://chromium-review.googlesource.com/1158109
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579741}
[modify] https://crrev.com/ae237f463630828a33682c9799a54e2d4d1a5f2f/third_party/blink/renderer/core/editing/granularity_strategy_test.cc
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 2 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/66c0582defa993c938d818b8dccf672eab116315

commit 66c0582defa993c938d818b8dccf672eab116315
Author: tanvir.rizvi <tanvir.rizvi@samsung.com>
Date: Thu Aug 02 12:09:33 2018

Clean up call sites of FrameSelection::SetSelectionAndEndTyping

Many call sites which use this function do not
bother about the clearing the typing but they use
this function which is confusing.
Rather we will use FrameSelection::SetSelection
and pass default options now.
This CL does it from the GranularityStrategyTest.

Bug: 867563
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I4e88d64fb91b5b1fb7848ec74b216a7fdda7f8bf
Reviewed-on: https://chromium-review.googlesource.com/1159940
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Tanvir Rizvi <tanvir.rizvi@samsung.com>
Cr-Commit-Position: refs/heads/master@{#580138}
[modify] https://crrev.com/66c0582defa993c938d818b8dccf672eab116315/third_party/blink/renderer/core/editing/granularity_strategy_test.cc
►

Sign in to add a comment