After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 758167 - Not able to reply in notifications on Wayland
Not able to reply in notifications on Wayland
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: wayland
3.20.x
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
: 766290 769516 770539 (view as bug list)
Depends on:
Blocks: WaylandRelated
 
 
Reported: 2015-11-16 10:33 UTC by Jiri Eischmann
Modified: 2016-08-29 23:15 UTC
See Also:
GNOME target: 3.22
GNOME version: ---


Attachments
events: Only pass key events to Wayland if focus is on the stage (3.70 KB, patch)
2016-03-11 14:59 UTC, Florian Müllner
none Details | Review
events: Only pass key events to Wayland if focus is on the stage (3.35 KB, patch)
2016-03-11 15:38 UTC, Florian Müllner
none Details | Review
events: Only pass key events to Wayland if focus is on the stage (3.38 KB, patch)
2016-03-11 15:50 UTC, Florian Müllner
committed Details | Review

Description Jiri Eischmann 2015-11-16 10:33:39 UTC
I've observed that while running GNOME on Wayland, I'm not able to reply directly in notifications that contain incoming messages (from Polari in this case). The text field at the bottom of the notification is simply inactive.
Comment 1 Matthias Clasen 2016-03-01 21:15:06 UTC
Florian, did you get a fix for this merged ?
Comment 2 Matthias Clasen 2016-03-08 02:55:25 UTC
Florian, any update on this ?
Comment 3 Florian Müllner 2016-03-11 14:59:29 UTC
Created attachment 323717 [details] [review]
events: Only pass key events to Wayland if focus is on the stage

Even without a compositor grab, key events may still be expected to
be processed by the compositor and not applications, for instance
when using ctrl-alt-tab to keynav in the top bar. On X11, focus is
moved to the stage window in that case, so that events are processed
before they are dispatched by the window manager. On wayland, we need
to handle this case ourselves, so make sure to not pass key events to
wayland in that case, and move the key focus back to the stage when
appropriate.
Comment 4 Florian Müllner 2016-03-11 15:38:42 UTC
Created attachment 323718 [details] [review]
events: Only pass key events to Wayland if focus is on the stage

Even without a compositor grab, key events may still be expected to
be processed by the compositor and not applications, for instance
when using ctrl-alt-tab to keynav in the top bar. On X11, focus is
moved to the stage window in that case, so that events are processed
before they are dispatched by the window manager. On wayland, we need
to handle this case ourselves, so make sure to not pass key events to
wayland in that case, and move the key focus back to the stage when
appropriate.
Comment 5 Florian Müllner 2016-03-11 15:50:42 UTC
Created attachment 323719 [details] [review]
events: Only pass key events to Wayland if focus is on the stage

Even without a compositor grab, key events may still be expected to
be processed by the compositor and not applications, for instance
when using ctrl-alt-tab to keynav in the top bar. On X11, focus is
moved to the stage window in that case, so that events are processed
before they are dispatched by the window manager. On wayland, we need
to handle this case ourselves, so make sure to not pass key events to
wayland in that case, and move the key focus back to the stage when
appropriate.
Comment 6 Jasper St. Pierre (not reading bugmail) 2016-03-11 18:09:20 UTC
Review of attachment 323719 [details] [review]:

This is normally handled by GNOME Shell here: https://git.gnome.org/browse/gnome-shell/tree/src/shell-global.c#n581

