New issue
Advanced search Search tips

Issue 824687 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

We should unify AdjustSelectionToAvoidCrossingEditingBoundaries() and Adjust{For,Back}wardPositionToAvoidCrossingEditingBoundaries()

Project Member Reported by yosin@chromium.org, Mar 22 2018

Issue description

Logically AdjustSelectionToAvoidCrossingEditingBoundaries() and
HonorEditingBoundaryAtOr{After,Before}() do same thing.

Thus, we can implement AdjustSelectionToAvoidCrossingEditingBoundaries() with
HonorEditingBoundaryAtOr{After,Before}().
or implement HonorEditingBoundaryAtOr{After,Before}() by
AdjustSelectionToAvoidCrossingEditingBoundaries().
 
Labels: -Type-Bug Type-Task

Comment 2 by yosin@chromium.org, Mar 23 2018

Experiment: http://crrev.com/c/975083

There are 18 failures caused by HonorEditingBoundaryAtOr{After,Before}() returns
null position.

1. editing/execCommand/justy-center-with-uneditable-crash.html
2. editing/execCommand/meter-element-with-child-crash.html
3. editing/inserting/insert-with-javascript-protocol-crash.html
4. editing/selection/expanding-selections.html
5. editing/selection/expanding-selections2.html
6. editing/selection/mixed-editability-2.html
7. editing/selection/mouse/doubleclick-inline-first-last-contenteditable.html
8. editing/selection/select_all/select_all_details_crash.html
9. editing/shadow/breaking-editing-boundaries-2.html
10. editing/style/inline-style-container.html
11. editing/undo/redo_correct_selection.html
12. external/wpt/editing/event.html
13. external/wpt/editing/run/bold.html
14. external/wpt/editing/run/fontname.html
15. fast/forms/search/search-cancel-button-mouseup.html
16. images/crash-bad-cast.html
17. virtual/exotic-color-space/images/crash-bad-cast.html; same as 16
18. virtual/gpu-rasterization/images/crash-bad-cast.html; same as 16

Once, we fix HonorEditingBoundaryAtOr{After,Before}() not to return
null position. We can implement AdjustSelectionXXX() by HonorXXX().



Project Member

Comment 3 by bugdroid1@chromium.org, Mar 26 2018

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

commit a964717cbfefdac94b9a367c0aa8a2f8009265cd
Author: Yoshifumi Inoue <yosin@chromium.org>
Date: Mon Mar 26 10:39:20 2018

Rename HonorEditingBoundaryAtOr{Before,After}() to Adjust{Back,Forward}PositionToAvoidCrossingEditingBoundaries()

This patch renames |HonorEditingBoundaryAtOr{Before,After}()| to
|Adjust{Back,Forward}PositionToAvoidCrossingEditingBoundaries()| to follow
|SelectionAfjuster::AdjustSelecitonToAvoidCrossingEditingBoundaries()| for
improving readability.


Bug:  824687 
Change-Id: I156c09e884ea8ff805e920cdef928e473f7cb420
Reviewed-on: https://chromium-review.googlesource.com/977343
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545759}
[modify] https://crrev.com/a964717cbfefdac94b9a367c0aa8a2f8009265cd/third_party/WebKit/Source/core/editing/SelectionModifierCharacter.cpp
[modify] https://crrev.com/a964717cbfefdac94b9a367c0aa8a2f8009265cd/third_party/WebKit/Source/core/editing/SelectionModifierWord.cpp
[modify] https://crrev.com/a964717cbfefdac94b9a367c0aa8a2f8009265cd/third_party/WebKit/Source/core/editing/VisibleUnits.cpp
[modify] https://crrev.com/a964717cbfefdac94b9a367c0aa8a2f8009265cd/third_party/WebKit/Source/core/editing/VisibleUnits.h
[modify] https://crrev.com/a964717cbfefdac94b9a367c0aa8a2f8009265cd/third_party/WebKit/Source/core/editing/VisibleUnitsLine.cpp
[modify] https://crrev.com/a964717cbfefdac94b9a367c0aa8a2f8009265cd/third_party/WebKit/Source/core/editing/VisibleUnitsSentence.cpp
[modify] https://crrev.com/a964717cbfefdac94b9a367c0aa8a2f8009265cd/third_party/WebKit/Source/core/editing/VisibleUnitsTest.cpp
[modify] https://crrev.com/a964717cbfefdac94b9a367c0aa8a2f8009265cd/third_party/WebKit/Source/core/editing/VisibleUnitsWord.cpp

