Get rid of m_granularity from VisibleSelection and SelectionTemplate |
|||||
Issue descriptionm_granularity is used for mouse double-clicking and triple-click, we should use it in SelectionController and FrameSelection only.
,
Feb 16 2017
See also [1], we also think getting rid of granularity from FrameSelection. [1] http://crbug.com/427801 Get rid of FrameSelection::granularity()
,
Mar 25 2017
,
Apr 25 2017
Lower to Pri-3, since this is for code health.
,
Jun 23 2017
VisbileSelection::granularity_ is introduced by patch[1]. [1] http://crrev.com/1648253002: Make VisibleSelection::updateIfNeeded() to consider text granularity
,
Jun 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/385b35c99e76cccd7d7866a9545b6715951b53d1 commit 385b35c99e76cccd7d7866a9545b6715951b53d1 Author: yosin <yosin@chromium.org> Date: Mon Jun 26 07:25:18 2017 Get rid of *live granularity* behavior from selection This patch gets rid of *live granularity* unintentionally introduced by the patch[1] by making |ExpandSelectionToRespectUserSelectAll()| in |SelectionController| to return expanded selection for improving interoperability. The *live granularity* behavior is invoked by double-click or triple-click to select word/paragraph. Once *live granularity* behavior attached to selection, selection is automatically expanded in word/paragraph granularity. Example of live granularity: 1. On HTML fragment <span>xyz</span>, user does double click at "y". 2. Selection becomes <span>^xyz|</span>, where "^" is anchor and "|" is focus. 3. Inserting |Text| node "abc" before |Text| node "xyz" => "abcxyz" 4. Selection becomes <span>^abcxyz|</span> 5. Inserting |Text| node "def" after "xyz" => "abcxyzdef" 6. Selection becomes <span>^abcxyzdef|</span> If selection doesn't have live granularity behavior, it stays "abc^xyz|def". This is done by on-demand computation of |VisibleSelection| implemented in |SelectionEditor| which re-compute selection by using granularity in |SelectionInDOM| passed by |SelectionCotnroller| as result of user's double- click or triple-click. In other words, we propagate granularity property, which is set by double- click or triple-click from |SelectionCotnroller| to |SelectionEditor| and we keep it until explicit change of selection other than DOM mutation. This patch is a follow-up patch of [1]. Following patches will remove granularity member from |VisibleSelectionTemplate| and |SelectionTemplate|. [1] http://crrev.com/1648253002: Make VisibleSelection::updateIfNeeded() to consider text granularity BUG= 692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html Review-Url: https://codereview.chromium.org/2951353003 Cr-Commit-Position: refs/heads/master@{#482209} [add] https://crrev.com/385b35c99e76cccd7d7866a9545b6715951b53d1/third_party/WebKit/LayoutTests/editing/selection/double_click_and_modify.html [modify] https://crrev.com/385b35c99e76cccd7d7866a9545b6715951b53d1/third_party/WebKit/Source/core/editing/SelectionController.cpp
,
Jun 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/47e7201e24fc08e37f0680f96b21bcbeff37230c commit 47e7201e24fc08e37f0680f96b21bcbeff37230c Author: yosin <yosin@chromium.org> Date: Tue Jun 27 01:30:56 2017 Make UpdateSelectionForMouseDownDispatchingSelectStart() to take SelectionInFlatTree This patch makes |UpdateSelectionForMouseDownDispatchingSelectStart()| in |SelectionController| class to take |SelectionInFlatTree| to reduce usage of |VisibleSelection| for improving code health. BUG= 692923 TEST=n/a; no behavior changes Review-Url: https://codereview.chromium.org/2950283002 Cr-Commit-Position: refs/heads/master@{#482499} [modify] https://crrev.com/47e7201e24fc08e37f0680f96b21bcbeff37230c/third_party/WebKit/Source/core/editing/SelectionController.cpp [modify] https://crrev.com/47e7201e24fc08e37f0680f96b21bcbeff37230c/third_party/WebKit/Source/core/editing/SelectionController.h
,
Jul 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/09dcc24b3adaa3fa2db4cbef0a8e2d4bcf0f1724 commit 09dcc24b3adaa3fa2db4cbef0a8e2d4bcf0f1724 Author: yosin <yosin@chromium.org> Date: Wed Jul 05 01:58:00 2017 Get rid of VisibleSelectionTemplate::has_trailing_whitespace_ This patch gets rid of |has_trailing_whitespace_| member variable from |VisibleSelectionTemplate| class since after the patch[1], this variable doesn't do nothing for improving code health. [1] http://crrev.com/2951353003 Get rid of *live granularity* behavior from selection BUG= 692923 TEST=n/a; no behavior changes Review-Url: https://codereview.chromium.org/2966433002 Cr-Commit-Position: refs/heads/master@{#484177} [modify] https://crrev.com/09dcc24b3adaa3fa2db4cbef0a8e2d4bcf0f1724/third_party/WebKit/Source/core/editing/VisibleSelection.cpp [modify] https://crrev.com/09dcc24b3adaa3fa2db4cbef0a8e2d4bcf0f1724/third_party/WebKit/Source/core/editing/VisibleSelection.h
,
Jul 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/28c838c598c2e513ac94512f828c4ee43af4e5d6 commit 28c838c598c2e513ac94512f828c4ee43af4e5d6 Author: yosin <yosin@chromium.org> Date: Wed Jul 05 08:37:50 2017 Use FrameSelection::Granularity in TextControlElement::ComputeSelection{Start,End}() This patch changes |TextControlElement::ComputeSelection{Start,End}()| to use |FrameSelection::Granularity()| instead of |VisibleSelection::Granularity()| as a preparation of getting rid of |VisibleSelection::Granularity()|. BUG= 692923 TEST=n/a; no behavior changes Review-Url: https://codereview.chromium.org/2969183002 Cr-Commit-Position: refs/heads/master@{#484218} [modify] https://crrev.com/28c838c598c2e513ac94512f828c4ee43af4e5d6/third_party/WebKit/Source/core/html/TextControlElement.cpp
,
Jul 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5166ccf41871457e8922b1a701bd6a2817dc2537 commit 5166ccf41871457e8922b1a701bd6a2817dc2537 Author: yosin <yosin@chromium.org> Date: Wed Jul 05 10:15:53 2017 Expose Compute{Start,End}RespectingGranularity() This patch exposes flat tree version of |ComputeStartRespectingGranularity()| and |ComputeEndRespectingGranularity()| used in |VisibleSelection::Validate()| as a preparation of getting rid of |granularity_| member variable from |VisibleSelectionTemplate|. The patch[1] contains usages of |Compute{Start,End}RespectingGranularity()| in "SelectionCotroller.cpp". [1] http://crrev.com/2959153002: Get rid of SelectionTemplate::granularity_ BUG= 692923 TEST=n/a; no behavior changes Review-Url: https://codereview.chromium.org/2967113002 Cr-Commit-Position: refs/heads/master@{#484234} [modify] https://crrev.com/5166ccf41871457e8922b1a701bd6a2817dc2537/third_party/WebKit/Source/core/editing/VisibleSelection.cpp [modify] https://crrev.com/5166ccf41871457e8922b1a701bd6a2817dc2537/third_party/WebKit/Source/core/editing/VisibleSelection.h
,
Jul 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a9d878d2268ec9db32e6f682c96bfe3cf93a1f4e commit a9d878d2268ec9db32e6f682c96bfe3cf93a1f4e Author: yosin <yosin@chromium.org> Date: Wed Jul 05 11:02:18 2017 Get rid of SelectionTemplate::has_trailing_whitespace_ This patch gets rid of redundant member variable |has_trailing_whitespace_| from |SelectionTemplate| for improving code health, since the patch[1], nobody sets this member variable true, [1] http://crrev.com/2966433002: Get rid of VisibleSelectionTemplate::has_trailing_whitespace_ BUG= 692923 TEST=n/a; no behavior changes Review-Url: https://codereview.chromium.org/2973583002 Cr-Commit-Position: refs/heads/master@{#484239} [modify] https://crrev.com/a9d878d2268ec9db32e6f682c96bfe3cf93a1f4e/third_party/WebKit/Source/core/editing/FrameSelection.cpp [modify] https://crrev.com/a9d878d2268ec9db32e6f682c96bfe3cf93a1f4e/third_party/WebKit/Source/core/editing/SelectionEditor.cpp [modify] https://crrev.com/a9d878d2268ec9db32e6f682c96bfe3cf93a1f4e/third_party/WebKit/Source/core/editing/SelectionTemplate.cpp [modify] https://crrev.com/a9d878d2268ec9db32e6f682c96bfe3cf93a1f4e/third_party/WebKit/Source/core/editing/SelectionTemplate.h [modify] https://crrev.com/a9d878d2268ec9db32e6f682c96bfe3cf93a1f4e/third_party/WebKit/Source/core/editing/SelectionTemplateTest.cpp
,
Jul 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/28f8baaa24a926b54e677c334a867b3dc63bef06 commit 28f8baaa24a926b54e677c334a867b3dc63bef06 Author: yosin <yosin@chromium.org> Date: Thu Jul 06 02:02:31 2017 Make FrameSelection::MoveRangeSelection() not to set live granularity selection This patch chagnes |FrameSelection::MoveRangeSelection()| not to set live granularity selection as similar as the patch[1], which fixes selection created by double-click. [1] http://crrev.com/2951353003: Get rid of *live granularity* behavior from selection BUG= 692923 TEST=run_webkit_unit_tests --gtest_filter=FrameSelectionTest.MoveRangeSelectionNoLiveness Review-Url: https://codereview.chromium.org/2972753002 Cr-Commit-Position: refs/heads/master@{#484443} [modify] https://crrev.com/28f8baaa24a926b54e677c334a867b3dc63bef06/third_party/WebKit/Source/core/editing/FrameSelection.cpp [modify] https://crrev.com/28f8baaa24a926b54e677c334a867b3dc63bef06/third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp
,
Jul 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9ad2300eb815732fda127eb344a43a4eff0a073e commit 9ad2300eb815732fda127eb344a43a4eff0a073e Author: yosin <yosin@chromium.org> Date: Thu Jul 06 09:51:53 2017 Introduce CreateVisibleSelectionWithGranularity() This patch Introduces |CreateVisibleSelectionWithGranularity()| to reduce usages of |SelectionTemplate::SetGranularity()| for a preparation of getting rid of |VisibleSelectio::granularity_|[1]. [1] http://crrev.com/959153002: Get rid of SelectionTemplate::granularity_ BUG= 692923 TEST=n/a; no behavior changes Review-Url: https://codereview.chromium.org/2972863003 Cr-Commit-Position: refs/heads/master@{#484518} [modify] https://crrev.com/9ad2300eb815732fda127eb344a43a4eff0a073e/third_party/WebKit/Source/core/editing/FrameSelection.cpp [modify] https://crrev.com/9ad2300eb815732fda127eb344a43a4eff0a073e/third_party/WebKit/Source/core/editing/SelectionController.cpp [modify] https://crrev.com/9ad2300eb815732fda127eb344a43a4eff0a073e/third_party/WebKit/Source/core/editing/VisibleSelection.cpp [modify] https://crrev.com/9ad2300eb815732fda127eb344a43a4eff0a073e/third_party/WebKit/Source/core/editing/VisibleSelection.h [modify] https://crrev.com/9ad2300eb815732fda127eb344a43a4eff0a073e/third_party/WebKit/Source/core/editing/VisibleSelectionTest.cpp [modify] https://crrev.com/9ad2300eb815732fda127eb344a43a4eff0a073e/third_party/WebKit/Source/core/editing/WebSubstringUtil.mm [modify] https://crrev.com/9ad2300eb815732fda127eb344a43a4eff0a073e/third_party/WebKit/Source/core/editing/commands/EditorCommand.cpp
,
Jul 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8ba4cfa3d3beed403d349394e8569905381a21aa commit 8ba4cfa3d3beed403d349394e8569905381a21aa Author: yosin <yosin@chromium.org> Date: Fri Jul 07 05:12:15 2017 Simplify SelectionController::UpdateSelectionForMouseDrag() This patch changes |SelectionController::UpdateSelectionForMouseDrag()| to utilize |AdjustPositionRespectUserSelectAll()| and |ExtendSelectionAsDirectional()| as what we are doing for Shift+Click to simplify implementation for improving code health. Processing of Shift+Click and mouse drag are similar, we do as following: * Shift-Click: 1. Click to set selection base at clicked position selection.collapse(clickedPositon) 2. Shift+Click to extend selection to clicked position selection.extend(clickedPoosition) * Mouse Drag: 1. Mouse left-button down to set selection base at mouse down position. selection.collapse(downPosition) 2. Mouse move, holding left-button down, to extend selection to position where move moved. selection.extend(movedPosition) Step 2 of both are same, extend to position where mouse is. BUG= 692923 TEST=n/a; no behavior changes Review-Url: https://codereview.chromium.org/2970043002 Cr-Commit-Position: refs/heads/master@{#484836} [modify] https://crrev.com/8ba4cfa3d3beed403d349394e8569905381a21aa/third_party/WebKit/Source/core/editing/SelectionController.cpp
,
Jul 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eadc611f5b2dbdc4aeb42c9092119444042f8521 commit eadc611f5b2dbdc4aeb42c9092119444042f8521 Author: Yoshifumi Inoue <yosin@chromium.org> Date: Mon Jul 10 11:17:15 2017 Change handling of Shift+Click and mouse drag selection extension with granularity This patch change handling Shift+Click and mouse drag selection extension with granularity, aka granularity expansion, to use current base position of selection instead of a position where granularity expansion started, e.g. double-click or triple-click. # Background The granularity expansion is started by double-click, for word, or triple-click, for paragraph. Once granularity extending started, farther selection extension is done by granularity, e.g. for word granularity, selection is extended by word, e.g. "^abc de|f" to "^abc def|" where "^" is base position and "|" is extend position. Before this patch, we use base position as a position where user starts granularity expansion and extent which specified by user for constructing |VisibleSelection| and hold base/extent as user specified positions and adjust positions with granularity into start/end positions. After this patch, we use current base position and granularity adjusted extent position for constructing |VisibleSelection| and base/extent and start/end hold extended positions. Note: start/end are result of calling of |Most{Back,For}wardCaretPosition()| of base/extent. # Changes in "shift-click.html" For Mac, it is handled by |ExtendSelectionAsNonDirectional()|, this patch fixes the issue[1], see in "shift-click.html" changes for "mac" behavior. When extent position at end of word, granularity expansion moves extent position to start of words, e.g. "^foo| bar" => "^foo |bar", in other words, word granularity expansion selection whitespaces. For non-Mac, is is handle by |ExtendSelectionAsnDirectional()|, this patch keeps correct direction of selection. Here is steps of user actions: 1. "<div id="first">|one <span id="start"></span>two^ three</div>" base is after "two" 2. "<div id="first">one |<span id="start"></span>two^ three</div>" Shift+Click before "span id=start" 3. "<div id="first">one <span id="start"></span>|two^ three</div>" Granularity expansion with visible position canonicalization Before this patch step 3 is 3. "<div id="first">one <span id="start"></span>^two| three</div>" Granularity expansion with visible position canonicalization for base/extent are before "two" where double-click is occurred. Note: This patch is also a preparation of unify base/extent and start/end[2] [1] https://bugs.webkit.org/show_bug.cgi?id=36256 shift+click to extend a wordgranularity selection backwards selects the space after [2] http://crbug.com/230267 We should get rid of the concept of base and extent Bug: 692923 , 230267 Change-Id: Icbb04e4c19637f4a1481891c4d3acdbc885424bc Reviewed-on: https://chromium-review.googlesource.com/562914 Reviewed-by: Yoichi Osato <yoichio@chromium.org> Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#485230} [modify] https://crrev.com/eadc611f5b2dbdc4aeb42c9092119444042f8521/third_party/WebKit/LayoutTests/editing/selection/shift-click.html [modify] https://crrev.com/eadc611f5b2dbdc4aeb42c9092119444042f8521/third_party/WebKit/Source/core/editing/SelectionController.cpp [modify] https://crrev.com/eadc611f5b2dbdc4aeb42c9092119444042f8521/third_party/WebKit/Source/core/editing/VisibleSelection.cpp
,
Jul 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5d524b67a1b83e06c2a982ca6ee6bc77f9582db1 commit 5d524b67a1b83e06c2a982ca6ee6bc77f9582db1 Author: Yoshifumi Inoue <yosin@chromium.org> Date: Tue Jul 11 11:23:26 2017 Get rid of useless member variable VisibleSelection::granularity_ This patch gets rid of useless member variable |granularity_| from |VisibleSelectionTemplate| for improving code health. Bug: 692923 Change-Id: I0ed67a6076e4c34496cf8c6eaf39f497572f614d Reviewed-on: https://chromium-review.googlesource.com/566200 Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Yoichi Osato <yoichio@chromium.org> Cr-Commit-Position: refs/heads/master@{#485583} [modify] https://crrev.com/5d524b67a1b83e06c2a982ca6ee6bc77f9582db1/third_party/WebKit/Source/core/editing/VisibleSelection.cpp [modify] https://crrev.com/5d524b67a1b83e06c2a982ca6ee6bc77f9582db1/third_party/WebKit/Source/core/editing/VisibleSelection.h
,
Jul 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5cb481239f13fdd3454fe56aadcb56d7ffb822d8 commit 5cb481239f13fdd3454fe56aadcb56d7ffb822d8 Author: Yoshifumi Inoue <yosin@chromium.org> Date: Wed Jul 12 06:47:09 2017 Get rid of SelectionTemplate::granularity_ This patch gets rid of |SelectionTemplate::granularity_| since it is always |TextGranularity::kCharacter| since the patch[1] for improving code health. This patch also renames |SelectionTypeWithLegacyGranularity()| to |Type()| since |SelectionTemplate| doesn't have granularity any more. [1] http://crrev.com/2970043002: Simplify SelectionController::UpdateSelectionForMouseDrag() Bug: 692923 Change-Id: Ic58dfcf8eef7f7fe2121df27b5639d98da16986d Reviewed-on: https://chromium-review.googlesource.com/567781 Reviewed-by: Kent Tamura <tkent@chromium.org> Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#485881} [modify] https://crrev.com/5cb481239f13fdd3454fe56aadcb56d7ffb822d8/third_party/WebKit/Source/core/editing/Editor.cpp [modify] https://crrev.com/5cb481239f13fdd3454fe56aadcb56d7ffb822d8/third_party/WebKit/Source/core/editing/FrameSelection.cpp [modify] https://crrev.com/5cb481239f13fdd3454fe56aadcb56d7ffb822d8/third_party/WebKit/Source/core/editing/GranularityStrategy.h [modify] https://crrev.com/5cb481239f13fdd3454fe56aadcb56d7ffb822d8/third_party/WebKit/Source/core/editing/SelectionController.cpp [modify] https://crrev.com/5cb481239f13fdd3454fe56aadcb56d7ffb822d8/third_party/WebKit/Source/core/editing/SelectionEditor.cpp [modify] https://crrev.com/5cb481239f13fdd3454fe56aadcb56d7ffb822d8/third_party/WebKit/Source/core/editing/SelectionTemplate.cpp [modify] https://crrev.com/5cb481239f13fdd3454fe56aadcb56d7ffb822d8/third_party/WebKit/Source/core/editing/SelectionTemplate.h [modify] https://crrev.com/5cb481239f13fdd3454fe56aadcb56d7ffb822d8/third_party/WebKit/Source/core/editing/SelectionTemplateTest.cpp [modify] https://crrev.com/5cb481239f13fdd3454fe56aadcb56d7ffb822d8/third_party/WebKit/Source/core/editing/VisibleSelection.cpp [modify] https://crrev.com/5cb481239f13fdd3454fe56aadcb56d7ffb822d8/third_party/WebKit/Source/core/html/TextControlElement.cpp
,
Jul 28 2017
,
Aug 2 2017
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by yosin@chromium.org
, Feb 16 2017Owner: yosin@chromium.org
Status: Started (was: Available)