Skip to content

Commit 1c83412

Browse files
fix(prefs): disconnect page settings listeners on destroy
The General and Keyboard Shortcuts pages never disconnected their settings listeners. The preferences process outlives its window, so a later settings change ran the handlers on disposed widgets. The Maximizing page's disconnect-on-destroy pattern moves to pageListener() in prefs/rows.js and is used by all three pages.
1 parent 55273b6 commit 1c83412

5 files changed

Lines changed: 36 additions & 8 deletions

File tree

‎prefs/general.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,12 @@ import Gio from 'gi://Gio';
33
import GLib from 'gi://GLib';
44
import Gtk from 'gi://Gtk';
55
import { Logger } from '../lib/utils/logger.js';
6+
import { pageListener } from './rows.js';
67

78
/** Preferences page for gaps and debugging. */
89
export function buildGeneralPage(settings) {
910
const page = new Adw.PreferencesPage({ title: 'General', icon_name: 'preferences-system-symbolic' });
11+
const listen = pageListener(page, settings);
1012

1113
// --- Gaps Group ---
1214
const gapsGroup = new Adw.PreferencesGroup({ title: 'Gaps' });
@@ -33,7 +35,7 @@ export function buildGeneralPage(settings) {
3335
innerGapsRow.visible = enabled;
3436
outerGapsRow.visible = enabled;
3537
};
36-
settings.connect('changed::enable-gaps', updateGapsVisibility);
38+
listen('changed::enable-gaps', updateGapsVisibility);
3739
updateGapsVisibility();
3840

3941
page.add(gapsGroup);

‎prefs/maximize.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import Adw from 'gi://Adw';
22
import Gio from 'gi://Gio';
33
import Gtk from 'gi://Gtk';
44
import { appEntry, withEntry } from '../lib/domain/policies/override/self-maximize.js';
5-
import { choiceRow } from './rows.js';
5+
import { choiceRow, pageListener } from './rows.js';
66

77
const SELF_MAXIMIZE_MODES = [
88
{ value: 'never', label: 'Never', subtitle: 'Tiled windows maximize only through the shortcuts' },
@@ -107,9 +107,7 @@ function pickApp(parent, apps, onPicked) {
107107
/** Preferences page for maximize and fullscreen behaviour of tiled windows. */
108108
export function buildMaximizePage(settings) {
109109
const page = new Adw.PreferencesPage({ title: 'Maximizing', icon_name: 'view-fullscreen-symbolic' });
110-
const signalIds = [];
111-
const listen = (signal, callback) => signalIds.push(settings.connect(signal, callback));
112-
page.connect('destroy', () => signalIds.splice(0).forEach((id) => settings.disconnect(id)));
110+
const listen = pageListener(page, settings);
113111
let cachedApps = null;
114112
const apps = () => (cachedApps ??= installedApps());
115113

‎prefs/rows.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,17 @@
11
import Adw from 'gi://Adw';
22
import Gtk from 'gi://Gtk';
33

4+
/**
5+
* Settings listener tied to a page: everything connected through it is disconnected when the page is destroyed, so
6+
* handlers never run on the widgets of a closed preferences window.
7+
* @returns {(signal: string, callback: Function) => void}
8+
*/
9+
export function pageListener(page, settings) {
10+
const signalIds = [];
11+
page.connect('destroy', () => signalIds.splice(0).forEach((id) => settings.disconnect(id)));
12+
return (signal, callback) => signalIds.push(settings.connect(signal, callback));
13+
}
14+
415
/** Combo row storing one of `choices` in a string key; the subtitle explains the current choice. */
516
export function choiceRow(settings, key, title, choices) {
617
const row = new Adw.ComboRow({ title, model: Gtk.StringList.new(choices.map((c) => c.label)) });

‎prefs/shortcuts.js‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import Gio from 'gi://Gio';
44
import GObject from 'gi://GObject';
55
import Gtk from 'gi://Gtk';
66
import { Logger } from '../lib/utils/logger.js';
7+
import { pageListener } from './rows.js';
78

89
const ShortcutRowMixin = {
910
_recordShortcut() {
@@ -145,6 +146,7 @@ export function buildShortcutsPage(settings) {
145146
title: 'Keyboard Shortcuts',
146147
icon_name: 'input-keyboard-symbolic',
147148
});
149+
const listen = pageListener(shortcutsPage, settings);
148150

149151
const wmSettings = new Gio.Settings({ schema_id: 'org.gnome.desktop.wm.keybindings' });
150152
const allRows = [];
@@ -237,7 +239,7 @@ export function buildShortcutsPage(settings) {
237239
const showCustom = mode === 'custom';
238240
focusRows.forEach((r) => (r.visible = showCustom));
239241
};
240-
settings.connect('changed::focus-window-mode', () => {
242+
listen('changed::focus-window-mode', () => {
241243
updateFocusVisibility();
242244
updateConflicts();
243245
});
@@ -275,7 +277,7 @@ export function buildShortcutsPage(settings) {
275277
const showCustom = mode === 'custom';
276278
moveRows.forEach((r) => (r.visible = showCustom));
277279
};
278-
settings.connect('changed::keybindings-mode', () => {
280+
listen('changed::keybindings-mode', () => {
279281
updateVisibility();
280282
updateConflicts();
281283
});
@@ -314,7 +316,7 @@ export function buildShortcutsPage(settings) {
314316
const strv = settings.get_strv('shortcut-close-monitor');
315317
closeMinRow.visible = strv.length > 0 && strv[0] !== '';
316318
};
317-
settings.connect('changed::shortcut-close-monitor', updateCloseMinRowVisibility);
319+
listen('changed::shortcut-close-monitor', updateCloseMinRowVisibility);
318320
updateCloseMinRowVisibility();
319321
monitorGroup.add(closeMinRow);
320322

‎tests/prefs.test.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,21 @@ describe('WorkflowTilingPreferences', () => {
419419
});
420420
});
421421

422+
it('disconnects the General and Keyboard Shortcuts listeners when their pages are destroyed', () => {
423+
prefs.fillPreferencesWindow(mockWindow);
424+
const listeners = (key) => (mockListeners[`changed::${key}`] ?? []).length;
425+
// a ShortcutRow keeps its own listener on its key; it is released in vfunc_dispose
426+
const closeMonitorListeners = listeners('shortcut-close-monitor');
427+
428+
addedPages.find(p => p.title === 'General').emit('destroy');
429+
addedPages.find(p => p.title === 'Keyboard Shortcuts').emit('destroy');
430+
431+
expect(listeners('enable-gaps')).toBe(0);
432+
expect(listeners('focus-window-mode')).toBe(0);
433+
expect(listeners('keybindings-mode')).toBe(0);
434+
expect(listeners('shortcut-close-monitor')).toBe(closeMonitorListeners - 1);
435+
});
436+
422437
it('should store the monitor change policy chosen on the Monitors page', () => {
423438
mockSettingsStore['hotplug-policy'] = 'follow-primary';
424439
prefs.fillPreferencesWindow(mockWindow);

0 commit comments

Comments
 (0)