Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2751833006:
Treat NotifyVirtual events as mouse move events for blink. (Closed)
DescriptionTreat NotifyVirtual events as mouse move events for blink.
This restores the old logic we had for blink to deal with these as mouse
move events but keeping the mouseleave events when we leave the window.
NotifyInferior messages are already dropped at the X11 desktop tree
host but I didn't want to risk breaking any views with addressing this
release block stable issue.
BUG=701637, 450631, 240300, 352106, 386896, 569998, 575208
Review-Url: https://codereview.chromium.org/2751833006
Cr-Commit-Position: refs/heads/master@{#457411}
Committed: https://chromium.googlesource.com/chromium/src/+/955f4364ffa5c275f04cb3c2d19f7ac63f7d8320
Patch Set 1 #Patch Set 2 : Fix build #Patch Set 3 : Fix ozone build #Patch Set 4 : Try to fix build #
Total comments: 3
Patch Set 5 : Roll all code into one file #Messages
Total messages: 30 (22 generated)
The CQ bit was checked by dtapuska@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...
dtapuska@chromium.org changed reviewers: + sadrul@chromium.org
sadrul@ please take a look this got tagged as a RB-Stable issue.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
The CQ bit was checked by dtapuska@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: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by dtapuska@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: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...) mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by dtapuska@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...
https://codereview.chromium.org/2751833006/diff/60001/ui/events/blink/web_inp... File ui/events/blink/web_input_event.cc (right): https://codereview.chromium.org/2751833006/diff/60001/ui/events/blink/web_inp... ui/events/blink/web_input_event.cc:400: if (native_event && ui::IsVirtualMouseLeaveEvent(*native_event)) { Can you just move IsVirtualMouseLeaveEvent() code in here (either a separate function, or just inline)? Although, what are the 'intermediate windows' you are referring to?
https://codereview.chromium.org/2751833006/diff/60001/ui/events/blink/web_inp... File ui/events/blink/web_input_event.cc (right): https://codereview.chromium.org/2751833006/diff/60001/ui/events/blink/web_inp... ui/events/blink/web_input_event.cc:400: if (native_event && ui::IsVirtualMouseLeaveEvent(*native_event)) { On 2017/03/15 18:38:14, sadrul wrote: > Can you just move IsVirtualMouseLeaveEvent() code in here (either a separate > function, or just inline)? > > Although, what are the 'intermediate windows' you are referring to? Also, should we just not generate a EXITED event in this case (i.e. make a change https://cs.chromium.org/chromium/src/ui/events/x/events_x_utils.cc?sq=package...) to return UNKNWON for NotifyVirtual?
On 2017/03/15 18:48:14, sadrul wrote: > https://codereview.chromium.org/2751833006/diff/60001/ui/events/blink/web_inp... > File ui/events/blink/web_input_event.cc (right): > > https://codereview.chromium.org/2751833006/diff/60001/ui/events/blink/web_inp... > ui/events/blink/web_input_event.cc:400: if (native_event && > ui::IsVirtualMouseLeaveEvent(*native_event)) { > On 2017/03/15 18:38:14, sadrul wrote: > > Can you just move IsVirtualMouseLeaveEvent() code in here (either a separate > > function, or just inline)? > > > > Although, what are the 'intermediate windows' you are referring to? > > Also, should we just not generate a EXITED event in this case (i.e. make a > change > https://cs.chromium.org/chromium/src/ui/events/x/events_x_utils.cc?sq=package...) > to return UNKNWON for NotifyVirtual? I could; albeit I'm not overly familiar with X11 and didn't know if views would be effected. The root cause of the bug is that we started reporting ET_MOUSE_EXITED to blink as such. NotifyInferior is dropped here... https://cs.chromium.org/chromium/src/ui/views/widget/desktop_aura/desktop_win... If we are making a change I think it would be best to do it there. I imagine on middle click the pointer is somehow targeted at a different window in the hierarchy which causes X11 to generate a mouse exited event with NotifyVirtual.
The CQ bit was checked by dtapuska@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...
https://codereview.chromium.org/2751833006/diff/60001/ui/events/blink/web_inp... File ui/events/blink/web_input_event.cc (right): https://codereview.chromium.org/2751833006/diff/60001/ui/events/blink/web_inp... ui/events/blink/web_input_event.cc:400: if (native_event && ui::IsVirtualMouseLeaveEvent(*native_event)) { On 2017/03/15 18:38:14, sadrul wrote: > Can you just move IsVirtualMouseLeaveEvent() code in here (either a separate > function, or just inline)? > > Although, what are the 'intermediate windows' you are referring to? Done.
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 dtapuska@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": 80001, "attempt_start_ts": 1489669239869920,
"parent_rev": "92c8e73cacc372fe7e44adc316c49937821d2d7c", "commit_rev":
"955f4364ffa5c275f04cb3c2d19f7ac63f7d8320"}
Message was sent while issue was closed.
Description was changed from ========== Treat NotifyVirtual events as mouse move events for blink. This restores the old logic we had for blink to deal with these as mouse move events but keeping the mouseleave events when we leave the window. NotifyInferior messages are already dropped at the X11 desktop tree host but I didn't want to risk breaking any views with addressing this release block stable issue. BUG=701637,450631,240300,352106,386896,569998,575208 ========== to ========== Treat NotifyVirtual events as mouse move events for blink. This restores the old logic we had for blink to deal with these as mouse move events but keeping the mouseleave events when we leave the window. NotifyInferior messages are already dropped at the X11 desktop tree host but I didn't want to risk breaking any views with addressing this release block stable issue. BUG=701637,450631,240300,352106,386896,569998,575208 Review-Url: https://codereview.chromium.org/2751833006 Cr-Commit-Position: refs/heads/master@{#457411} Committed: https://chromium.googlesource.com/chromium/src/+/955f4364ffa5c275f04cb3c2d19f... ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://chromium.googlesource.com/chromium/src/+/955f4364ffa5c275f04cb3c2d19f... |
