Skip to content

Commit 19ee461

Browse files
anaximenoanaximeno
committed
Properly disconnect from local state signals to avoid errors during applet reload
gwl: Improve window state management Refresh all workspaces when the title display setting is changed This works well enough and guarantees all workspaces are in the same state both when enabling and disabling the title display options. gwl: Avoid adding the window again after removal This was triggering when using the show window in all workspaces option and was leading the app group representation to an irregular state where removed windows would still be counted and show irresponsible thumbnails. gwl: Only set appGroup focus if app actually has focus gwl: Clarify comment gwl: Improve state handling during window removal This fixes some annyoing issues during the removal of windows that would some time leave a ghost window instance in app groups. I've also improved the handled of some signals to ensure the window is only added again if it is inside a valid workspace and the workspace is similar to the current workspace receiving the signal. gwl: Simplify appGroup windowRemoved method It makes sense to call the callback to clean the appGroup if willUnmount is set true as well, however no need to only perform updates to the groupstate otherwise considering things like removing thumbnails have to be performed anyways, and considering the appGroup, in case of no open windows in the appGroup, or willUnmount is set, will be removed anyways, it likely shouldn't cause issues. gwl: Let workspace.windowAdded arbitrate over if metaWindow should be added to the workspace This already checks if window is in correct workspace and/or windows should be shown in all workspaces, plus checking the monitor using the shouldWindowBeAdded method. gwl: Adjust state check handling
1 parent 7e9c81f commit 19ee461

4 files changed

Lines changed: 79 additions & 66 deletions

File tree

‎files/usr/share/cinnamon/applets/grouped-window-list@cinnamon.org/appGroup.js‎

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,17 @@ const getFocusState = function(metaWindow) {
6161
return false;
6262
};
6363

