Created attachment 208627 [details] [review]
st-scroll-bar: use clutter_grab_pointer()

StScrollBar was intercepting motion events by using captured-event on
the stage, which required additional dirty tricks, which required
additional hacks. Simplify it by just using clutter_grab_pointer()
instead.

https://bugzilla.gnome.org/show_bug.cgi?id=648788

Created attachment 208628 [details] [review]
Introduce a new GrabHelper

PopupMenu.PopupMenuManager and MessageTray.FocusGrabber had a lot of
code in common. Let's refactor this common code out into a new class,
"GrabHelper". This replaces FocusGrabber completely, and nukes half
of PopupMenuManager.

Based on a patch by Dan Winship <danw@gnome.org>

https://bugzilla.gnome.org/show_bug.cgi?id=643687

Created attachment 208629 [details] [review]
grabHelper: Explicitly grab the key focus if we can't navigate inside

GrabHelper assumes that we've always grabbed the key focus if we've called
navigate. If the widget or any of its descendents are focusable, that's
a bogus assumption. Fix it by explicitly grabbing the key focus if we've
failed to navigate inside the widget.

Created attachment 208630 [details] [review]
messageTray: Make sure to make the summary box pointer a grab parent

The summary box pointer isn't a child of the message tray, so in order to
prevent losing grab focus when showing the summary box pointer we need
to add it as an allowed grab parent.

*** Bug 670639 has been marked as a duplicate of this bug. ***

*** Bug 670641 has been marked as a duplicate of this bug. ***

Review of attachment 208627 [details] [review]:

The differences to Dan's original patch look pretty minimal, so you should leave the original author in place. Other than that looks good (oh, there are also two bug URLs, which is a bit confusing)

Created attachment 208634 [details] [review]
st-scroll-bar: use clutter_grab_pointer()

StScrollBar was intercepting motion events by using captured-event on
the stage, which required additional dirty tricks, which required
additional hacks. Simplify it by just using clutter_grab_pointer()
instead.



OK, figured out how to do that.

Created attachment 208716 [details] [review]
st-scroll-bar: use clutter_grab_pointer()

StScrollBar was intercepting motion events by using captured-event on
the stage, which required additional dirty tricks, which required
additional hacks. Simplify it by just using clutter_grab_pointer()
instead.

Created attachment 208717 [details] [review]
Introduce a new GrabHelper

PopupMenu.PopupMenuManager and MessageTray.FocusGrabber had a lot of
code in common. Let's refactor this common code out into a new class,
"GrabHelper". This replaces FocusGrabber completely, and nukes half
of PopupMenuManager.

Based on a patch by Dan Winship <danw@gnome.org>

https://bugzilla.gnome.org/show_bug.cgi?id=643687

Created attachment 208718 [details] [review]
grabHelper: Explicitly grab the key focus if we can't navigate inside

GrabHelper assumes that we've always grabbed the key focus if we've called
navigate. If the widget or any of its descendents are focusable, that's
a bogus assumption. Fix it by explicitly grabbing the key focus if we've
failed to navigate inside the widget.

Created attachment 208719 [details] [review]
messageTray: Make sure to make the summary box pointer a grab parent

The summary box pointer isn't a child of the message tray, so in order to
prevent losing grab focus when showing the summary box pointer we need
to add it as an allowed grab parent.



Rebase the patch-set onto master.

Review of attachment 208716 [details] [review]:

::: js/ui/popupMenu.js
@@ -842,3 @@
-        // Can be set while a menu is up to let all events through without special
-        // menu handling useful for scrollbars in menus, and probably not otherwise.
-        this.passEvents = false;

Removing passEvents breaks the combo box in the user menu (open the child menu, then click somewhere else to dismiss it - breakage is particularly bad when "somewhere else" is an item in the parent menu)

Review of attachment 208718 [details] [review]:

This one looks good

Review of attachment 208719 [details] [review]:

Not sure why this is separate from the original grabHelper patch

Review of attachment 208719 [details] [review]:

I'll squash it when I commit it, I guess.

Created attachment 208734 [details] [review]
messageTray: Make sure to always grab focus

If a widget isn't focusable or none of its children are focusable, then
navigate_focus will return false and the key focus won't be set. We
need to explicitly grab the key focus in this case.



Ugh, so trying to fix grabhelper to handle child menus is a big pain...

I have a plan for how to refactor that in the future, but for now, let's
just take the easy way out.

Created attachment 208735 [details] [review]
messageTray: Make sure the summary box pointer is included in "contains" checks

The summary box pointer isn't a part of this.actor, it's added directly to the
uiGroup. If the focused actor changes to the summary box pointer, we need to
make sure that we aren't ungrabbing the focus, as it's technically part of
the message tray.

Review of attachment 208735 [details] [review]:

Ugh, this is just wrong.

Review of attachment 208734 [details] [review]:

Sure, go ahead ...

Attachment 208734 [details] pushed as 7d29e69 - messageTray: Make sure to always grab focus


So it seems that the other commit is only required for the GrabHelper port - this should fix the message tray entirely, here.