Project Member

Comment 4 by bugdroid1@chromium.org, Mar 28 2018

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

commit 9887782970c50159d926f28ada9a6ec6b543dd46
Author: Yoshifumi Inoue <yosin@chromium.org>
Date: Wed Mar 28 02:41:32 2018

Rewrite "editing/selection/expanding-selections{,2}.html" to utilize selection_test()

This patch changes "expanding-selections{,2}.html" to utilize |selection_test()|
and unify into "expanding-selections.html", since both tests do similar, for
ease of maintenance.

Note: overflow:hidden doesn't affect test results.
Note: Following patch will move this file into "selection/mouse/" directory.

This patch is a preparation of the patch[1].

[1] http://crrev.com/c/975083 Implement
AdjustSelectionToAvoidCrossingEditingBoundaries() with
HonorEditingBoundaryAtOr{After,Before}()

Bug: 679977,  824687 
Change-Id: Ie0719011bbca4571df5affc473ae5fb32b3d6a5d
Reviewed-on: https://chromium-review.googlesource.com/981847
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546374}
[modify] https://crrev.com/9887782970c50159d926f28ada9a6ec6b543dd46/third_party/WebKit/LayoutTests/editing/selection/expanding-selections.html
[delete] https://crrev.com/612031d22da59abaa68d37141b247ff6af5f0b9c/third_party/WebKit/LayoutTests/editing/selection/expanding-selections2.html
[delete] https://crrev.com/612031d22da59abaa68d37141b247ff6af5f0b9c/third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/editing/selection/expanding-selections-expected.txt
[delete] https://crrev.com/612031d22da59abaa68d37141b247ff6af5f0b9c/third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/editing/selection/expanding-selections2-expected.txt
[delete] https://crrev.com/612031d22da59abaa68d37141b247ff6af5f0b9c/third_party/WebKit/LayoutTests/platform/linux/editing/selection/expanding-selections-expected.png
[delete] https://crrev.com/612031d22da59abaa68d37141b247ff6af5f0b9c/third_party/WebKit/LayoutTests/platform/linux/editing/selection/expanding-selections-expected.txt
[delete] https://crrev.com/612031d22da59abaa68d37141b247ff6af5f0b9c/third_party/WebKit/LayoutTests/platform/linux/editing/selection/expanding-selections2-expected.png
[delete] https://crrev.com/612031d22da59abaa68d37141b247ff6af5f0b9c/third_party/WebKit/LayoutTests/platform/linux/editing/selection/expanding-selections2-expected.txt
[delete] https://crrev.com/612031d22da59abaa68d37141b247ff6af5f0b9c/third_party/WebKit/LayoutTests/platform/mac/editing/selection/expanding-selections-expected.png
[delete] https://crrev.com/612031d22da59abaa68d37141b247ff6af5f0b9c/third_party/WebKit/LayoutTests/platform/mac/editing/selection/expanding-selections-expected.txt
[delete] https://crrev.com/612031d22da59abaa68d37141b247ff6af5f0b9c/third_party/WebKit/LayoutTests/platform/mac/editing/selection/expanding-selections2-expected.png
[delete] https://crrev.com/612031d22da59abaa68d37141b247ff6af5f0b9c/third_party/WebKit/LayoutTests/platform/mac/editing/selection/expanding-selections2-expected.txt
[delete] https://crrev.com/612031d22da59abaa68d37141b247ff6af5f0b9c/third_party/WebKit/LayoutTests/platform/win/editing/selection/expanding-selections-expected.png
[delete] https://crrev.com/612031d22da59abaa68d37141b247ff6af5f0b9c/third_party/WebKit/LayoutTests/platform/win/editing/selection/expanding-selections-expected.txt
[delete] https://crrev.com/612031d22da59abaa68d37141b247ff6af5f0b9c/third_party/WebKit/LayoutTests/platform/win/editing/selection/expanding-selections2-expected.png
[delete] https://crrev.com/612031d22da59abaa68d37141b247ff6af5f0b9c/third_party/WebKit/LayoutTests/platform/win/editing/selection/expanding-selections2-expected.txt

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 29 2018

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