64+
const getLastFocusedWindow = function(metaWindows) {
65+
let lastFocusedWindow = null;
66+
for (let i = 0; i < metaWindows.length; i++) {
67+
const metaWindow = metaWindows[i];
68+
if (!lastFocusedWindow || (metaWindow.get_user_time() > lastFocusedWindow.get_user_time())) {
69+
lastFocusedWindow = metaWindow;
70+
}
71+
}
72+
return lastFocusedWindow;
73+
};
74+
6475
var AppGroup = class AppGroup {
6576
constructor(params) {
6677
this.state = params.state;
@@ -86,7 +97,7 @@ var AppGroup = class AppGroup {
8697
pressed: true
8798
});
8899

89-
this.groupState.connect({
100+
this.groupStateConnectionId = this.groupState.connect({
90101
isFavoriteApp: () => this.handleFavorite(true),
91102
getActor: () => this.actor,
92103
launchNewInstance: (...args) => this.launchNewInstance(...args),
@@ -983,32 +994,31 @@ var AppGroup = class AppGroup {
983994
}
984995

985996
windowRemoved(metaWorkspace, metaWindow, refWindow, cb) {
986-
if (refWindow === -1) return;
997+
if (!metaWindow) return;
987998

988999
this.signals.disconnect('notify::title', metaWindow);
9891000
this.signals.disconnect('notify::appears-focused', metaWindow);
1001+
this.signals.disconnect('notify::icon', metaWindow);
1002+
this.signals.disconnect('notify::progress', metaWindow);
9901003

991-
this.groupState.metaWindows.splice(refWindow, 1);
1004+
const metaWindows = this.groupState.metaWindows.filter((w) => w != metaWindow);
9921005

9931006
if (this.progressOverlay.visible) this.onProgressChange();
9941007

995-
if (this.groupState.metaWindows.length > 0 && !this.groupState.willUnmount) {
996-
this.onWindowTitleChanged(this.groupState.lastFocused);
997-
this.groupState.set({
998-
metaWindows: this.groupState.metaWindows,
999-
lastFocused: this.groupState.metaWindows[this.groupState.metaWindows.length - 1]
1000-
}, true);
1001-
if (this.hoverMenu) this.groupState.trigger('removeThumbnailFromMenu', metaWindow);
1002-
this.calcWindowNumber();
1003-
} else {
1004-
// This is the last window, so this group needs to be destroyed. We'll call back windowRemoved
1005-
// in workspace to put the final nail in the coffin.
1006-
if (typeof cb === 'function') {
1007-
if (this.hoverMenu && this.groupState.isFavoriteApp) {
1008-
this.groupState.trigger('removeThumbnailFromMenu', metaWindow);
1009-
}
1010-
cb(this.groupState.appId, this.groupState.isFavoriteApp);
1011-
}
1008+
const lastFocused = this.groupState.lastFocused === metaWindow
1009+
? getLastFocusedWindow(metaWindows)
1010+
: this.groupState.lastFocused;
1011+
1012+
this.groupState.set({metaWindows: metaWindows, lastFocused: lastFocused}, true);
1013+
1014+
if (lastFocused) this.onWindowTitleChanged(lastFocused);
1015+
1016+
if (this.hoverMenu) this.groupState.trigger('removeThumbnailFromMenu', metaWindow);
1017+
1018+
this.calcWindowNumber();
1019+
1020+
if ((metaWindows.length === 0 || this.groupState.willUnmount) && typeof cb === 'function') {
1021+
cb(this.groupState.appId, this.groupState.isFavoriteApp);
10121022
}
10131023
}
10141024

@@ -1121,10 +1131,12 @@ var AppGroup = class AppGroup {
11211131
calcWindowNumber() {
11221132
if (this.groupState.willUnmount) return;
11231133

1124-
this.groupState.set({windowCount: this.groupState.metaWindows ? this.groupState.metaWindows.length : 0});
1125-
1126-
if (this.groupState.windowCount > 1 && this.state.settings.enableWindowCountBadges) {
1127-
this.windowsBadgeLabel.text = this.groupState.windowCount.toString();
1134+
const windowCount = this.groupState.metaWindows ? this.groupState.metaWindows.length : 0;
1135+
1136+
this.groupState.set({windowCount: windowCount});
1137+
1138+
if (windowCount > 1 && this.state.settings.enableWindowCountBadges) {
1139+
this.windowsBadgeLabel.text = windowCount.toString();
11281140
this.windowsBadge.show();
11291141
} else {
11301142
this.windowsBadge.hide();
@@ -1194,6 +1206,9 @@ var AppGroup = class AppGroup {
11941206
}
11951207

11961208
destroy(skipRefCleanup) {
1209+
if (this.groupStateConnectionId > 0) {
1210+
this.groupState.disconnect(this.groupStateConnectionId);
1211+
}
11971212
this.signals.disconnectAllSignals();
11981213
this.groupState.set({willUnmount: true});
11991214

‎files/usr/share/cinnamon/applets/grouped-window-list@cinnamon.org/applet.js‎

Lines changed: 14 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ class GroupedWindowListApplet extends Applet.Applet {
208208
// applet to avoid passing down the parent class down the constructor chain and creating circular references.
209209
// In addition to manual event emitting, store.js can emit updates on property changes when set through
210210
// store.set. Any keys emitted through store.trigger that are not declared here first will throw an error.
211-
this.state.connect({
211+
this.stateConnectionId = this.state.connect({
212212
setSettingsValue: (k, v) => this.settings.setValue(k, v),
213213
getPanel: () => (this.panel ? this.panel : null),
214214
getPanelHeight: () => this._panelHeight,
@@ -430,6 +430,9 @@ class GroupedWindowListApplet extends Applet.Applet {
430430
workspace.destroy();
431431
}
432432
});
433+
if (this.stateConnectionId) {
434+
this.state.disconnect(this.stateConnectionId);
435+
}
433436
this.settings.finalize();
434437
unref(this, RESERVE_KEYS);
435438
MessageTray.extensionsHandlingNotifications--;
@@ -447,13 +450,13 @@ class GroupedWindowListApplet extends Applet.Applet {
447450
return false;
448451
}
449452

450-
onWindowMonitorChanged(display, metaWindow, metaWorkspace) {
451-
if (this.state.monitorWatchList.length !== this.numberOfMonitors) {
452-
const currentWorkspace = this.getCurrentWorkspace();
453-
if (currentWorkspace !== null) {
454-
currentWorkspace.windowRemoved(metaWorkspace, metaWindow);
455-
currentWorkspace.windowAdded(metaWorkspace, metaWindow);
456-
}
453+
onWindowMonitorChanged(display, metaWindow, monitor) {
454+
if ((this.state.monitorWatchList.length !== this.numberOfMonitors)) {
455+
this.workspaces.forEach( workspace => {
456+
if (!workspace) return;
457+
workspace.windowRemoved(workspace.metaWorkspace, metaWindow);
458+
workspace.windowAdded(workspace.metaWorkspace, metaWindow);
459+
});
457460
}
458461
}
459462

@@ -622,21 +625,8 @@ class GroupedWindowListApplet extends Applet.Applet {
622625
}
623626

624627
updateTitleDisplay(titleDisplay) {
625-
if (titleDisplay === TitleDisplay.None
626-
|| this.state.lastTitleDisplay === TitleDisplay.None) {
627-
this.refreshCurrentWorkspace();
628-
}
629-
630-
this.workspaces.forEach( workspace => {
631-
workspace.appGroups.forEach( appGroup => {
632-
if (titleDisplay === TitleDisplay.Focused) {
633-
appGroup.hideLabel();
634-
}
635-
appGroup.handleTitleDisplayChange();
636-
});
637-
});
638-
639628
this.state.set({lastTitleDisplay: titleDisplay});
629+
this.refreshAllWorkspaces();
640630
}
641631

642632
getAppFromWindow(metaWindow) {
@@ -781,7 +771,7 @@ class GroupedWindowListApplet extends Applet.Applet {
781771
currentWorkspace.appGroups[z].groupState.lastFocused
782772
: source.groupState.metaWindows[z];
783773
Main.activateWindow(_window, global.get_current_time());
784-
setTimeout(() => this.state.set({scrollActive: false}, 4000));
774+
setTimeout(() => this.state.set({scrollActive: false}), 4000);
785775
}
786776
}
787777

@@ -1041,7 +1031,6 @@ class GroupedWindowListApplet extends Applet.Applet {
10411031

10421032
_onWindowAppChanged(tracker, metaWindow) {
10431033
if (!metaWindow) return;
1044-
10451034
this.workspaces.forEach(workspace => {
10461035
if (!workspace) return;
10471036
workspace.windowRemoved(workspace.metaWorkspace, metaWindow);
@@ -1056,7 +1045,7 @@ class GroupedWindowListApplet extends Applet.Applet {
10561045

10571046
_onNotificationReceived(mtray, notification) {
10581047
let appId = notification.source.app?.get_id();
1059-
1048+
10601049
if (!appId) {
10611050
return;
10621051
}

‎files/usr/share/cinnamon/applets/grouped-window-list@cinnamon.org/state.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -252,14 +252,14 @@ var createStore = function(state = {}, listeners = [], connections = 0) {
252252
}
253253
if (listener) {
254254
let newKeys = intersect(keys, listener.keys, true);
255-
listener.keys.concat(newKeys);
255+
listener.keys = listener.keys.concat(newKeys);
256256
} else {
257257
listeners.push({keys, callback, id});
258258
}
259259
}
260260

261261
function connect(actions, callback) {
262-
const id = connections++;
262+
const id = ++connections;
263263
if (Array.isArray(actions)) {
264264
_connect(actions, callback, id);
265265
} else if (typeof actions === 'string') {

‎files/usr/share/cinnamon/applets/grouped-window-list@cinnamon.org/workspace.js‎

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ const {RESERVE_KEYS} = Me.imports.constants;
1313
var Workspace = class Workspace {
1414
constructor(params) {
1515
this.state = params.state;
16-
this.state.connect({
16+
this.stateConnectionId = this.state.connect({
1717
orientation: (state) => {
1818
this.on_orientation_changed(state.orientation);
1919
},
@@ -277,14 +277,19 @@ var Workspace = class Workspace {
277277

278278
shouldWindowBeAdded(metaWindow) {
279279
return (this.state.settings.showAllWorkspaces
280-
|| metaWindow.is_on_all_workspaces()
281-
|| metaWindow.get_workspace() === this.metaWorkspace)
280+
|| metaWindow.is_on_all_workspaces()
281+
|| (metaWindow.get_workspace() === this.metaWorkspace))
282282
&& Main.isInteresting(metaWindow)
283283
&& this.state.monitorWatchList.indexOf(metaWindow.get_monitor()) > -1;
284284
}
285285

286-
windowWorkspaceChanged(display, metaWorkspace, metaWindow) {
287-
this.windowAdded(metaWindow, metaWorkspace);
286+
windowWorkspaceChanged(display, metaWindow, metaWorkspace) {
287+
// If the window is removed the metaWorkspace will be null.
288+
if (metaWorkspace) {
289+
this.windowAdded(metaWorkspace, metaWindow);
290+
} else {
291+
this.windowRemoved(metaWorkspace, metaWindow);
292+
}
288293
}
289294

290295
windowAdded(metaWorkspace, metaWindow, app, isFavoriteApp) {
@@ -395,15 +400,16 @@ var Workspace = class Workspace {
395400
windowRemoved(metaWorkspace, metaWindow) {
396401
if (!this.state) return;
397402

398-
if ((metaWindow.is_on_all_workspaces() || this.state.settings.showAllWorkspaces
399-
|| !this.state.settings.showAllWorkspaces)
403+
// Abort the remove if the window is just changing workspaces, window
404+
// should always remain indexed on all workspaces while its mapped.
405+
if (this.state.settings.showAllWorkspaces && !this.state.removingWindowFromWorkspaces &&
406+
metaWindow.has_focus() && (global.workspace_manager.get_active_workspace_index() !== metaWorkspace.index()))
407+
return;
408+
409+
// If the window is on all workspaces or we're showing windows in all workspaces,
410+
// make sure to remove the window from all workspaces.
411+
if ((metaWindow.is_on_all_workspaces() || this.state.settings.showAllWorkspaces)
400412
&& !this.state.removingWindowFromWorkspaces) {
401-
// Abort the remove if the window is just changing workspaces, window
402-
// should always remain indexed on all workspaces while its mapped.
403-
// if (!metaWindow.showing_on_its_workspace()) return;
404-
if ((this.state.settings.showAllWorkspaces) && (metaWindow.has_focus()
405-
&& global.workspace_manager.get_active_workspace_index()
406-
!== metaWorkspace.index())) return;
407413
this.state.removingWindowFromWorkspaces = true;
408414
this.state.trigger('removeWindowFromAllWorkspaces', metaWindow);
409415
return;
@@ -435,7 +441,7 @@ var Workspace = class Workspace {
435441

436442
const refAppId = this.appGroups[refApp].groupState.appId;
437443

438-
this.appGroups[refApp].destroy(true);
444+
this.appGroups[refApp].destroy();
439445
this.appGroups[refApp] = undefined;
440446
this.appGroups.splice(refApp, 1);
441447

@@ -455,7 +461,7 @@ var Workspace = class Workspace {
455461
}
456462
}
457463
} else {
458-
this.appGroups[refApp].destroy(true);
464+
this.appGroups[refApp].destroy();
459465
this.appGroups[refApp] = undefined;
460466
this.appGroups.splice(refApp, 1);
461467
}
@@ -468,6 +474,9 @@ var Workspace = class Workspace {
468474
GLib.source_remove(this.scrollToAppDebounceTimeoutId);
469475
this.scrollToAppDebounceTimeoutId = 0;
470476
}
477+
if (this.stateConnectionId) {
478+
this.state.disconnect(this.stateConnectionId);
479+
}
471480
this.signals.disconnectAllSignals();
472481
this.appGroups.forEach( appGroup => appGroup.destroy() );
473482
this.scrollBox.destroy();

0 commit comments

Comments
 (0)