Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2900123002:
Account for mouse leave events in dragging flow (Closed)
|
Created:
3 years, 7 months ago by Navid Zolghadr Modified:
3 years, 7 months ago Reviewers:
dtapuska CC:
chromium-reviews, blink-reviews, dtapuska+blinkwatch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionAccount for mouse leave events in dragging flow
When mouse is outside of the page while dragging
if user presses Esc key we get a leave event
which seems correct logically. But the code was
not handling it properly.
BUG=723207, 724316
Review-Url: https://codereview.chromium.org/2900123002
Cr-Commit-Position: refs/heads/master@{#474450}
Committed: https://chromium.googlesource.com/chromium/src/+/12dedc4fa09fc54ee6d0bd54d7ffde5c5ab2bdba
Patch Set 1 #Patch Set 2 : Add the test #
Total comments: 1
Messages
Total messages: 21 (12 generated)
The CQ bit was checked by nzolghadr@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...
nzolghadr@chromium.org changed reviewers: + dtapuska@chromium.org
On 2017/05/23 17:59:38, Navid Zolghadr wrote: lgtm. Are there any tests for this that should be updated? This seems like something we should have covered.
On 2017/05/23 18:01:24, dtapuska wrote: > On 2017/05/23 17:59:38, Navid Zolghadr wrote: > > lgtm. Are there any tests for this that should be updated? This seems like > something we should have covered. Yeah. I'll see what bots will failing and update the tests accordingly.
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 nzolghadr@chromium.org to run a CQ dry run
On 2017/05/23 18:20:42, Navid Zolghadr wrote: > On 2017/05/23 18:01:24, dtapuska wrote: > > On 2017/05/23 17:59:38, Navid Zolghadr wrote: > > > > lgtm. Are there any tests for this that should be updated? This seems like > > something we should have covered. > > Yeah. I'll see what bots will failing and update the tests accordingly. Apparently there was no test for this. I added one.
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2900123002/diff/20001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/fast/dnd/cancel-dragging-outside-page.html (right): https://codereview.chromium.org/2900123002/diff/20001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/fast/dnd/cancel-dragging-outside-page.html:26: eventSender.mouseMoveTo(centerX+200, centerY+200); Is this guaranteed to be outside of the page? Should we just the bounding client rect for the document instead?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2017/05/23 20:50:21, dtapuska wrote: > https://codereview.chromium.org/2900123002/diff/20001/third_party/WebKit/Layo... > File third_party/WebKit/LayoutTests/fast/dnd/cancel-dragging-outside-page.html > (right): > > https://codereview.chromium.org/2900123002/diff/20001/third_party/WebKit/Layo... > third_party/WebKit/LayoutTests/fast/dnd/cancel-dragging-outside-page.html:26: > eventSender.mouseMoveTo(centerX+200, centerY+200); > Is this guaranteed to be outside of the page? Should we just the bounding client > rect for the document instead? We cannot send something outside of the document to the page from eventSender. This 200 is to for starting the drag. The mouseLeave form is the one that sends the leave event.
On 2017/05/24 14:15:16, Navid Zolghadr wrote: > On 2017/05/23 20:50:21, dtapuska wrote: > > > https://codereview.chromium.org/2900123002/diff/20001/third_party/WebKit/Layo... > > File third_party/WebKit/LayoutTests/fast/dnd/cancel-dragging-outside-page.html > > (right): > > > > > https://codereview.chromium.org/2900123002/diff/20001/third_party/WebKit/Layo... > > third_party/WebKit/LayoutTests/fast/dnd/cancel-dragging-outside-page.html:26: > > eventSender.mouseMoveTo(centerX+200, centerY+200); > > Is this guaranteed to be outside of the page? Should we just the bounding > client > > rect for the document instead? > > We cannot send something outside of the document to the page from eventSender. > This 200 is to for starting the drag. The mouseLeave form is the one that sends > the leave event. still lgtm
The CQ bit was checked by nzolghadr@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": 20001, "attempt_start_ts": 1495658704646910,
"parent_rev": "3f0d81259460feaa1075b670f45a2fffb4f6cd9c", "commit_rev":
"12dedc4fa09fc54ee6d0bd54d7ffde5c5ab2bdba"}
Message was sent while issue was closed.
Description was changed from ========== Account for mouse leave events in dragging flow When mouse is outside of the page while dragging if user presses Esc key we get a leave event which seems correct logically. But the code was not handling it properly. BUG=723207, 724316 ========== to ========== Account for mouse leave events in dragging flow When mouse is outside of the page while dragging if user presses Esc key we get a leave event which seems correct logically. But the code was not handling it properly. BUG=723207, 724316 Review-Url: https://codereview.chromium.org/2900123002 Cr-Commit-Position: refs/heads/master@{#474450} Committed: https://chromium.googlesource.com/chromium/src/+/12dedc4fa09fc54ee6d0bd54d7ff... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/12dedc4fa09fc54ee6d0bd54d7ff... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
