GNOME Bugzilla – Bug 688589
overview: Fix stuck grabs when mashing the overlay-key
Last modified: 2012-12-10 19:39:20 UTC
See patch. Found this while trying to reproduce a crash for ricotz and the new theme node stuff. I've seen a few commits about stuck grabs -- this may fix one of them.
Created attachment 229292 [details] [review] overview: Fix stuck grabs when mashing the overlay-key When pressing the overlay key three times, things went like this: * show(), push a modal * hide(), will pop a modal after hiding is done * show(), push a modal Thus, when the showing is done, and then it activated the hiding, it popped one modal, but not the other. This patch changes things to be: * show(), push a modal * hide(), will pop a modal after hiding is done * hide(), no-op That is, mashing the overlay-key when it's showing will always make it hide, not mashing an odd number of times.
Review of attachment 229292 [details] [review]: I don't really like this. I think the problem is show() pushing modals without checking if it's already modal, i.e. something like: - // Do this manually instead of using _syncInputMode, to handle failure - if (!Main.pushModal(this._group, { keybindingMode: Main.KeybindingMode.OVERVIEW })) + + this._shown = true; + this._syncInputMode(); + if (!this._modal) return; - this._modal = true; + this._animateVisible(); - this._shown = true;
Yeah, my original patch looked like that, but during testing it was weird to have the "press three times, still in overview". And at that point, the original bug was "fixed", so I removed that part of the patch.
Sorry, but I don't understand why pressing three times slowly should make any difference from pressing three times fast. The sequence is show-hide-show, it is clear from any point of view. And it complicates the code for nothing.
(In reply to comment #4) > Sorry, but I don't understand why pressing three times slowly should make any > difference from pressing three times fast. The sequence is show-hide-show, it > is clear from any point of view. > And it complicates the code for nothing. I think that if we press it while animating, it's clear that a user wants to exit no matter how many times they press it.
Review of attachment 229292 [details] [review]: (In reply to comment #5) > (In reply to comment #4) > > Sorry, but I don't understand why pressing three times slowly should make any > > difference from pressing three times fast. The sequence is show-hide-show, it > > is clear from any point of view. > > And it complicates the code for nothing. > > I think that if we press it while animating, it's clear that a user wants to > exit no matter how many times they press it. Fair enough, go ahead.
Created attachment 231121 [details] [review] overview: Clean up overview code The comment here is a liar. We certainly can handle failure using _syncInputMode().
Review of attachment 231121 [details] [review]: Yeah, this seems better to me.
Attachment 229292 [details] pushed as 14fb51e - overview: Fix stuck grabs when mashing the overlay-key Attachment 231121 [details] pushed as e48dbe6 - overview: Clean up overview code