Send mouse exited event after mouse release outside view.

Send mouse exited event after mouse release outside view.

In case when mouse is released outside view, this view do not receive the mouse exited event.

Fix for this case.

[snake, 2016-10-28 16:02:05]

Description was changed from

==========
Send mouse exited event after mouse release outside view.

In case when mouse is released outside view, this view do not receive the mouse
exited event.

Fix for this case.
==========

to

==========
Send mouse exited event after mouse release outside view.

In case when mouse is released outside view, this view do not receive the mouse
exited event.

Fix for this case.
==========

[snake, 2016-10-28 16:02:05]

art-snake@yandex-team.ru changed reviewers:
+ sadrul@chromium.org, sky@chromium.org

[sadrul, 2016-10-28 16:07:39]

https://codereview.chromium.org/2456903003/diff/1/ui/views/widget/root_view.cc
File ui/views/widget/root_view.cc (right):

https://codereview.chromium.org/2456903003/diff/1/ui/views/widget/root_view.c...
ui/views/widget/root_view.cc:462: OnMouseMoved(GeneratedMouseMoveEvent(event));
Always sending a mouse-move after a mouse-release doesn't sound like a good idea
to me.

[snake, 2016-10-28 16:21:38]

https://codereview.chromium.org/2456903003/diff/1/ui/views/widget/root_view.cc
File ui/views/widget/root_view.cc (right):

https://codereview.chromium.org/2456903003/diff/1/ui/views/widget/root_view.c...
ui/views/widget/root_view.cc:462: OnMouseMoved(GeneratedMouseMoveEvent(event));
On 2016/10/28 16:07:39, sadrul wrote:
> Always sending a mouse-move after a mouse-release doesn't sound like a good
idea
> to me.
But we should do this, because View under cursor (which may not
mouse_pressed_handler), don't know that mouse was moved on it before. This is
just post notification about this.

Should I write test on this case?
  The test will be:
   MouseDown on first view.
   Move mouse to second view
   Release mouse on second view.
   Second view should receive mouse exited and mouse moved notifications.

[sadrul, 2016-10-31 14:59:58]

On 2016/10/28 16:21:38, snake wrote:
> https://codereview.chromium.org/2456903003/diff/1/ui/views/widget/root_view.cc
> File ui/views/widget/root_view.cc (right):
> 
>
https://codereview.chromium.org/2456903003/diff/1/ui/views/widget/root_view.c...
> ui/views/widget/root_view.cc:462:
OnMouseMoved(GeneratedMouseMoveEvent(event));
> On 2016/10/28 16:07:39, sadrul wrote:
> > Always sending a mouse-move after a mouse-release doesn't sound like a good
> idea
> > to me.
> But we should do this, because View under cursor (which may not
> mouse_pressed_handler), don't know that mouse was moved on it before. This is
> just post notification about this.
> 
> Should I write test on this case?
>   The test will be:
>    MouseDown on first view.
>    Move mouse to second view
>    Release mouse on second view.
>    Second view should receive mouse exited and mouse moved notifications.

This makes sense. I tested on a Windows build in the toolbar: I press the
mouse-button down on the refresh button, and drag the cursor over the back
button. As soon as I release the mouse-button, the back button gains the hover
effect, i.e. it seems to be receiving some event. So this is probably working
OK?

Also, with your change, it looks like the mouse-move will always be generated,
even if the press+release happened on the same View. We should not do that.

[snake, 2016-10-31 16:05:33]