commit 4e78842cd2c665f7a08e6fea86487a0186629f51
Author: Yoshifumi Inoue <yosin@chromium.org>
Date: Thu Mar 29 02:34:26 2018

Rewrite "editing/selection/modify_move/move-by-word-visually-multi-line.html" to utilize selection_test()

This patch changes "move-by-word-visually-multi-line.html" to utilize
|selection_test()| and split into multiple files to remove from slow test
list for ease of maintenance.

This patch is a preparation of the patch[1].

Note: Test files are generated by the tool[2].

[1] http://crrev.com/c/975083 Implement
AdjustSelectionToAvoidCrossingEditingBoundaries() with
HonorEditingBoundaryAtOr{After,Before}()
[2] http://crrev.com/c/620331 Generate selection_test() for Selection#modify()

Bug: 679977,  824687 
Change-Id: I965c8dc2e2d6af7d0ed5f906d7185f12a3db2c12
Reviewed-on: https://chromium-review.googlesource.com/981917
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546705}
[modify] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/SlowTests
[delete] https://crrev.com/6438cf1ff7f9e249d150b5dc22d0157193c09cac/third_party/WebKit/LayoutTests/editing/selection/modify_move/move-by-word-visually-multi-line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_left_word_01_ltr_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_left_word_01_rtl_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_left_word_02_ltr_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_left_word_02_rtl_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_left_word_03_ltr_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_left_word_03_rtl_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_left_word_04_ltr_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_left_word_04_rtl_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_left_word_05_ltr_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_left_word_05_rtl_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_left_word_06_ltr_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_left_word_06_rtl_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_left_word_07_ltr_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_left_word_07_rtl_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_left_word_08_ltr_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_left_word_08_rtl_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_left_word_09_ltr_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_left_word_09_rtl_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_right_word_01_ltr_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_right_word_01_rtl_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_right_word_02_ltr_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_right_word_02_rtl_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_right_word_03_ltr_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_right_word_03_rtl_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_right_word_04_ltr_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_right_word_04_rtl_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_right_word_05_ltr_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_right_word_05_rtl_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_right_word_06_ltr_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_right_word_06_rtl_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_right_word_07_ltr_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_right_word_07_rtl_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_right_word_08_ltr_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_right_word_08_rtl_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_right_word_09_ltr_multi_line.html
[add] https://crrev.com/4e78842cd2c665f7a08e6fea86487a0186629f51/third_party/WebKit/LayoutTests/editing/selection/modify_move/move_right_word_09_rtl_multi_line.html

Comment 6 by yosin@chromium.org, Mar 29 2018

Status: WontFix (was: Available)
Summary: We should unify AdjustSelectionToAvoidCrossingEditingBoundaries() and Adjust{For,Back}wardPositionToAvoidCrossingEditingBoundaries() (was: We should unify AdjustSelectionToAvoidCrossingEditingBoundaries() and HonorEditingBoundaryAtOr{After,Before}())
Mark WontFix since 
AdjustSelectionToAvoidCrossingEditingBoundaries() and Adjust{For,Back}wardPositionToAvoidCrossingEditingBoundaries() do differently.

For selection, we make base and extent in lowest editing host.
For position, we make base and extent in highest editing host.

Sign in to add a comment