GNOME Bugzilla – Bug 695801
Keep overview open when ctrl pressed breaks search
Last modified: 2013-03-14 16:06:36 UTC
When I want to quickly open a new instance of an already open application I do super + type + ctrl + enter .... This no longer works in 3.8 as the overview stays open which is very annoying.
Created attachment 238818 [details] [review] overview: Don't keep the overview open when launching a new instance from search
Created attachment 238819 [details] [review] overview: Don't always keep the overview open when ctrl is held When a user launches a new instance from search using ctrl+enter the expected behaviour is to quickly switch to the new window, so don't keep the overview open in that case.
Can we get a fix for this is in before .92 ? This is really annoying.
Review of attachment 238819 [details] [review]: Now that I've thought about it, I think this is the correct approach. ::: js/ui/appDisplay.js @@ +480,3 @@ + + if (quickExit && openNewWindow) + Main.overview.hide(openNewWindow); Why not just always pass true here?
I'm not so sure about this - "keep overview open while pressing ctrl" is easier to grasp than "keep overview open while pressing ctrl except when activating application launchers by keyboard". Maybe a better solution would be to use different modifiers? Given that ctrl-click/return has been present for a long time, my suggestion would be to change the keep-open modifier to super ...
(In reply to comment #5) > I'm not so sure about this - "keep overview open while pressing ctrl" is easier > to grasp than "keep overview open while pressing ctrl except when activating > application launchers by keyboard". Maybe a better solution would be to use > different modifiers? Given that ctrl-click/return has been present for a long > time, my suggestion would be to change the keep-open modifier to super ... Well, we currently close the overview with a key-press of super...
(In reply to comment #5) > I'm not so sure about this - "keep overview open while pressing ctrl" is easier > to grasp than "keep overview open while pressing ctrl except when activating > application launchers by keyboard". Maybe a better solution would be to use > different modifiers? Given that ctrl-click/return has been present for a long > time, my suggestion would be to change the keep-open modifier to super ... Well we choose ctrl for overview opening for a reason i.e to apply the "keep ctrl pressed to selected multiple elements" behaviour to the overview. I am not to worried about the keyboard vs. mouse distinction here ... I don't really think people expect the overview to stay open when activating stuff using the keyboard.
(In reply to comment #4) > Review of attachment 238819 [details] [review]: > > Now that I've thought about it, I think this is the correct approach. > > ::: js/ui/appDisplay.js > @@ +480,3 @@ > + > + if (quickExit && openNewWindow) > + Main.overview.hide(openNewWindow); > > Why not just always pass true here? To allow the user to select multiple search results using the mouse. Or do you mean instead of passing openNewWindow? ... Yes we could do that.
openNewWindow is always true there, so it doesn't make sense to pass it. (In reply to comment #7) > I am not to worried about the keyboard vs. mouse distinction here ... I don't > really think people expect the overview to stay open when activating stuff > using the keyboard. Right. I'm not sure what the correct behavior is if you're activating search results other than apps.
Review of attachment 238819 [details] [review]: (In reply to comment #7) > I am not to worried about the keyboard vs. mouse distinction here ... I don't > really think people expect the overview to stay open when activating stuff > using the keyboard. Mmh, you are probably right. However, this is not what the patch does (ignoring the bug pointed out below) - it only handles one specific case (application search results). I'd be much happier if you changed this to only apply the keep-open behavior to mouse events then ... ::: js/ui/overview.js @@ +589,3 @@ + if (!ignoreControlPressed) + this._controlPressed = false; "If control key should *not* be ignored, reset it"? That breaks the ctrl key for everything :-)
Created attachment 238883 [details] [review] overview: Don't use a captured-event handler to detect the control key captured-event handlers are easily messed up by an earlier event handler capturing the event. Instead, use the current Clutter event and check for the state of that. https://bugzilla.gnome.org/show_bug.cgi?id=695161
Created attachment 238884 [details] [review] overview: Only keep ourselves in the overview on button events This prevents using Control to quickly launch a new instance of an app in search results or with keyboard navigation.
Review of attachment 238883 [details] [review]: Makes sense.
Review of attachment 238884 [details] [review]: ::: js/ui/overview.js @@ +577,3 @@ + if (type == Clutter.EventType.BUTTON_PRESS || + type == Clutter.EventType.BUTTON_RELEASE || + (event.get_state() & Clutter.ModifierType.CONTROL_MASK) != 0) This is broken. Should be: "(type == Clutter.EventType.BUTTON_PRESS || type == Clutter.EventType.BUTTON_RELEASE) && (event.get_state() & Clutter.ModifierType.CONTROL_MASK) != 0)"
Created attachment 238891 [details] [review] overview: Only keep ourselves in the overview on button events This prevents using Control to quickly launch a new instance of an app in search results or with keyboard navigation.
Review of attachment 238891 [details] [review]: OK.
Attachment 238883 [details] pushed as b39e762 - overview: Don't use a captured-event handler to detect the control key Attachment 238891 [details] pushed as a8c87f3 - overview: Only keep ourselves in the overview on button events