Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2951353003:
Get rid of *live granularity* behavior from selection (Closed)
|
Created:
3 years, 6 months ago by yosin_UTC9 Modified:
3 years, 5 months ago CC:
blink-reviews, chromium-reviews Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionGet 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}
Committed: https://chromium.googlesource.com/chromium/src/+/385b35c99e76cccd7d7866a9545b6715951b53d1
Patch Set 1 : 2017-06-23T18:30:58 #
Total comments: 2
Messages
Total messages: 36 (30 generated)
The CQ bit was checked by yosin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
Description was changed from ========== 2017-06-23T14:11:51 BUG= 2017-06-23T14:04:48 ========== to ========== 2017-06-23T14:11:51 BUG=692923 2017-06-23T14:04:48 ==========
Description was changed from ========== 2017-06-23T14:11:51 BUG=692923 2017-06-23T14:04:48 ========== to ========== Make ExpandSelectionToRespectUserSelectAll() to return expanded selection This patch makes |ExpandSelectionToRespectUserSelectAll()| to return expanded selection instead of collapsed selection with granularity, in case of double-click and trip-click, as preparation of getting rid of granularity member variable from |SelectionTemplate| and |VisibleSelection|. BUG=692923 TEST=n/a; no behavior changes ==========
The CQ bit was checked by yosin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
Description was changed from ========== Make ExpandSelectionToRespectUserSelectAll() to return expanded selection This patch makes |ExpandSelectionToRespectUserSelectAll()| to return expanded selection instead of collapsed selection with granularity, in case of double-click and trip-click, as preparation of getting rid of granularity member variable from |SelectionTemplate| and |VisibleSelection|. BUG=692923 TEST=n/a; no behavior changes ========== to ========== Make ExpandSelectionToRespectUserSelectAll() to return expanded selection This patch makes |ExpandSelectionToRespectUserSelectAll()| to return expanded selection instead of collapsed selection with granularity, in case of double-click and trip-click, as preparation of getting rid of granularity member variable from |SelectionTemplate| and |VisibleSelection|. BUG=692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html ==========
Description was changed from ========== Make ExpandSelectionToRespectUserSelectAll() to return expanded selection This patch makes |ExpandSelectionToRespectUserSelectAll()| to return expanded selection instead of collapsed selection with granularity, in case of double-click and trip-click, as preparation of getting rid of granularity member variable from |SelectionTemplate| and |VisibleSelection|. BUG=692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html ========== to ========== Make ExpandSelectionToRespectUserSelectAll() to return expanded selection This patch makes |ExpandSelectionToRespectUserSelectAll()| to return expanded selection instead of collapsed selection with granularity, in case of double-click and trip-click, as preparation of getting rid of granularity member variable from |SelectionTemplate| and |VisibleSelection|. This patch is a preparation of the patch[1]. [1] http://crrev.com/2950283002: Make UpdateSelectionForMouseDownDispatchingSelectStart() to take SelectionInFlatTree BUG=692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html ==========
The CQ bit was checked by yosin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
yosin@chromium.org changed reviewers: + xiaochengh@chromium.org, yoichio@chromium.org
PTAL
Description was changed from ========== Make ExpandSelectionToRespectUserSelectAll() to return expanded selection This patch makes |ExpandSelectionToRespectUserSelectAll()| to return expanded selection instead of collapsed selection with granularity, in case of double-click and trip-click, as preparation of getting rid of granularity member variable from |SelectionTemplate| and |VisibleSelection|. This patch is a preparation of the patch[1]. [1] http://crrev.com/2950283002: Make UpdateSelectionForMouseDownDispatchingSelectStart() to take SelectionInFlatTree BUG=692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html ========== to ========== Use expanded selection instead of expanding at use Before this patch, Blink updates selection at use[1][2] to avoid to use invalid selection. During selection update, we use granularity member variable of |VisibleSelection| to expand selection. However, this introduces Blink to use unintentionally selected word or paragraph when JavaScript modifies DOM tree around selected word or paragraph, e.g. inserting |Text| node before selected word extends selected word from inserted |Text| node to users selected word, see attached test. After this patch, |FrameSelection| holds expanded selection instead of using granularity member in |VisibleSelection|, to avoid unintentional selection expansion. This is done by changing |ExpandSelectionToRespectUserSelectAll()| used by |SelectionController|, where Blink has |VisibleSelection| having word or paragraph granularity. This is proper solution what the patch[2] to fixed. This patch is a preparation of the patch[3]. [1] http://crrev.com/1612983003: Introduce FrameSelection::updateIfNeeded() to update selection positions [2] http://crrev.com/1648253002: Make VisibleSelection::updateIfNeeded() to consider text granularity [3] http://crrev.com/2950283002: Make UpdateSelectionForMouseDownDispatchingSelectStart() to take SelectionInFlatTree BUG=692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html ==========
Description was changed from ========== Use expanded selection instead of expanding at use Before this patch, Blink updates selection at use[1][2] to avoid to use invalid selection. During selection update, we use granularity member variable of |VisibleSelection| to expand selection. However, this introduces Blink to use unintentionally selected word or paragraph when JavaScript modifies DOM tree around selected word or paragraph, e.g. inserting |Text| node before selected word extends selected word from inserted |Text| node to users selected word, see attached test. After this patch, |FrameSelection| holds expanded selection instead of using granularity member in |VisibleSelection|, to avoid unintentional selection expansion. This is done by changing |ExpandSelectionToRespectUserSelectAll()| used by |SelectionController|, where Blink has |VisibleSelection| having word or paragraph granularity. This is proper solution what the patch[2] to fixed. This patch is a preparation of the patch[3]. [1] http://crrev.com/1612983003: Introduce FrameSelection::updateIfNeeded() to update selection positions [2] http://crrev.com/1648253002: Make VisibleSelection::updateIfNeeded() to consider text granularity [3] http://crrev.com/2950283002: Make UpdateSelectionForMouseDownDispatchingSelectStart() to take SelectionInFlatTree BUG=692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html ========== to ========== 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" 4. Selection becomes <span>^abcxyz|</span> 5. Inserting |Text| node "0123" before "abc" 6. Selection becomes <span>^0123abcxyz|</span> 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 also a preparation of the patch[3]. [1] http://crrev.com/1648253002: Make VisibleSelection::updateIfNeeded() to consider text granularity [2] http://crrev.com/1612983003: Introduce FrameSelection::updateIfNeeded() to update selection positions [3] http://crrev.com/2950283002: Make UpdateSelectionForMouseDownDispatchingSelectStart() to take SelectionInFlatTree BUG=692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html ==========
Description was changed from ========== 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" 4. Selection becomes <span>^abcxyz|</span> 5. Inserting |Text| node "0123" before "abc" 6. Selection becomes <span>^0123abcxyz|</span> 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 also a preparation of the patch[3]. [1] http://crrev.com/1648253002: Make VisibleSelection::updateIfNeeded() to consider text granularity [2] http://crrev.com/1612983003: Introduce FrameSelection::updateIfNeeded() to update selection positions [3] http://crrev.com/2950283002: Make UpdateSelectionForMouseDownDispatchingSelectStart() to take SelectionInFlatTree BUG=692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html ========== to ========== 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" 4. Selection becomes <span>^abcxyz|</span> 5. Inserting |Text| node "0123" before "abc" 6. Selection becomes <span>^0123abcxyz|</span> 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 also a preparation of the patch[2]. [1] http://crrev.com/1648253002: Make VisibleSelection::updateIfNeeded() to consider text granularity [2] http://crrev.com/2950283002: Make UpdateSelectionForMouseDownDispatchingSelectStart() to take SelectionInFlatTree BUG=692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html ==========
Description was changed from ========== 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" 4. Selection becomes <span>^abcxyz|</span> 5. Inserting |Text| node "0123" before "abc" 6. Selection becomes <span>^0123abcxyz|</span> 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 also a preparation of the patch[2]. [1] http://crrev.com/1648253002: Make VisibleSelection::updateIfNeeded() to consider text granularity [2] http://crrev.com/2950283002: Make UpdateSelectionForMouseDownDispatchingSelectStart() to take SelectionInFlatTree BUG=692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html ========== to ========== 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" 4. Selection becomes <span>^abcxyz|</span> 5. Inserting |Text| node "0123" before "abc" 6. Selection becomes <span>^0123abcxyz|</span> 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] and a preparation of the patch[2]. Following patches will remove granularity member from |VisibleSelectionTempalte| and |SelectionTemplate|. [1] http://crrev.com/1648253002: Make VisibleSelection::updateIfNeeded() to consider text granularity [2] http://crrev.com/2950283002: Make UpdateSelectionForMouseDownDispatchingSelectStart() to take SelectionInFlatTree BUG=692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html ==========
Description was changed from ========== 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" 4. Selection becomes <span>^abcxyz|</span> 5. Inserting |Text| node "0123" before "abc" 6. Selection becomes <span>^0123abcxyz|</span> 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] and a preparation of the patch[2]. Following patches will remove granularity member from |VisibleSelectionTempalte| and |SelectionTemplate|. [1] http://crrev.com/1648253002: Make VisibleSelection::updateIfNeeded() to consider text granularity [2] http://crrev.com/2950283002: Make UpdateSelectionForMouseDownDispatchingSelectStart() to take SelectionInFlatTree BUG=692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html ========== to ========== 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" 4. Selection becomes <span>^abcxyz|</span> 5. Inserting |Text| node "0123" before "abc" 6. Selection becomes <span>^0123abcxyz|</span> 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] and a preparation of the patch[2]. Following patches will remove granularity member from |VisibleSelectionTemplate| and |SelectionTemplate|. [1] http://crrev.com/1648253002: Make VisibleSelection::updateIfNeeded() to consider text granularity [2] http://crrev.com/2950283002: Make UpdateSelectionForMouseDownDispatchingSelectStart() to take SelectionInFlatTree BUG=692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html ==========
Description was changed from ========== 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" 4. Selection becomes <span>^abcxyz|</span> 5. Inserting |Text| node "0123" before "abc" 6. Selection becomes <span>^0123abcxyz|</span> 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] and a preparation of the patch[2]. Following patches will remove granularity member from |VisibleSelectionTemplate| and |SelectionTemplate|. [1] http://crrev.com/1648253002: Make VisibleSelection::updateIfNeeded() to consider text granularity [2] http://crrev.com/2950283002: Make UpdateSelectionForMouseDownDispatchingSelectStart() to take SelectionInFlatTree BUG=692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html ========== to ========== 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" 4. Selection becomes <span>^abcxyz|</span> 5. Inserting |Text| node "def" after "xyz" 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] and a preparation of the patch[2]. Following patches will remove granularity member from |VisibleSelectionTemplate| and |SelectionTemplate|. [1] http://crrev.com/1648253002: Make VisibleSelection::updateIfNeeded() to consider text granularity [2] http://crrev.com/2950283002: Make UpdateSelectionForMouseDownDispatchingSelectStart() to take SelectionInFlatTree BUG=692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html ==========
Description was changed from ========== 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" 4. Selection becomes <span>^abcxyz|</span> 5. Inserting |Text| node "def" after "xyz" 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] and a preparation of the patch[2]. Following patches will remove granularity member from |VisibleSelectionTemplate| and |SelectionTemplate|. [1] http://crrev.com/1648253002: Make VisibleSelection::updateIfNeeded() to consider text granularity [2] http://crrev.com/2950283002: Make UpdateSelectionForMouseDownDispatchingSelectStart() to take SelectionInFlatTree BUG=692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html ========== to ========== 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] and a preparation of the patch[2]. Following patches will remove granularity member from |VisibleSelectionTemplate| and |SelectionTemplate|. [1] http://crrev.com/1648253002: Make VisibleSelection::updateIfNeeded() to consider text granularity [2] http://crrev.com/2950283002: Make UpdateSelectionForMouseDownDispatchingSelectStart() to take SelectionInFlatTree BUG=692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html ==========
The behavioral change is good. Only one comment. https://codereview.chromium.org/2951353003/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/editing/SelectionController.cpp (right): https://codereview.chromium.org/2951353003/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/editing/SelectionController.cpp:90: if (selection.IsBaseFirst()) Can we use |builder.SetBaseAndExtent(selection.Base(), selection.Extent())| directly?
https://codereview.chromium.org/2951353003/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/editing/SelectionController.cpp (right): https://codereview.chromium.org/2951353003/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/editing/SelectionController.cpp:90: if (selection.IsBaseFirst()) On 2017/06/23 at 17:31:11, Xiaocheng wrote: > Can we use |builder.SetBaseAndExtent(selection.Base(), selection.Extent())| directly? No. When granularity is word/pargraph, base == extent and start/end hold expanded position. This is reason why SelectionTemplate::isCaret() is template <typename Strategy> bool SelectionTemplate<Strategy>::IsCaret() const { return base_.IsNotNull() && base_ == extent_ && granularity_ == kCharacterGranularity; }
Description was changed from ========== 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] and a preparation of the patch[2]. Following patches will remove granularity member from |VisibleSelectionTemplate| and |SelectionTemplate|. [1] http://crrev.com/1648253002: Make VisibleSelection::updateIfNeeded() to consider text granularity [2] http://crrev.com/2950283002: Make UpdateSelectionForMouseDownDispatchingSelectStart() to take SelectionInFlatTree BUG=692923 TEST=run_layout_tests editing/selection/double_click_and_modify.html ========== to ========== 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 ==========
lgtm Thanks for the clarification.
The CQ bit was checked by yosin@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1498454888500540,
"parent_rev": "2a648e1a4e315410950c444b6e118a4a3262872f", "commit_rev":
"385b35c99e76cccd7d7866a9545b6715951b53d1"}
Message was sent while issue was closed.
Description was changed from ========== 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 ========== to ========== 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} Committed: https://chromium.googlesource.com/chromium/src/+/385b35c99e76cccd7d7866a9545b... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:40001) as https://chromium.googlesource.com/chromium/src/+/385b35c99e76cccd7d7866a9545b... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
