Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2891693002:
Make mouse drag to ignore unfocused selection (Closed)
|
Created:
3 years, 7 months ago by yosin_UTC9 Modified:
3 years, 7 months ago CC:
blink-reviews, chromium-reviews, dcheng Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionMake mouse drag to ignore unfocused selection
This patch makes |SelectionController::UpdateSelectionForMouseDrag()| to
ignore unfocused selection to make mouse dragging not to change unfocused
selection.
This patch updates "drag_user_select_none.html" to expect that there are
no selection when starting from "user-select:none".
BUG=481985, 714000
TEST=run_layout_test editing/input/drag_in_unselectable.html
Review-Url: https://codereview.chromium.org/2891693002
Cr-Commit-Position: refs/heads/master@{#473130}
Committed: https://chromium.googlesource.com/chromium/src/+/d541129f1b272eb3e611b59b5e12dd0be0a4ba58
Patch Set 1 : 2017-05-17T14:55:08 #Patch Set 2 : 2017-05-17T16:41:10 #
Total comments: 8
Patch Set 3 : 2017-05-17T18:53:32 #
Total comments: 6
Patch Set 4 : 2017-05-19T15:07:18 #
Messages
Total messages: 37 (25 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...
Description was changed from ========== 2017-05-17T14:55:08 BUG=714000 2017-05-17T14:53:38 ========== to ========== Make mouse drag to ignore unfocused selection This patch makes |SelectionController::UpdateSelectionForMouseDrag()| to ignore unfocused selection to make mouse dragging not to change unfocused selection. BUG=714000 TEST=run_layout_test editing/input/drag_in_unselectable.html ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Description was changed from ========== Make mouse drag to ignore unfocused selection This patch makes |SelectionController::UpdateSelectionForMouseDrag()| to ignore unfocused selection to make mouse dragging not to change unfocused selection. BUG=714000 TEST=run_layout_test editing/input/drag_in_unselectable.html ========== to ========== Make mouse drag to ignore unfocused selection This patch makes |SelectionController::UpdateSelectionForMouseDrag()| to ignore unfocused selection to make mouse dragging not to change unfocused selection. BUG=481985,714000 TEST=run_layout_test editing/input/drag_in_unselectable.html ==========
Description was changed from ========== Make mouse drag to ignore unfocused selection This patch makes |SelectionController::UpdateSelectionForMouseDrag()| to ignore unfocused selection to make mouse dragging not to change unfocused selection. BUG=481985,714000 TEST=run_layout_test editing/input/drag_in_unselectable.html ========== to ========== Make mouse drag to ignore unfocused selection This patch makes |SelectionController::UpdateSelectionForMouseDrag()| to ignore unfocused selection to make mouse dragging not to change unfocused selection. This patch updates "drag_user_select_none.html" to expect that there are no selection when starting from "user-select:none". BUG=481985,714000 TEST=run_layout_test editing/input/drag_in_unselectable.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...
hugoh@opera.com changed reviewers: + hugoh@opera.com
https://codereview.chromium.org/2891693002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html (right): https://codereview.chromium.org/2891693002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html:14: <body> Nit: Maybe add a <input> and test that too since this bug not only happens on contenteditable. https://codereview.chromium.org/2891693002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html:15: <div contenteditable id="sample1">0123456789</div> Nit: 2 spaces, <div c... https://codereview.chromium.org/2891693002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html:26: const charWidth = sample3.offsetWidth / sample3.firstChild.length; Can be done on sample2? (Is the <span id="sample3"> really needed?). https://codereview.chromium.org/2891693002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html:69: }), 'Drag unselectable below editable'); Nit: "A drag at unselectable should not modify selection in contenteditable." (I like to mention the expectation in this string.)
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
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...
yosin@chromium.org changed reviewers: + xiaochengh@chromium.org, yoichio@chromium.org
PTAL https://codereview.chromium.org/2891693002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html (right): https://codereview.chromium.org/2891693002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html:14: <body> On 2017/05/17 at 08:15:57, hugoh_UTC2 wrote: > Nit: Maybe add a <input> and test that too since this bug not only happens on contenteditable. Done. https://codereview.chromium.org/2891693002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html:15: <div contenteditable id="sample1">0123456789</div> On 2017/05/17 at 08:15:57, hugoh_UTC2 wrote: > Nit: 2 spaces, <div c... Done. https://codereview.chromium.org/2891693002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html:26: const charWidth = sample3.offsetWidth / sample3.firstChild.length; On 2017/05/17 at 08:15:57, hugoh_UTC2 wrote: > Can be done on sample2? (Is the <span id="sample3"> really needed?). No. sample2.offsetWidth is width of page, instead of width of content, e.g. "0123456789". https://codereview.chromium.org/2891693002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html:69: }), 'Drag unselectable below editable'); On 2017/05/17 at 08:15:57, hugoh_UTC2 wrote: > Nit: "A drag at unselectable should not modify selection in contenteditable." (I like to mention the expectation in this string.) Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
xiaochengh@chromium.org changed reviewers: + lanwei@chromium.org
+lanwei for usage of pointerActionSequence https://codereview.chromium.org/2891693002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html (right): https://codereview.chromium.org/2891693002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html:40: chrome.gpuBenchmarking.pointerActionSequence( lanwei@: This test requires drag-and-drop. Last time you told me that pointerActionSequence didn't support drag-and-drop. Is it fixed now? Or is there a tracking bug to follow? Thanks!
On 2017/05/17 18:32:24, Xiaocheng wrote: > +lanwei for usage of pointerActionSequence > > https://codereview.chromium.org/2891693002/diff/40001/third_party/WebKit/Layo... > File third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html > (right): > > https://codereview.chromium.org/2891693002/diff/40001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html:40: > chrome.gpuBenchmarking.pointerActionSequence( > lanwei@: This test requires drag-and-drop. > > Last time you told me that pointerActionSequence didn't support drag-and-drop. > Is it fixed now? Or is there a tracking bug to follow? > > Thanks! Sorry, we do not. It is hard to fix, we have to change where we generate the events used in pointerActionSequence on each platform. I know some other team has plan to do it. I will let you know if they have a tracking issue.
On 2017/05/17 18:44:38, lanwei wrote: > On 2017/05/17 18:32:24, Xiaocheng wrote: > > +lanwei for usage of pointerActionSequence > > > > > https://codereview.chromium.org/2891693002/diff/40001/third_party/WebKit/Layo... > > File third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html > > (right): > > > > > https://codereview.chromium.org/2891693002/diff/40001/third_party/WebKit/Layo... > > third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html:40: > > chrome.gpuBenchmarking.pointerActionSequence( > > lanwei@: This test requires drag-and-drop. > > > > Last time you told me that pointerActionSequence didn't support drag-and-drop. > > Is it fixed now? Or is there a tracking bug to follow? > > > > Thanks! > > Sorry, we do not. It is hard to fix, we have to change where we generate the > events used in pointerActionSequence on each platform. I know some other team > has plan to do it. I will let you know if they have a tracking issue. This is the issue link https://bugs.chromium.org/p/chromium/issues/detail?id=722921, but I do not know when this will be finished.
On 2017/05/17 at 18:52:21, lanwei wrote: > On 2017/05/17 18:44:38, lanwei wrote: > > On 2017/05/17 18:32:24, Xiaocheng wrote: > > > +lanwei for usage of pointerActionSequence > > > > > > > > https://codereview.chromium.org/2891693002/diff/40001/third_party/WebKit/Layo... > > > File third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html > > > (right): > > > > > > > > https://codereview.chromium.org/2891693002/diff/40001/third_party/WebKit/Layo... > > > third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html:40: > > > chrome.gpuBenchmarking.pointerActionSequence( > > > lanwei@: This test requires drag-and-drop. > > > > > > Last time you told me that pointerActionSequence didn't support drag-and-drop. > > > Is it fixed now? Or is there a tracking bug to follow? > > > > > > Thanks! > > > > Sorry, we do not. It is hard to fix, we have to change where we generate the > > events used in pointerActionSequence on each platform. I know some other team > > has plan to do it. I will let you know if they have a tracking issue. > > This is the issue link > https://bugs.chromium.org/p/chromium/issues/detail?id=722921, but I do not know when this will be finished. Thanks!
https://codereview.chromium.org/2891693002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html (right): https://codereview.chromium.org/2891693002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html:40: chrome.gpuBenchmarking.pointerActionSequence( On 2017/05/17 at 18:32:24, Xiaocheng wrote: > lanwei@: This test requires drag-and-drop. > > Last time you told me that pointerActionSequence didn't support drag-and-drop. Is it fixed now? Or is there a tracking bug to follow? > > Thanks! It seems mouse dragging works for this test. When I attempted to drag in INPUT and contenteditable, tests could make range selection.
https://codereview.chromium.org/2891693002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html (right): https://codereview.chromium.org/2891693002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html:40: chrome.gpuBenchmarking.pointerActionSequence( Could you confirm dragging by listening mousemove event on unselectable event? https://codereview.chromium.org/2891693002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html:64: sample1.setSelectionRange(7, 7); Could you define |beforeSelectionStart = sample1.selectionStart;| end assert_equals it or so?
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...
PTAL Update a test script. https://codereview.chromium.org/2891693002/diff/40001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html (right): https://codereview.chromium.org/2891693002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html:40: chrome.gpuBenchmarking.pointerActionSequence( On 2017/05/18 at 01:47:02, yoichio wrote: > Could you confirm dragging by listening mousemove event > on unselectable event? Done. Good point! To avoid regression of this test script, it is better to check we really move mouse on |unselectable| element. https://codereview.chromium.org/2891693002/diff/40001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/editing/input/drag_in_unselectable.html:64: sample1.setSelectionRange(7, 7); On 2017/05/18 at 01:47:02, yoichio wrote: > Could you define |beforeSelectionStart = sample1.selectionStart;| > end assert_equals it or so? We don't check precondition. We assume all API except for testing in the test, work as expected.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
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": 60001, "attempt_start_ts": 1495183040642690,
"parent_rev": "a5c323b49440e551e321155f7a9bdb80fec5f80a", "commit_rev":
"d541129f1b272eb3e611b59b5e12dd0be0a4ba58"}
Message was sent while issue was closed.
Description was changed from ========== Make mouse drag to ignore unfocused selection This patch makes |SelectionController::UpdateSelectionForMouseDrag()| to ignore unfocused selection to make mouse dragging not to change unfocused selection. This patch updates "drag_user_select_none.html" to expect that there are no selection when starting from "user-select:none". BUG=481985,714000 TEST=run_layout_test editing/input/drag_in_unselectable.html ========== to ========== Make mouse drag to ignore unfocused selection This patch makes |SelectionController::UpdateSelectionForMouseDrag()| to ignore unfocused selection to make mouse dragging not to change unfocused selection. This patch updates "drag_user_select_none.html" to expect that there are no selection when starting from "user-select:none". BUG=481985,714000 TEST=run_layout_test editing/input/drag_in_unselectable.html Review-Url: https://codereview.chromium.org/2891693002 Cr-Commit-Position: refs/heads/master@{#473130} Committed: https://chromium.googlesource.com/chromium/src/+/d541129f1b272eb3e611b59b5e12... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/d541129f1b272eb3e611b59b5e12... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
