WindowSwitcher: correctly handle multiple keybind modifiers#2807
WindowSwitcher: correctly handle multiple keybind modifiers#2807
Conversation
6753146 to
890c7b6
Compare
890c7b6 to
29a31fa
Compare
leolost2605
left a comment
There was a problem hiding this comment.
I think it would help to know what the original reason was for only using the primary modifier in a mask. I had a quick a look and gnome shell is still doing it? The only reason I could think of are the backwards bindings e.g. to make sure we close the switcher if alt or super is released even though shift is still held?
Also for some reason this breaks Super + ` (which is the default shortcut for cycling windows of an app)?
|
The primaryModifier code was introduced in Gnome in the following commit. The above discusses computing the primaryModifier instead of hard coding it to Alt for Ctrl+Alt+Tab and Alt+Tab. The next place to look at is the following commit implementing Ctrl+Alt+Tab. The initial commit already starts of with Alt being hard coded, copied from the Alt+Tab implementation. I'm pasting the comment about the hard coding being copied from AltTab, but please read the context around it. The comment is about a server round trip, not about the key combination. @@ +148,3 @@
+ _keyReleaseEvent : function(actor, event) {
+ let [x, y, mods] = global.get_pointer();
+ let state = mods & Clutter.ModifierType.MOD1_MASK;
Wow, that's an ugly hack round-tripping on every keyrelease event. But I guess it's copied straight-over from altTab.js so likely is as-good-as-possible. Though it doesn't look like the usage of global.get_pointer() was even considered for [bug 629368](https://bugzilla.gnome.org/show_bug.cgi?id=629368) ... yeah, the Mutter code to "get it right" is complex, but it would be better to export it out than just doing something cheesy, slow, and racy. Not a blocker for this patch, but I'd appreciate it if we had a bug filed. (OK, maybe this is better-enemy-of-good-enough, but if we already *have* the better code.)For completeness, code mentioning I think it's one of these two that we're dealing with, but hard to know which. You might have other ideas as well.
|
Fixes #2794