From ee43690e09b3a85b4351910c0debf0b3552c8980 Mon Sep 17 00:00:00 2001 From: Siddharth Kannan Date: Fri, 4 Apr 2025 10:47:45 +0900 Subject: [PATCH 1/5] Attempt to clean up state more thoroughly when grab fails --- navigator.js | 46 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/navigator.js b/navigator.js index 794f382e3..e59d471d6 100644 --- a/navigator.js +++ b/navigator.js @@ -79,6 +79,7 @@ class ActionDispatcher { this.actor = Tiling.spaces.spaceContainer; this.actor.set_flags(Clutter.ActorFlags.REACTIVE); this.navigator = getNavigator(); + this.success = true; if (grab) { Utils.debug("#dispatch", "already in grab"); @@ -89,8 +90,21 @@ class ActionDispatcher { grab = Main.pushModal(this.actor); // We expect at least a keyboard grab here if ((grab.get_seat_state() & Clutter.GrabState.KEYBOARD) === 0) { - console.error("Failed to grab modal"); - throw new Error('Could not grab modal'); + console.error("[Fix-Attempt-2] Failed to grab modal - failing gracefully"); + + // Release the current grab which does not match what we want. + // Set `grab` to `null`. So, we will attempt to grab the keyboard again next time. + this.success = false; + try { + if (grab) { + Main.popModal(grab); + grab = null; + } + } catch (e) { + Utils.debug("[Fix-Attempt-2] Failed to release grab: ", e); + } + + return; } this.signals.connect(this.actor, 'key-press-event', this._keyPressEvent.bind(this)); @@ -101,6 +115,9 @@ class ActionDispatcher { } show(backward, binding, mask) { + // If grab was not successful, don't try to do this. + if (!this.success) return; + this._modifierMask = getModLock(mask); this.navigator = getNavigator(); TopBar.fixTopBar(); @@ -441,16 +458,21 @@ function finishNavigation(force = false) { * @returns {ActionDispatcher} */ function getActionDispatcher(mode) { - if (dispatcher) { - dispatcher.mode |= mode; - return dispatcher; + if (dispatcher === null) { + dispatcher = new ActionDispatcher(); + + if (!dispatcher.success) { + console.error("[Fix-Attempt-2] Action dispatcher creation was not successful"); + return dispatcher; + } } - dispatcher = new ActionDispatcher(); - return getActionDispatcher(mode); + + dispatcher.mode |= mode; + return dispatcher; } /** - * Fishes current dispatcher (if any). + * Finishes current dispatcher (if any). */ function finishDispatching() { dispatcher?._finish(global.get_current_time()); @@ -473,5 +495,11 @@ function dismissDispatcher(mode) { function preview_navigate(meta_window, space, { display, screen, binding }) { let tabPopup = getActionDispatcher(Clutter.GrabState.KEYBOARD); - tabPopup.show(binding.is_reversed(), binding.get_name(), binding.get_mask()); + // Getting the action dispatcher does not always succeed. In the cases where it does not + // succeed, attempt to fail gracefully by destroying what we created and returning silently. + if (!tabPopup.success) { + tabPopup.destroy(); + } else { + tabPopup.show(binding.is_reversed(), binding.get_name(), binding.get_mask()); + } } From f5c20dbad4a8963e4d63e9c31c2a8bde71e22667 Mon Sep 17 00:00:00 2001 From: Siddharth Kannan Date: Mon, 7 Apr 2025 10:58:27 +0900 Subject: [PATCH 2/5] Use editorconfig --- .editorconfig | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .editorconfig diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 000000000..9541a249b --- /dev/null +++ b/.editorconfig @@ -0,0 +1,3 @@ +[*.js] +indent_style = space +indent_size = 4 \ No newline at end of file From 3c87740fa66662f0ce46a145f58251b72f84e37c Mon Sep 17 00:00:00 2001 From: Siddharth Kannan Date: Mon, 7 Apr 2025 10:58:17 +0900 Subject: [PATCH 3/5] Fix indentation --- navigator.js | 58 ++++++++++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/navigator.js b/navigator.js index e59d471d6..2b7d61dcd 100644 --- a/navigator.js +++ b/navigator.js @@ -79,7 +79,7 @@ class ActionDispatcher { this.actor = Tiling.spaces.spaceContainer; this.actor.set_flags(Clutter.ActorFlags.REACTIVE); this.navigator = getNavigator(); - this.success = true; + this.success = true; if (grab) { Utils.debug("#dispatch", "already in grab"); @@ -92,19 +92,19 @@ class ActionDispatcher { if ((grab.get_seat_state() & Clutter.GrabState.KEYBOARD) === 0) { console.error("[Fix-Attempt-2] Failed to grab modal - failing gracefully"); - // Release the current grab which does not match what we want. - // Set `grab` to `null`. So, we will attempt to grab the keyboard again next time. - this.success = false; - try { - if (grab) { - Main.popModal(grab); - grab = null; - } - } catch (e) { - Utils.debug("[Fix-Attempt-2] Failed to release grab: ", e); - } - - return; + // Release the current grab which does not match what we want. + // Set `grab` to `null`. So, we will attempt to grab the keyboard again next time. + this.success = false; + try { + if (grab) { + Main.popModal(grab); + grab = null; + } + } catch (e) { + Utils.debug("[Fix-Attempt-2] Failed to release grab: ", e); + } + + return; } this.signals.connect(this.actor, 'key-press-event', this._keyPressEvent.bind(this)); @@ -115,8 +115,8 @@ class ActionDispatcher { } show(backward, binding, mask) { - // If grab was not successful, don't try to do this. - if (!this.success) return; + // If grab was not successful, don't try to do this. + if (!this.success) return; this._modifierMask = getModLock(mask); this.navigator = getNavigator(); @@ -459,16 +459,16 @@ function finishNavigation(force = false) { */ function getActionDispatcher(mode) { if (dispatcher === null) { - dispatcher = new ActionDispatcher(); + dispatcher = new ActionDispatcher(); - if (!dispatcher.success) { + if (!dispatcher.success) { console.error("[Fix-Attempt-2] Action dispatcher creation was not successful"); - return dispatcher; - } + return dispatcher; + } } - dispatcher.mode |= mode; - return dispatcher; + dispatcher.mode |= mode; + return dispatcher; } /** @@ -495,11 +495,11 @@ function dismissDispatcher(mode) { function preview_navigate(meta_window, space, { display, screen, binding }) { let tabPopup = getActionDispatcher(Clutter.GrabState.KEYBOARD); - // Getting the action dispatcher does not always succeed. In the cases where it does not - // succeed, attempt to fail gracefully by destroying what we created and returning silently. - if (!tabPopup.success) { - tabPopup.destroy(); - } else { - tabPopup.show(binding.is_reversed(), binding.get_name(), binding.get_mask()); - } + // Getting the action dispatcher does not always succeed. In the cases where it does not + // succeed, attempt to fail gracefully by destroying what we created and returning silently. + if (!tabPopup.success) { + tabPopup.destroy(); + } else { + tabPopup.show(binding.is_reversed(), binding.get_name(), binding.get_mask()); + } } From e17412fc29c8915398ca70d0b7aa12fa6116c074 Mon Sep 17 00:00:00 2001 From: Siddharth Kannan Date: Mon, 7 Apr 2025 11:10:14 +0900 Subject: [PATCH 4/5] Check for all falsy values I saw an error in the journal which shows that `dispatcher` was undefined. This would happen when PaperWM is started immediately after Gnome Shell starts: Apr 07 09:36:43 skannan--20250106--BQ374 gnome-shell[3651]: JS ERROR: TypeError: dispatcher is undefined getActionDispatcher@/home/siddharth/.local/share/gnome-shell/extensions/paperwm@paperwm.github.com/navigator.js:470:2 begin@/home/siddharth/.local/share/gnome-shell/extensions/paperwm@paperwm.github.com/grab.js:56:27 grabBegin@/home/siddharth/.local/share/gnome-shell/extensions/paperwm@paperwm.github.com/tiling.js:3639:20 init/<@/home/siddharth/.local/share/gnome-shell/extensions/paperwm@paperwm.github.com/tiling.js:1759:88 This was caused because I changed the check from `if (dispatcher)` which checks whether `dispatcher` is truthy, to `if (dispatcher === null)`, which only checks if it is `null`. `undefined` would have failed the previous check while the second check did succeed and ended up calling `dispatcher.mode |= mode` which resulted in an error. I had to restart PaperWM in order to recover from this error. This change restores the code back to checking for truthy values. Also, all the log lines now include "grab modal" so that I can get a summarized view of how this error is changing over time with the various attempted fixes. --- navigator.js | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/navigator.js b/navigator.js index 2b7d61dcd..c3c5d8c27 100644 --- a/navigator.js +++ b/navigator.js @@ -90,7 +90,7 @@ class ActionDispatcher { grab = Main.pushModal(this.actor); // We expect at least a keyboard grab here if ((grab.get_seat_state() & Clutter.GrabState.KEYBOARD) === 0) { - console.error("[Fix-Attempt-2] Failed to grab modal - failing gracefully"); + console.error("[Fix-Attempt-2: grab modal] Failed to grab modal - failing gracefully"); // Release the current grab which does not match what we want. // Set `grab` to `null`. So, we will attempt to grab the keyboard again next time. @@ -101,7 +101,7 @@ class ActionDispatcher { grab = null; } } catch (e) { - Utils.debug("[Fix-Attempt-2] Failed to release grab: ", e); + Utils.debug("[Fix-Attempt-2: grab modal] Failed to release grab: ", e); } return; @@ -458,16 +458,19 @@ function finishNavigation(force = false) { * @returns {ActionDispatcher} */ function getActionDispatcher(mode) { - if (dispatcher === null) { - dispatcher = new ActionDispatcher(); + // Falsy values include null, undefined, and a few other values. Everything else in Javascript + // is truthy. + // https://developer.mozilla.org/en-US/docs/Glossary/Falsy + if (dispatcher) { + dispatcher.mode |= mode; + return dispatcher; + } - if (!dispatcher.success) { - console.error("[Fix-Attempt-2] Action dispatcher creation was not successful"); - return dispatcher; - } + dispatcher = new ActionDispatcher(); + if (!dispatcher.success) { + console.error("[Fix-Attempt-2: grab modal] Action dispatcher creation was not successful"); } - dispatcher.mode |= mode; return dispatcher; } From f1f9f32d23c9e3c682aa9001f88c5546c49e6f38 Mon Sep 17 00:00:00 2001 From: Siddharth Kannan Date: Mon, 7 Apr 2025 11:17:58 +0900 Subject: [PATCH 5/5] Replace recursive call with inline mode change --- navigator.js | 1 + 1 file changed, 1 insertion(+) diff --git a/navigator.js b/navigator.js index c3c5d8c27..cdc527fa5 100644 --- a/navigator.js +++ b/navigator.js @@ -471,6 +471,7 @@ function getActionDispatcher(mode) { console.error("[Fix-Attempt-2: grab modal] Action dispatcher creation was not successful"); } + dispatcher.mode |= mode; return dispatcher; }