GNOME Bugzilla – Bug 613543
OSD's don't work when in overview mode
Last modified: 2013-03-03 12:09:36 UTC
OSD's like changing volume or brightness don't work when in overview mode.
*** Bug 633577 has been marked as a duplicate of this bug. ***
This is actually two bugs: 1) special keys are grabbed when in overview mode and thus are not delivered to gnome-power-manager (brightness) / gnome-settings-daemon (screen rotation, volume) / gnome-session (logout) 2) OSD windows are not shown in overview mode (dup of bug 607029)
> 1) special keys are grabbed when in overview mode and thus are not delivered to > gnome-power-manager (brightness) / gnome-settings-daemon (screen rotation, > volume) / gnome-session (logout) Note that all those key shortcuts are now handled by gnome-settings-daemon itself.
*** Bug 664192 has been marked as a duplicate of this bug. ***
well, I confirm this bug too.
I confirm it too. It's quite annoying when you use special keys for volume up/down for example.
Please also mention your GNOME version and distro when confirming - thanks!
Sorry for that André, I'm quite new to this and do not have all the good practices. Here's the completed comment : I can confirm this bug on Fedora 16 with gnome-shell 3.2.2.1.
I would assume this is also the same bug that inhibits the changing of brightness and volume through the keys on the new login screen? Fedora 16 with gnome-shell 3.2.2.1 by the way.
Ok, GrabOverride was supposed to save us all, but it didn't happen, and this bug is extremely annoying, so I decided to go all the way down and reimplement keybinding handling in the shell directly. It actually turned out to be quite easy, with good code clean ups in mutter and the already present shell keybindings.
Created attachment 217081 [details] [review] Keybindings: add a mechanism to grab a specific key combination Instead of requiring a GSettings object, allow to hardcode a specific combination for a keybinding. This will be used by the Shell to replace the media keys plugin in gnome-settings-daemon.
Created attachment 217082 [details] [review] Handle some keybindings even when a compositor grab is active Do not ignore all key events automatically when a compositor grab is active, and introduce a flag for masking which keybindings should be active. This does not mean that automatically all keybindings are active when the compositor is modal, it merely moves the policy down to the handler.
Created attachment 217083 [details] [review] Allow a keybinding handler to ignore a keybinding Previous commit moved policy for keybindings when grabbed down to the handler, but did not replay the event if it is was not handled. This commit adds the missing bit.
Created attachment 217084 [details] [review] Keybindings: uniquify the name for non-builtin keybindings Multiple settings objects could have the same key, so that alone is not enough to identify the binding. Add also the pointer value of the GSettings object.
Created attachment 217085 [details] [review] Main: filter keybindings when the shell is modal Mutter now handles some keybindings even when the compositor is grabbed, so we need to filter the invocation at the handler level. This allows to finally get rid of a old hack.
Created attachment 217086 [details] [review] Handle global keybindings in the shell Handling global keybindings, such as volume and brightness keys but also custom keybindings, directly in the compositor is the only way to deal with grabs and modal operations that could be active (primarily the overview, for which special policy was introduced in the last commit)
Created attachment 217088 [details] [review] Disable media-keys plugin when gnome-shell is active Introduce a small utility for marking a plugin as "fallback only" (so automatically disabled when gnome-shell is on the bus), and use it for media-keys.
I have a set of patches that I'm going to work on to allow override-redirect windows in the overview. Really, we shouldn't even be managing OR windows; it's only because of composite that we can. This should remove the OSD implementation from gnome-shell, something which I'm not a fan of.
( I also tried something like this approach in https://bugzilla.gnome.org/show_bug.cgi?id=665547 , but didn't get as far as you )
Review of attachment 217084 [details] [review]: This is semi-related to https://bugzilla.gnome.org/show_bug.cgi?id=666513 , which Florian didn't like. I'm for it, but it's unfortunate that we couldn't get a patch in for this before that.
Review of attachment 217084 [details] [review]: ::: src/core/keybindings.c @@ +686,3 @@ + name = g_strdup_printf ("custom-keybinding-%p-%s", settings, key); + else + name = g_strdup (key); This should really be g_strdup_printf ("internal-keybinding-%s", key); to reduce collisions even further.
Review of attachment 217083 [details] [review]: Makes sense. ::: src/core/keybindings.c @@ +1401,1 @@ invoke_handler_by_name (MetaDisplay *display, It doesn't look like you ever respect the return value of invoke_handler_by_name.
Review of attachment 217086 [details] [review]: Not a fan. I'd rather you expose a DBus service that allows g-s-d to get a keybinding.
Review of attachment 217088 [details] [review]: No.
Review of attachment 217085 [details] [review]: I really don't understand how this is supposed to work -- what's stopping something like Alt-Tab from being executed in the overview? ::: js/misc/util.js @@ +303,3 @@ + * with Meta.KeybindingFlags.HANDLE_WHEN_GRABBED + */ +function wrapKeybinding(handler, onlyInOverview) { I don't think this belongs in util. And you don't ever pass false for 'onlyInOverview'.
Review of attachment 217085 [details] [review]: Er, nevermind. I see you have the magic flag.
Review of attachment 217082 [details] [review]: Yeah, this makes sense. Your commit message is a bit misleading -- as of this commit, panel-main-menu, etc. will be active when the compositor is modal, regardless of the policy of a handler.
Review of attachment 217081 [details] [review]: I'm not a fan of a generic key grab API.
(In reply to comment #23) > Review of attachment 217086 [details] [review]: > > Not a fan. I'd rather you expose a DBus service that allows g-s-d to get a > keybinding. I don't see what would be the point. You'd still need to watch GSettings in the shell and grab each key (or worse, reimplement keybindings in _globalKeyHandler). There would be no performance advantage, since the individual actions are all very simple. Also, you'd still have the problem that changing g-s-d for new keybindings requires matching shell changes, so if you're doing that you may just as well add the JS implementation. As for the OSD, all UI should live in the shell. Having OR windows in the overview is wrong, and would break with windows created by non standard apps (wine for example creates OR windows in some cases, and for reasons unknown to me...). (Not to mention that at some point we'll move to wayland, and there is no place in that for OR or passive grabs...)
(In reply to comment #29) > (In reply to comment #23) > > Review of attachment 217086 [details] [review] [details]: > > > > Not a fan. I'd rather you expose a DBus service that allows g-s-d to get a > > keybinding. > > I don't see what would be the point. > You'd still need to watch GSettings in the shell and grab each key (or worse, > reimplement keybindings in _globalKeyHandler). I guess if we add a generic grab key/ungrab key API, it wouldn't be so bad. > There would be no performance > advantage, since the individual actions are all very simple. > Also, you'd still have the problem that changing g-s-d for new keybindings > requires matching shell changes, so if you're doing that you may just as well > add the JS implementation. I do not understand? We'd expose something like: org.gnome.SettingsDaemon.MediaKeys.KeyGrabber and then we'd have one implementation in the shell and then a fallback implementation in g-s-d. What matching shell changes would that require other than exporting a grab/ungrab API over DBus? > As for the OSD, all UI should live in the shell. Having OR windows in the > overview is wrong, and would break with windows created by non standard apps > (wine for example creates OR windows in some cases, and for reasons unknown to > me...). wtf. Still, having OR windows in the overview has its advantages. Maybe we could have a special window property.
Comment on attachment 217082 [details] [review] Handle some keybindings even when a compositor grab is active That is not really what we want - we do want some keybindings to be processed in situations where we have a grab, but we don't want to treat all grabs equally. For instance, we do want switch-workspace to be handled when in the overview (which this patch does), but not in other situations like while showing a system modal or in the lock screen.
Comment on attachment 217082 [details] [review] Handle some keybindings even when a compositor grab is active Sorry, I should have kept reading ... Still, I don't really like how the filtering is spread all over the place (e.g. for built-in bindings, it's in init_builtin_key_bindings(), the built-in handler and the custom handler). Can't we have a hook in the compositor (something like meta_compositor_filter_keybinding(display->compositor, binding))?
*** Bug 658317 has been marked as a duplicate of this bug. ***
Apparently this, in addition to being an extremely irritating bug, is a dependence of bug 682229, which is a blocker for 3.6, so let's see if we can get it going.
Apparently I thought bug 643111 was the "canonical" bug for keybinding overhaul, so I started working on this before I became aware of the patches here - I'm still yak-shaving, but so far what's outlined in comment #32 works quite well (including a fix for bug 682229 and a couple of other issues), so I'd like to at least put up my patches for discussion as well. Will take me another day or so though ...
Uh, ok, I wasn't aware of bug 643111. We can move the discussion there and leave this bug for the UI if you prefer, although IIRC Jasper had a patch somewhere to avoid messing with OR windows.
(In reply to comment #36) > Uh, ok, I wasn't aware of bug 643111. > We can move the discussion there and leave this bug for the UI if you prefer, > although IIRC Jasper had a patch somewhere to avoid messing with OR windows. I don't think I ever published it anywhere.
(In reply to comment #36) > Uh, ok, I wasn't aware of bug 643111. > We can move the discussion there and leave this bug for the UI if you prefer, > although IIRC Jasper had a patch somewhere to avoid messing with OR windows. Are you referring to bug 679500? I still think it is a bad idea, as external UI like docky, awn, maliit etc. will suddenly show up in "random" places in the overview, blocking whatever happens to be positioned below.
(In reply to comment #32) > (From update of attachment 217082 [details] [review]) > Sorry, I should have kept reading ... > > Still, I don't really like how the filtering is spread all over the place (e.g. > for built-in bindings, it's in init_builtin_key_bindings(), the built-in > handler and the custom handler). Can't we have a hook in the compositor > (something like meta_compositor_filter_keybinding(display->compositor, > binding))? I think it's cleaner to handle this in the individual handlers (through Util.wrapKeybinding). The alternative is having another table in JS, to associate each binding with its policy (be it an enum or a function call).
Created attachment 222874 [details] [review] Keybindings: add a mechanism to grab a specific key combination Instead of requiring a GSettings object, allow to hardcode a specific combination for a keybinding. This will be used by the Shell to replace the media keys plugin in gnome-settings-daemon. Even if g-s-d is kept as the central place of global keybindings, this is needed.
Created attachment 222875 [details] [review] Handle some keybindings even when a compositor grab is active Do not ignore all key events automatically when a compositor grab is active, and introduce a flag for masking which keybindings should be active. This does not mean that automatically all keybindings are active when the compositor is modal, it merely moves the policy down to the handler. Rebased on the overlay key changes.
Created attachment 222876 [details] [review] Keybindings: uniquify the name for non-builtin keybindings Multiple settings objects could have the same key, so that alone is not enough to identify the binding. Add also the pointer value of the GSettings object. Rebased and comment addressed.
(In reply to comment #39) > I think it's cleaner to handle this in the individual handlers (through > Util.wrapKeybinding). > The alternative is having another table in JS, to associate each binding with > its policy (be it an enum or a function call). Yeah, that's what I'm doing, to allow for finer grained control of which keybindings are available in a specific mode (for instance, we want SWITCH_WORKSPACE in the overview but not in the lock screen, SWITCH_PANEL on the other hand should be available in both).
Created attachment 222877 [details] [review] Main: filter keybindings when the shell is modal Mutter now handles some keybindings even when the compositor is grabbed, so we need to filter the invocation at the handler level. This allows to finally get rid of a old hack. Rebased and tested with screen lock and modal message tray.
Created attachment 222878 [details] [review] Handle global keybindings in the shell Handling global keybindings, such as volume and brightness keys but also custom keybindings, directly in the compositor is the only way to deal with grabs and modal operations that could be active (primarily the overview, for which special policy was introduced in the last commit) I'm attaching this one too, in case during 3.7 cycle we drop fallback mode, which is the only reason for the ugly hack of handling keybindings through dbus.
Does the patch also enable printscreen in overview? Or should I create another bug?
Fallback mode is officially dropped, media-keys in g-s-d is legacy. I believe it makes no sense for gnome-shell to grab the keys, turn the X event into a dbus message and then send that to gsd. It is definitely easier and more performant to handle them in the shell. Therefore, I'm attaching the complete mutter+gnome-shell patchset again, rebased on current master, tested and polished. Florian, if you have working code for the DBus mechanism, could you post that here too? Btw, the patchset, up to but not including the last commit for gnome-shell, is valid even if we decide not to move media keys handling in the shell, as it cleans up handling when modal and extension keybindings registration. Also, as a nice side effect, with these I see fixed both bug 683024 and bug 687922, because overlay key handling is consolidated in mutter.
Created attachment 228627 [details] [review] Keybindings: add a mechanism to grab a specific key combination Instead of requiring a GSettings object, allow to hardcode a specific combination for a keybinding. This will be used by the Shell to replace the media keys plugin in gnome-settings-daemon. (This might or might not be needed)
Created attachment 228628 [details] [review] Allow a keybinding handler to ignore a keybinding Previous commit moved policy for keybindings when grabbed down to the handler, but did not replay the event if it is was not handled. This commit adds the missing bit.
Created attachment 228629 [details] [review] Handle some keybindings even when a compositor grab is active Do not ignore all key events automatically when a compositor grab is active, and introduce a flag for masking which keybindings should be active. This does not mean that automatically all keybindings are active when the compositor is modal, it merely moves the policy down to the handler.
Created attachment 228630 [details] [review] Keybindings: uniquify the name for non-builtin keybindings Multiple settings objects could have the same key, so that alone is not enough to identify the binding. Add also the pointer value of the GSettings object.
Created attachment 228631 [details] [review] Main: filter keybindings when the shell is modal Mutter now handles some keybindings even when the compositor is grabbed, so we need to filter the invocation at the handler level. This allows to finally get rid of a old hack.
Created attachment 228632 [details] [review] Prefix keybinding names with 'internal-keybinding-' This is what mutter does internally now, to disambiguate them with custom keybindings introduced with display.add_keybinding().
Created attachment 228633 [details] [review] Handle global keybindings in the shell Handling global keybindings, such as volume and brightness keys but also custom keybindings, directly in the compositor is the only way to deal with grabs and modal operations that could be active (primarily the overview, for which special policy was introduced in the last commit)
To aid testing, I also pushed wip/media-keys branches of mutter and gnome-shell. When trying them, don't forget to run gsettings set org.gnome.settings-daemon.plugins.media-keys active false
Did this code regress wrt. handling device specific volume? In g-s-d we used XI2 to change the volume on speakers and headsets rather than the default ouput when the buttons on those were used. Is XI2 key capture available?
(In reply to comment #56) > Did this code regress wrt. handling device specific volume? In g-s-d we used > XI2 to change the volume on speakers and headsets rather than the default ouput > when the buttons on those were used. > Is XI2 key capture available? Ugh, no, mutter doesn't do XI2 yet. I could try rebasing wip/media-keys on top of wip/xinput2r and see what happens...
That branch is going to be replaced soon. Owen's unhappy with the current state, as it introduces a lot of complexity (multiple grabs, multiple pointers, etc.) for little to no gain, and it also supported Core Events. I'm going to rewrite it to simply use XI2 instead of Core Events. This should give us most things without a lot of drawbacks.
(In reply to comment #47) > Florian, if you have working code for the DBus mechanism, could you post that > here too? I'm still missing some g-s-d bits, so I've only posted the non-grab set so far, e.g. what I consider a cleaner approach for moving keybinding handling into mutter (comment #32). It also adds a keybinding-mode system, which allows to have different sets of keybindings in different modes (overview, login screen, message tray etc.). Given that the two patch sets have conflict written all over them (though I'm sure both the media-keys patch and the keybinding-mode set could be rebased on the "other" approach), I opened bug 688202 instead.
*** Bug 691464 has been marked as a duplicate of this bug. ***
I've two keyboard layouts in my system: english and russian. I've used gnome-tweak-tool to set caps lock as kb layout switch Then I'm press WIN key (or move mouse into upper-left corner), gnome-shell menu appears. But it's impossible to switch kb layout while menu is active (and caps lock works changes letters case in this menu). So if I'm chatting in russian, then press WIN and trying to type TERMINAL in search menu, I can't do this, I should switch layout, then launch menu, then type what I want. I've reported this behaviour in 691464. Is it really the same problem? If so, WHY it's still unconfirmed?
(In reply to comment #61) > I've reported this behaviour in 691464. Is it really the same problem? If so, > WHY it's still unconfirmed? Because we don't use the UNCONFIRMED state. The fact that this patch has half-a-dozen patches from gnome-shell developers should make it clear that it's being worked on.
This may be fixed with bug 688202. It is unfortunate that that fix missed 3.6.2. I only use stable versions.
(In reply to comment #63) > This may be fixed with bug 688202. It isn't. > It is unfortunate that that fix missed 3.6.2. I only use stable versions. It won't be fixed in GNOME 3.6.x at all.
I was about to file a bug for OSDs not showing up when a GNOME Shell menu is open (battery status, for example). As I'm running Shell 3.6, pinging first to see if this issue is related to the bug being discussed here, as I suspect.
Ok, so most of the stuff here is obsolete, now that Florian landed bug 643111. But we still need to handle the OSDs. Two possibilities exists: a) We can special case windows from gnome-settings-daemon (with WM_CLASS or a special property), place them in a different window group and paint them normally in the overview. b) We can move OSD rendering in the shell, and have gsd ask the shell when it wants to show something. b) is probably simpler, but a) should have better performance (less dbus traffic, less code in the shell)
(In reply to comment #66) > Two possibilities exists: > a) We can special case windows from gnome-settings-daemon (with WM_CLASS or a > special property), place them in a different window group and paint them > normally in the overview. > b) We can move OSD rendering in the shell, and have gsd ask the shell when it > wants to show something. > > b) is probably simpler, but a) should have better performance (less dbus > traffic, less code in the shell) I'm currently working on the g-s-d side of (b), the shell side is pretty much done. It's not a lot of code, and looking at all the hoops g-s-d has to jump though to draw a translucent rectangle with rounded corners, it makes a lot of sense to move this into the shell (where the same is a simple widget + a couple of lines of css). I'll attach patches in a bit.
Oh, nice, because I implemented a) after I wrote that comment, and it is simple after all. I'll attach anyway, and then you can decide which way to go. (Because, you know, grabbing the key in mutter, sending a DBus message to gsd, comparing it to GSettings, and then sending a DBus message again to gnome-shell doesn't strike me as the best possible design...)
Created attachment 237814 [details] [review] Add a new stacking layer for OSD windows. OSD windows are the OR windows created by gnome-settings-daemon. The new layer is required to paint them above the overlay layer, so they're visible in the gnome-shell overview.
Created attachment 237815 [details] [review] Layout: add support for the OSD window group The OSD window group is the new window group holding gnome-settings-daemon OSD windows (such as volume or brightness). The stacking order then becomes: window group -> overlay group (overview) -> chrome (panel) -> top window group -> osd window group -> screenshield -> chrome (tray, keyboard) -> chrome (popup menus) -> other
(In reply to comment #67) > (In reply to comment #66) > > Two possibilities exists: > > a) We can special case windows from gnome-settings-daemon (with WM_CLASS or a > > special property), place them in a different window group and paint them > > normally in the overview. > > b) We can move OSD rendering in the shell, and have gsd ask the shell when it > > wants to show something. > > > > b) is probably simpler, but a) should have better performance (less dbus > > traffic, less code in the shell) > > I'm currently working on the g-s-d side of (b), the shell side is pretty much > done. It's not a lot of code, and looking at all the hoops g-s-d has to jump > though to draw a translucent rectangle with rounded corners, it makes a lot of > sense to move this into the shell (where the same is a simple widget + a couple > of lines of css). That, and I've really wanted the OSDs to be styled the same way for volume popups and shell ones, and stop them clashing (have them disappear when the alt-tab popup shows up for example). That's not possible from g-s-d. I don't see a major problem with the back-and-forth from g-s to g-s-d, there's plenty of things that g-s-d can know better than g-s when it comes to configuration.
(In reply to comment #71) > That, and I've really wanted [to] stop them clashing (have them disappear when the > alt-tab popup shows up for example). Oh, that makes a lot of sense, added that to the patches.
Created attachment 237835 [details] [review] osdWindow: Add a simple OSD popup Implement a basic OSD popup that shows an icon and optionally a label and a fill level. It is based on the existing OSD implementation in gnome-settings-daemon, which it will replace.
Created attachment 237836 [details] [review] shellDBus: Export ShowOSD() method on the bus Export a simple method to trigger an OSD popup. gnome-settings-daemon will use this to replace its own OSD UI. I have been wondering if it makes sense to restrict this method - I can imagine that this might be useful for applications like Totem as well, but maybe we should only fulfill the request if the sender is either g-s-d or the currently focused application?
Created attachment 237837 [details] [review] switcherPopup: Cancel the OSD popup before showing It is irritating to show two different system popups at the same time, so let switcher popups like alt-tab cancel out the OSD.
Review of attachment 237835 [details] [review]: I know I was opposed to OSDs in gnome-shell for a long time, but this is really not that much code. I'm impressed.
Review of attachment 237837 [details] [review]: OK.
Review of attachment 237836 [details] [review]: Minor style nit. ::: js/ui/shellDBus.js @@ +121,3 @@ + let icon = null; + if (params['gicon']) + icon = Gio.icon_new_for_string(params['gicon']); Gio.Icon.new_for_string. @@ +123,3 @@ + icon = Gio.icon_new_for_string(params['gicon']); + else if (params['icon']) + icon = new Gio.ThemedIcon({ name: params['icon'] }); Why support both?
Attachment 237835 [details] pushed as 19533f8 - osdWindow: Add a simple OSD popup Attachment 237836 [details] pushed as 90ea27c - shellDBus: Export ShowOSD() method on the bus Attachment 237837 [details] pushed as deb77a4 - switcherPopup: Cancel the OSD popup before showing (In reply to comment #78) > @@ +123,3 @@ > + icon = Gio.icon_new_for_string(params['gicon']); > + else if (params['icon']) > + icon = new Gio.ThemedIcon({ name: params['icon'] }); > > Why support both? Because creating a temporary GThemedIcon just to call g_icon_to_string() is a hassle, and I had missed that GThemedIcon's to_string() implementation explicitly guarantees that it will return the plain icon name in case of a single icon name. Changed to take only an 'icon' parameter, which is expected to hold a serialized GIcon (e.g. what 'gicon' did in the previous version).