Why isn't this working? If anything, I would prefer to move this focus manipulation code into mutter.
Comment 7 Rui Matos 2016-03-11 18:26:33 UTC
(In reply to Jasper St. Pierre from comment #6)
> Review of attachment 323719 [details] [review] [review]:
> 
> This is normally handled by GNOME Shell here:
> https://git.gnome.org/browse/gnome-shell/tree/src/shell-global.c#n581
> 
> Why isn't this working? If anything, I would prefer to move this focus
> manipulation code into mutter.

I agree. In this case and also when Ctrl+Alt+Tab'ing to the panel we end up in this state where the input focus is on the clutter stage but we haven't pushModal()d so mutter doesn't setup its internal MetaEventRoute.

I think we shouldn't allow this in the first place and instead ensure that we get a compositor grab.
Comment 8 Jasper St. Pierre (not reading bugmail) 2016-03-11 18:53:38 UTC
That's intentional, we don't want a compositor grab, since it's "too heavy" for what we want to do.
Comment 9 Florian Müllner 2016-03-11 19:08:36 UTC
(In reply to Jasper St. Pierre from comment #6)
> Review of attachment 323719 [details] [review] [review]:
> 
> This is normally handled by GNOME Shell here:
> https://git.gnome.org/browse/gnome-shell/tree/src/shell-global.c#n581
> 
> Why isn't this working?

On Wayland, the stage is always focused, so that function is a no-op there. I think resetting the key-focus in meta_window_focus() as in the patch is enough and we can kill off that function in the shell, but I haven't tried.


(In reply to Rui Matos from comment #7)
> I think we shouldn't allow this in the first place and instead ensure that
> we get a compositor grab.

That would block pointer/touch events as well.
Comment 10 Jasper St. Pierre (not reading bugmail) 2016-03-11 19:18:48 UTC
OK. Some spelunking:

https://git.gnome.org/browse/mutter/commit/?id=04b5232960553b93e70baf0335557b008073c459

Hm. When clicking on the notification, we should set the key focus to the actor, which should take key focus away from the window. At one point in time, my idea is that the Clutter key focus should dictate where key events would get dispatched to, and if that was a window actor, it would be dispatched to Wayland.

That dream died here:

https://git.gnome.org/browse/mutter/commit/?id=7f195aec7a9f5d86464013db94f23db91abc2283
https://git.gnome.org/browse/mutter/commit/?id=23b0f7be4377fdbe026edfede6e237591cc3c72d

... because gnome-shell's actor tracking code didn't like it.

OK. I wonder if we can move the actor/window tracking code entirely into mutter, and then accurately track Clutter key focus for event delivery. I would much prefer this than more special casing of key events.
Comment 11 Rui Matos 2016-03-11 19:39:59 UTC
(In reply to Jasper St. Pierre from comment #8)
> That's intentional, we don't want a compositor grab, since it's "too heavy"
> for what we want to do.

What does too heavy mean? What I'm proposing is basically using a GrabHelper to manage notification focus which is already proven to work reliably for popup menus.
Comment 12 Matthias Clasen 2016-03-17 10:26:10 UTC
Can Rui get an answer ? This bug is still on our target list, so having an agreed-on fix for it would be great
Comment 13 Florian Müllner 2016-03-17 11:12:46 UTC
(In reply to Rui Matos from comment #11)
> (In reply to Jasper St. Pierre from comment #8)
> > That's intentional, we don't want a compositor grab, since it's "too heavy"
> > for what we want to do.
> 
> What does too heavy mean?

As mentioned in comment #9, it affects pointer/touch events in addition to key events.


> What I'm proposing is basically using a GrabHelper
> to manage notification focus which is already proven to work reliably for
> popup menus.

Grabbing the pointer is expected for popup menus. It's not what we currently do for notifications, but I wouldn't mind in that case. But I don't think it's 
right for keynav in panels.
Comment 14 Rui Matos 2016-03-17 16:41:55 UTC
(In reply to Florian Müllner from comment #13)
> As mentioned in comment #9, it affects pointer/touch events in addition to
> key events.

You mean that pointer/touch events outside of the notification actor would get caught? (and we could decide what to do, i.e. drop the grab and return focus to the focused window)

> > What I'm proposing is basically using a GrabHelper
> > to manage notification focus which is already proven to work reliably for
> > popup menus.
> 
> Grabbing the pointer is expected for popup menus. It's not what we currently
> do for notifications, but I wouldn't mind in that case. But I don't think
> it's 
> right for keynav in panels.

I don't see why not. In fact it seems like the sensible thing to do by making all shell chrome interaction more predictable by harmonizing behavior.
Comment 15 Jasper St. Pierre (not reading bugmail) 2016-03-17 16:56:05 UTC
Grabs are heavy. They require an extra click for the user to get out of. A lot of keyboard shortcuts are blocked by the use of grabs.

In fact, one of my least favorite things right now about the shell's input model is how it uses grabs as a replacement against normal input focus, simply because we don't have separate X windows we can focus and restack, we just emulate one with a window shape and grabs.
Comment 16 Florian Müllner 2016-03-17 17:30:54 UTC
(In reply to Rui Matos from comment #14)
> (In reply to Florian Müllner from comment #13)
> > As mentioned in comment #9, it affects pointer/touch events in addition to
> > key events.
> 
> You mean that pointer/touch events outside of the notification actor would
> get caught? (and we could decide what to do, i.e. drop the grab and return
> focus to the focused window)

Yes. Unless we do some replay shenanigans after dropping the grab, we'd be changing behavior. As said, I'm undecided whether that's too bad for notifications ...


> > But I don't think it's right for keynav in panels.
> 
> I don't see why not. In fact it seems like the sensible thing to do by
> making all shell chrome interaction more predictable by harmonizing behavior.

An underline in the top bar (or the legacy tray) doesn't really suggest that you cannot interact with windows (or even change the focus window). Of course we could work around that as well by popping up a lightbox, but at that point we'll let code drive the design, which is backwards.
Comment 17 Florian Müllner 2016-05-11 20:24:30 UTC
*** Bug 766290 has been marked as a duplicate of this bug. ***
Comment 18 maxime.deroucy 2016-07-07 19:21:01 UTC
Just to confirm this bug on Archlinux :

```
max@max-laptop % yaourt -Qs mutter
extra/mutter 3.20.3-1 (gnome)
    A window manager for GNOME
max@max-laptop % yaourt -Qs clutter
extra/clutter 1.26.0-1
    A GObject based library for creating fast, visually rich graphical user interfaces
extra/clutter-gst 3.0.18-1
    GStreamer bindings for clutter
extra/clutter-gst2 2.0.18-1
    GStreamer bindings for clutter
extra/clutter-gtk 1.8.0-1
    GTK clutter widget
```

Do you need some other information.
Comment 19 Florian Müllner 2016-08-04 14:34:51 UTC
*** Bug 769516 has been marked as a duplicate of this bug. ***
Comment 20 Michael Catanzaro 2016-08-25 20:42:42 UTC
Can we make this top priority? I'm told it would be difficult to find a proper solution for this, and I hate asking volunteers to do work, but we do have very quick options that are better than releasing broken:

 a) Commit the proposed hack, despite Jasper's objection; or
 b) Remove the reply entry when running under Wayland; or
 c) Totally disable the Telepathy client when running under Wayland
Comment 21 Jonas Ådahl 2016-08-26 09:15:20 UTC
I'd be fine with with a stopgap solution until we have the new ClutterGrab API Carlos is working on.
Comment 22 Florian Müllner 2016-08-29 11:18:28 UTC
*** Bug 770539 has been marked as a duplicate of this bug. ***
Comment 23 Florian Müllner 2016-08-29 11:38:56 UTC
(In reply to Michael Catanzaro from comment #20)
>  b) Remove the reply entry when running under Wayland; or
>  c) Totally disable the Telepathy client when running under Wayland

Of course those only "fix" the most visible symptom, but not the broken keynav in top bar and legacy tray.
Comment 24 Michael Catanzaro 2016-08-29 12:12:31 UTC
(In reply to Florian Müllner from comment #23)
> (In reply to Michael Catanzaro from comment #20)
> >  b) Remove the reply entry when running under Wayland; or
> >  c) Totally disable the Telepathy client when running under Wayland
> 
> Of course those only "fix" the most visible symptom, but not the broken
> keynav in top bar and legacy tray.

The broken keynav is sad, but not a Wayland blocker. ;)
Comment 25 Rui Matos 2016-08-29 13:41:49 UTC
(In reply to Jasper St. Pierre (not reading bugmail) from comment #6)
> Review of attachment 323719 [details] [review] [review]:
> 
> This is normally handled by GNOME Shell here:
> https://git.gnome.org/browse/gnome-shell/tree/src/shell-global.c#n581
> 
> Why isn't this working? If anything, I would prefer to move this focus
> manipulation code into mutter.

This is working but what it does is setting the X input focus to the clutter stage X window and it's a NOP when we're a wayland compositor.
Comment 26 Rui Matos 2016-08-29 13:43:00 UTC
Review of attachment 323719 [details] [review]:

I think this is a fine solution, at least for now.
Comment 27 Florian Müllner 2016-08-29 23:15:03 UTC
Attachment 323719 [details] pushed as 89f6fdc - events: Only pass key events to Wayland if focus is on the stage