Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2151803003:
[Editing][Regression] Contenteditable w/ "-webkit-user-select:all" should be editable for drag. (Closed)
|
Created:
4 years, 5 months ago by yoichio Modified:
4 years, 4 months ago Reviewers:
yosin_UTC9 CC:
blink-reviews, chromium-reviews, dcheng Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[Editing][Regression] Contenteditable w/ "-webkit-user-select:all" should be editable
for drag.
Dragging inside Contenteditable w/ "-webkit-user-select:all" element should select
partially rather than all.
This CL introduces the usedValueOfUserSelect function
deciding whether selection is all or not and use it in rootUserSelectAllForNode
instead of simple nodeIsUserSelectAll.
BUG=629715
TEST=LayoutTests/editing/selection/mouse/drag-user-select-all-textarea.html, drag-
user-select-all-contenteditable.html
Committed: https://crrev.com/9e0f169e61cf7ba643540ec91d16c393a7eaa124
Cr-Commit-Position: refs/heads/master@{#408045}
Patch Set 1 #Patch Set 2 : update #
Total comments: 4
Patch Set 3 : update #Messages
Total messages: 35 (27 generated)
Description was changed from ========== drag contenteditable BUG= ========== to ========== drag contenteditable BUG= ==========
The CQ bit was checked by yoichio@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: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
Description was changed from ========== drag contenteditable BUG= ========== to ========== [Editing][Regression] Contenteditable w/ "-webkit-user-select:all" should be editable for drag. Dragging inside Contenteditable w/ "-webkit-user-select:all" element should select partially not all. This CL refactors procedure deciding whether selection is all or not I introduced: https://chromium.googlesource.com/chromium/src/+/d607a605c05ae726b55aa3a28b32... 7ff to the function |rootUserSelectAllAndNotEditableNode|. Then the function is introduced to the function |SelectionController::updateSelectionForMouseDrag| , which expand selection by user dragging. BUG= ==========
yoichio@chromium.org changed reviewers: + yosin@chromium.org
The CQ bit was checked by yoichio@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: This issue passed the CQ dry run.
Description was changed from ========== [Editing][Regression] Contenteditable w/ "-webkit-user-select:all" should be editable for drag. Dragging inside Contenteditable w/ "-webkit-user-select:all" element should select partially not all. This CL refactors procedure deciding whether selection is all or not I introduced: https://chromium.googlesource.com/chromium/src/+/d607a605c05ae726b55aa3a28b32... 7ff to the function |rootUserSelectAllAndNotEditableNode|. Then the function is introduced to the function |SelectionController::updateSelectionForMouseDrag| , which expand selection by user dragging. BUG= ========== to ========== [Editing][Regression] Contenteditable w/ "-webkit-user-select:all" should be editable for drag. Dragging inside Contenteditable w/ "-webkit-user-select:all" element should select partially rather than all. This CL refactors procedure deciding whether selection is all or not I introduced: https://chromium.googlesource.com/chromium/src/+/d607a605c05ae726b55aa3a28b32... 7ff to the function |rootUserSelectAllAndNotEditableNode|. Then the function is introduced to the function |SelectionController::updateSelectionForMouseDrag| , which expand selection by user dragging. BUG= ==========
Description was changed from ========== [Editing][Regression] Contenteditable w/ "-webkit-user-select:all" should be editable for drag. Dragging inside Contenteditable w/ "-webkit-user-select:all" element should select partially rather than all. This CL refactors procedure deciding whether selection is all or not I introduced: https://chromium.googlesource.com/chromium/src/+/d607a605c05ae726b55aa3a28b32... 7ff to the function |rootUserSelectAllAndNotEditableNode|. Then the function is introduced to the function |SelectionController::updateSelectionForMouseDrag| , which expand selection by user dragging. BUG= ========== to ========== [Editing][Regression] Contenteditable w/ "-webkit-user-select:all" should be editable for drag. Dragging inside Contenteditable w/ "-webkit-user-select:all" element should select partially rather than all. This CL refactors procedure deciding whether selection is all or not I introduced: https://chromium.googlesource.com/chromium/src/+/d607a605c05ae726b55aa3a28b32... 7ff to the function |rootUserSelectAllAndNotEditableNode|. Then the function is introduced to the function |SelectionController::updateSelectionForMouseDrag| , which expand selection by user dragging. TEST=LayoutTests/editing/selection/mouse/drag-user-select-all-textarea.html, drag- user-select-all-contenteditable.html ==========
Description was changed from ========== [Editing][Regression] Contenteditable w/ "-webkit-user-select:all" should be editable for drag. Dragging inside Contenteditable w/ "-webkit-user-select:all" element should select partially rather than all. This CL refactors procedure deciding whether selection is all or not I introduced: https://chromium.googlesource.com/chromium/src/+/d607a605c05ae726b55aa3a28b32... 7ff to the function |rootUserSelectAllAndNotEditableNode|. Then the function is introduced to the function |SelectionController::updateSelectionForMouseDrag| , which expand selection by user dragging. TEST=LayoutTests/editing/selection/mouse/drag-user-select-all-textarea.html, drag- user-select-all-contenteditable.html ========== to ========== [Editing][Regression] Contenteditable w/ "-webkit-user-select:all" should be editable for drag. Dragging inside Contenteditable w/ "-webkit-user-select:all" element should select partially rather than all. This CL refactors procedure deciding whether selection is all or not I introduced: https://chromium.googlesource.com/chromium/src/+/d607a605c05ae726b55aa3a28b32... 7ff to the function |rootUserSelectAllAndNotEditableNode|. Then the function is introduced to the function |SelectionController::updateSelectionForMouseDrag| , which expand selection by user dragging. BUG=629715 TEST=LayoutTests/editing/selection/mouse/drag-user-select-all-textarea.html, drag- user-select-all-contenteditable.html ==========
On 2016/07/20 at 01:56:38, yoichio wrote: > Is this specified in the spec? Firefox selects all content rather than partial selection.
https://codereview.chromium.org/2151803003/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/editing/selection/mouse/drag-user-select-all-textarea.html (right): https://codereview.chromium.org/2151803003/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/editing/selection/mouse/drag-user-select-all-textarea.html:13: assert_true(window.hasOwnProperty('eventSender'), 'this test requires window.eventSender'); |eventSender| might not be own property of window object. assert_true('eventSender' in window) is better, but this assertion is true for |window.eventSender == undefined|. So, assert_not_equal(window.eventSender, undefined) is safer. https://codereview.chromium.org/2151803003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/editing/SelectionController.cpp (right): https://codereview.chromium.org/2151803003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/editing/SelectionController.cpp:87: Node* const rootUserSelectAll = EditingInFlatTreeStrategy::rootUserSelectAllForNode(node); As talked offline, we should have a function, usedValueOfUserSelect(element) to compute used value of "user-select". We might be want to have usedValueOfUserSelect() in EditingUtilities.cpp or VisbileUnit.cpp. Once we have usedValueOfUserSelect(), SelectionControll doesn't need to check user-modify.
The CQ bit was checked by yoichio@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: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by yoichio@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: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
Patchset #3 (id:40001) has been deleted
Description was changed from ========== [Editing][Regression] Contenteditable w/ "-webkit-user-select:all" should be editable for drag. Dragging inside Contenteditable w/ "-webkit-user-select:all" element should select partially rather than all. This CL refactors procedure deciding whether selection is all or not I introduced: https://chromium.googlesource.com/chromium/src/+/d607a605c05ae726b55aa3a28b32... 7ff to the function |rootUserSelectAllAndNotEditableNode|. Then the function is introduced to the function |SelectionController::updateSelectionForMouseDrag| , which expand selection by user dragging. BUG=629715 TEST=LayoutTests/editing/selection/mouse/drag-user-select-all-textarea.html, drag- user-select-all-contenteditable.html ========== to ========== [Editing][Regression] Contenteditable w/ "-webkit-user-select:all" should be editable for drag. Dragging inside Contenteditable w/ "-webkit-user-select:all" element should select partially rather than all. This CL introduces the usedValueOfUserSelect function deciding whether selection is all or not and use it in rootUserSelectAllForNode instead of simple nodeIsUserSelectAll. BUG=629715 TEST=LayoutTests/editing/selection/mouse/drag-user-select-all-textarea.html, drag- user-select-all-contenteditable.html ==========
https://codereview.chromium.org/2151803003/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/editing/selection/mouse/drag-user-select-all-textarea.html (right): https://codereview.chromium.org/2151803003/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/editing/selection/mouse/drag-user-select-all-textarea.html:13: assert_true(window.hasOwnProperty('eventSender'), 'this test requires window.eventSender'); On 2016/07/20 06:58:58, Yosi_UTC9 wrote: > |eventSender| might not be own property of window object. > > assert_true('eventSender' in window) is better, but this assertion is true for > |window.eventSender == undefined|. > So, assert_not_equal(window.eventSender, undefined) is safer. Done. https://codereview.chromium.org/2151803003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/editing/SelectionController.cpp (right): https://codereview.chromium.org/2151803003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/editing/SelectionController.cpp:87: Node* const rootUserSelectAll = EditingInFlatTreeStrategy::rootUserSelectAllForNode(node); On 2016/07/20 06:58:58, Yosi_UTC9 wrote: > As talked offline, we should have a function, usedValueOfUserSelect(element) to > compute used value of "user-select". > We might be want to have usedValueOfUserSelect() in EditingUtilities.cpp or > VisbileUnit.cpp. > > Once we have usedValueOfUserSelect(), SelectionControll doesn't need to check > user-modify. > Created the function and use it in rootUserSelectAllForNode.
The CQ bit was checked by yosin@chromium.org
lgtm
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [Editing][Regression] Contenteditable w/ "-webkit-user-select:all" should be editable for drag. Dragging inside Contenteditable w/ "-webkit-user-select:all" element should select partially rather than all. This CL introduces the usedValueOfUserSelect function deciding whether selection is all or not and use it in rootUserSelectAllForNode instead of simple nodeIsUserSelectAll. BUG=629715 TEST=LayoutTests/editing/selection/mouse/drag-user-select-all-textarea.html, drag- user-select-all-contenteditable.html ========== to ========== [Editing][Regression] Contenteditable w/ "-webkit-user-select:all" should be editable for drag. Dragging inside Contenteditable w/ "-webkit-user-select:all" element should select partially rather than all. This CL introduces the usedValueOfUserSelect function deciding whether selection is all or not and use it in rootUserSelectAllForNode instead of simple nodeIsUserSelectAll. BUG=629715 TEST=LayoutTests/editing/selection/mouse/drag-user-select-all-textarea.html, drag- user-select-all-contenteditable.html ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== [Editing][Regression] Contenteditable w/ "-webkit-user-select:all" should be editable for drag. Dragging inside Contenteditable w/ "-webkit-user-select:all" element should select partially rather than all. This CL introduces the usedValueOfUserSelect function deciding whether selection is all or not and use it in rootUserSelectAllForNode instead of simple nodeIsUserSelectAll. BUG=629715 TEST=LayoutTests/editing/selection/mouse/drag-user-select-all-textarea.html, drag- user-select-all-contenteditable.html ========== to ========== [Editing][Regression] Contenteditable w/ "-webkit-user-select:all" should be editable for drag. Dragging inside Contenteditable w/ "-webkit-user-select:all" element should select partially rather than all. This CL introduces the usedValueOfUserSelect function deciding whether selection is all or not and use it in rootUserSelectAllForNode instead of simple nodeIsUserSelectAll. BUG=629715 TEST=LayoutTests/editing/selection/mouse/drag-user-select-all-textarea.html, drag- user-select-all-contenteditable.html Committed: https://crrev.com/9e0f169e61cf7ba643540ec91d16c393a7eaa124 Cr-Commit-Position: refs/heads/master@{#408045} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/9e0f169e61cf7ba643540ec91d16c393a7eaa124 Cr-Commit-Position: refs/heads/master@{#408045} |
