GNOME Bugzilla – Bug 740237
SelectArea unnecessarily opens the overview
Last modified: 2014-12-19 12:01:14 UTC
Here's a weird bug. 1. Invoke the "area selection", e.g. by using the "screenshot area" shortcut key, or by directly calling SelectArea: gdbus call -e -d org.gnome.Shell \ -o /org/gnome/Shell/Screenshot \ -m org.gnome.Shell.Screenshot.SelectArea 2. Start selecting at the left edge of the screen (x <= 20, roughly), and drag towards bottom-right. (Doesn't happen when dragging towards top-right.) 3. Notice that the overview will open by itself before the screenshot is made... as if I'd accidentally touched the hot corner, or something. (Therefore causing the screenshot to just contain part of the overview.) gnome-shell 3.15.1-29-g487b5cd
Created attachment 290833 [details] demo
Reproduced here on 3.14.x and master, the problem is the overview edge-drag action triggering erroneously. Carlos offered to take a look ...
Created attachment 291677 [details] [review] edgeDragAction: Restrict action based on keybindingMode Just like keybindings and the message tray pointer barrier, gestures don't always make sense - for instance, swiping up the screen shield should not trigger the message tray just as the SelectArea action around the left edge should not open the overview. To avoid this, restrict gestures based on the current keybinding mode.
*** Bug 739743 has been marked as a duplicate of this bug. ***
Review of attachment 291677 [details] [review]: I find the solution quite elegant, certainly gestures share a lot with keybindings in this regard, too bad even that there's no common gesture JS subclass so this applies to all :(. This is probably a symptom that ShellKeyBindingMode has outperformed its expectatives though, maybe should be better renamed to ShellActionMode in the future? This patch should go in nonetheless IMO.
Created attachment 292537 [details] [review] Rename KeyBindingMode to ActionMode The keybinding mode is no longer used exclusively for actions triggered by keybindings, so reflect this by a more generic name. (In reply to comment #5) > This is probably a symptom that ShellKeyBindingMode has outperformed its > expectatives though, maybe should be better renamed to ShellActionMode in the > future? Yeah, probably - the type is shared (via a copy-pasted header) with g-s-d though, so I'd rather not rename this on 3-14 ...
Created attachment 292541 [details] [review] gestures: Restrict actions based on keybindingMode (In reply to comment #5) > too bad even that there's no common gesture JS subclass so this applies to all :(. It would certainly be possible to change that, though I'm not sure it's really worth it - the check is in gesture_prepare() and no subclass chains up to the parent there, so even with a common subclass it wouldn't automagically work. We should still restrict other gestures though, so here's an (untested) update which does this ...
Created attachment 292543 [details] [review] Rename KeyBindingMode to ActionMode Updated patch on top of attachment 292541 [details] [review]
*** Bug 741474 has been marked as a duplicate of this bug. ***
Comment on attachment 292541 [details] [review] gestures: Restrict actions based on keybindingMode Looks good to me!
(In reply to comment #6) > Yeah, probably - the type is shared (via a copy-pasted header) with g-s-d > though, so I'd rather not rename this on 3-14 ... Yup, I agree. Let's just do this bit coordinately for 3.16
Comment on attachment 292543 [details] [review] Rename KeyBindingMode to ActionMode Very thorough, I think you caught all cases :). Refactors like these make me uneasy about possibly overseen ones though, git grep -i "keybindingmode" should be a good indicative here though, and comes out clean.
(In reply to comment #11) > Yup, I agree. Let's just do this bit coordinately for 3.16 It doesn't actually require strict coordination, as the DBus communication between g-s-d and g-s uses the numerical values, so the renaming doesn't break anything on its own. (In reply to comment #12) > (From update of attachment 292543 [details] [review]) > Refactors like these make me uneasy about possibly overseen ones though Right, which is why I'd rather not have that on the stable branch :-) I don't think I missed any case cases in my sed expression (or produced any false positives), but regular expressions ...
Attachment 292541 [details] pushed as ddeac23 - gestures: Restrict actions based on keybindingMode Attachment 292543 [details] pushed as e0eebc9 - Rename KeyBindingMode to ActionMode