On 2016/10/31 14:59:58, sadrul wrote:
> On 2016/10/28 16:21:38, snake wrote:
> >
https://codereview.chromium.org/2456903003/diff/1/ui/views/widget/root_view.cc
> > File ui/views/widget/root_view.cc (right):
> > 
> >
>
https://codereview.chromium.org/2456903003/diff/1/ui/views/widget/root_view.c...
> > ui/views/widget/root_view.cc:462:
> OnMouseMoved(GeneratedMouseMoveEvent(event));
> > On 2016/10/28 16:07:39, sadrul wrote:
> > > Always sending a mouse-move after a mouse-release doesn't sound like a
good
> > idea
> > > to me.
> > But we should do this, because View under cursor (which may not
> > mouse_pressed_handler), don't know that mouse was moved on it before. This
is
> > just post notification about this.
> > 
> > Should I write test on this case?
> >   The test will be:
> >    MouseDown on first view.
> >    Move mouse to second view
> >    Release mouse on second view.
> >    Second view should receive mouse exited and mouse moved notifications.
> 
> This makes sense. I tested on a Windows build in the toolbar: I press the
> mouse-button down on the refresh button, and drag the cursor over the back
> button. As soon as I release the mouse-button, the back button gains the hover
> effect, i.e. it seems to be receiving some event. So this is probably working
> OK?
I have investigate, that on Windows, the OS already send us the Generated
MouseMove event after releasing of mouse button.
But on MacViews(and may be on Linux) this is not true. And we should generate
this event.


> 
> Also, with your change, it looks like the mouse-move will always be generated,
> even if the press+release happened on the same View. We should not do that.

The Windows already have this logic in native.

[snake, 2016-11-02 12:54:11]

ping?

[snake, 2016-11-09 19:27:41]

On 2016/11/02 12:54:11, snake wrote:
> ping?

ping?

[snake, 2016-11-09 19:28:05]

The CQ bit was checked by art-snake@yandex-team.ru to run a CQ dry run

[commit-bot: I haz the power, 2016-11-09 19:28:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-09 19:31:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-09 19:31:37]

Dry run: Try jobs failed on following builders:
  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_...)

[snake, 2016-11-09 20:08:26]

The CQ bit was checked by art-snake@yandex-team.ru to run a CQ dry run

[commit-bot: I haz the power, 2016-11-09 20:08:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-09 20:46:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-09 20:46:55]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[sadrul, 2016-11-10 20:20:56]

On 2016/10/31 16:05:33, snake wrote:
> On 2016/10/31 14:59:58, sadrul wrote:
> > On 2016/10/28 16:21:38, snake wrote:
> > >
> https://codereview.chromium.org/2456903003/diff/1/ui/views/widget/root_view.cc
> > > File ui/views/widget/root_view.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2456903003/diff/1/ui/views/widget/root_view.c...
> > > ui/views/widget/root_view.cc:462:
> > OnMouseMoved(GeneratedMouseMoveEvent(event));
> > > On 2016/10/28 16:07:39, sadrul wrote:
> > > > Always sending a mouse-move after a mouse-release doesn't sound like a
> good
> > > idea
> > > > to me.
> > > But we should do this, because View under cursor (which may not
> > > mouse_pressed_handler), don't know that mouse was moved on it before. This
> is
> > > just post notification about this.
> > > 
> > > Should I write test on this case?
> > >   The test will be:
> > >    MouseDown on first view.
> > >    Move mouse to second view
> > >    Release mouse on second view.
> > >    Second view should receive mouse exited and mouse moved notifications.
> > 
> > This makes sense. I tested on a Windows build in the toolbar: I press the
> > mouse-button down on the refresh button, and drag the cursor over the back
> > button. As soon as I release the mouse-button, the back button gains the
hover
> > effect, i.e. it seems to be receiving some event. So this is probably
working
> > OK?
> I have investigate, that on Windows, the OS already send us the Generated
> MouseMove event after releasing of mouse button.
> But on MacViews(and may be on Linux) this is not true. And we should generate
> this event.
> 
> 
> > 
> > Also, with your change, it looks like the mouse-move will always be
generated,
> > even if the press+release happened on the same View. We should not do that.
> 
> The Windows already have this logic in native.

Not sure what you mean here. To clarify: if there's a View A, and there's a
mouse press+release on A, then we should not synthesize a mouse-move event. Can
you add a test that verifies that this is the case?

[sadrul, 2016-11-10 20:21:20]

(and sorry for sitting on this for so long. I meant to respond sooner)