Skip to content

Commit 274f05e

Browse files
committed
Fail gracefully instead of crashing if grab fails
I have been using PaperWM on a daily basis for the past few months and it is great. One of the problems that I kept running into repeatedly was #991. This issue was reported on Gnome 47. I ran into this problem on Gnome 44. I attempted a naive fix which simply tries to clean-up all the state instead of crashing. This works well; I have been using a version of this fix, applied on top of the gnome-44 branch, for the past month and I have not suffered any `Could not grab modal` crashes. It would be great if I could get some guidance about whether this fix looks logical, and whether there might be a way to reproduce this code path somehow. We need to somehow get into `preview_navigate` (which happens when one uses a keyboard shortcut to switch windows, I believe?) but get there in a state where the keyboard is already grabbed by something else, thus causing PaperWM to fail to grab the keyboard. I would really like to fix this issue within PaperWM because I have not run into any other bug at all and this is a great project! (cherry-picked from commit 793dd0d) Signed-off-by: Siddharth Kannan <[email protected]>
1 parent 3a736d7 commit 274f05e

File tree

1 file changed

+47
-6
lines changed

1 file changed

+47
-6
lines changed

navigator.js

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,21 @@ class ActionDispatcher {
8787

8888
// grab = stage.grab(this.actor)
8989
grab = Main.pushModal(this.actor);
90+
// Assume that grab succeeds and store that state in the object
91+
this.success = true;
9092
// We expect at least a keyboard grab here
9193
if ((grab.get_seat_state() & Clutter.GrabState.KEYBOARD) === 0) {
9294
console.error("Failed to grab modal");
93-
throw new Error('Could not grab modal');
95+
// Release current grab and let the user try again
96+
try {
97+
this.success = false;
98+
if (grab) {
99+
Main.popModal(grab);
100+
grab = null;
101+
}
102+
} catch (e) {
103+
console.error("Failed to release grab");
104+
}
94105
}
95106

96107
this.signals.connect(this.actor, 'key-press-event', this._keyPressEvent.bind(this));
@@ -100,8 +111,29 @@ class ActionDispatcher {
100111
this._doActionTimeout = null;
101112
}
102113

103-
show(backward, binding, mask) {
104-
this._modifierMask = getModLock(mask);
114+
/**
115+
* Adds a signal to this dispatcher. Will be destroyed when this
116+
* dispatcher is destroyed.
117+
*/
118+
addKeypressCallback(handler) {
119+
this.keyPressCallbacks.push(handler);
120+
return this;
121+
}
122+
123+
/**
124+
* Adds a signal to this dispatcher. Will be destroyed when this
125+
* dispatcher is destroyed.
126+
*/
127+
addKeyReleaseCallback(handler) {
128+
this.keyReleaseCallbacks.push(handler);
129+
return this;
130+
}
131+
132+
show(_backward, binding, mask) {
133+
// If required grab was not successful, then do not show anything
134+
if (!this.success) return;
135+
136+
this._modifierMask = primaryModifier(mask);
105137
this.navigator = getNavigator();
106138
TopBar.fixTopBar();
107139
let actionId = Keybindings.idOf(binding);
@@ -446,11 +478,11 @@ function getActionDispatcher(mode) {
446478
return dispatcher;
447479
}
448480
dispatcher = new ActionDispatcher();
449-
return getActionDispatcher(mode);
481+
return getActionDispatcher(mode);
450482
}
451483

452484
/**
453-
* Fishes current dispatcher (if any).
485+
* Finishes current dispatcher (if any).
454486
*/
455487
function finishDispatching() {
456488
dispatcher?._finish(global.get_current_time());
@@ -473,5 +505,14 @@ function dismissDispatcher(mode) {
473505

474506
function preview_navigate(meta_window, space, { display, screen, binding }) {
475507
let tabPopup = getActionDispatcher(Clutter.GrabState.KEYBOARD);
476-
tabPopup.show(binding.is_reversed(), binding.get_name(), binding.get_mask());
508+
509+
// Getting a dispatcher does not always succeed. Sometimes, it can fail because we are unable to
510+
// grab the keyboard.
511+
// In the case of a failure, fail gracefully by destroying the pop-up.
512+
if (!tabPopup.success) {
513+
tabPopup.destroy();
514+
return;
515+
}
516+
517+
tabPopup.show(binding.is_reversed(), binding.get_name(), binding.get_mask());
477518
}

0 commit comments

Comments
 (0)