From aeee75defc9a5165ecbee6e0d92d0bd38fdeb53d Mon Sep 17 00:00:00 2001 From: Italo Date: Tue, 20 Oct 2026 11:07:34 -0300 Subject: [PATCH 1/2] fix: keep floating window decorations in sync with compositor --- package.json | 3 +- src/engine/stack.ts | 25 ++- src/extension.ts | 15 +- src/window/window.ts | 23 +- tests/floating-decorations.test.mjs | 333 ++++++++++++++++++++++++++++ tests/shell-methods.mjs | 39 ++++ 6 files changed, 424 insertions(+), 14 deletions(-) create mode 100644 tests/floating-decorations.test.mjs create mode 100644 tests/shell-methods.mjs diff --git a/package.json b/package.json index 9337550..45954e9 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,8 @@ "lint:dist": "../GER/ego-lint ./dist", "shexli": "make shexli", "log": "journalctl -f -o cat /usr/bin/gnome-shell | grep o-tiling", - "local-install": "make install" + "local-install": "make install", + "test:decorations": "node tests/floating-decorations.test.mjs" }, "devDependencies": { "@eslint/js": "^10.0.1", diff --git a/src/engine/stack.ts b/src/engine/stack.ts index 47cd91d..fa7fb86 100644 --- a/src/engine/stack.ts +++ b/src/engine/stack.ts @@ -6,6 +6,7 @@ import * as Ecs from '../core/ecs.js'; import * as a from '../core/arena.js'; import * as utils from '../utils/utils.js'; import * as log from '../utils/log.js'; +import * as lib from '../utils/lib.js'; import { get_primary_monitor_index } from './fork.js'; const Arena = a.Arena; @@ -14,6 +15,7 @@ import GObject from 'gi://GObject'; import St from 'gi://St'; import Meta from 'gi://Meta'; import Pango from 'gi://Pango'; +import GLib from 'gi://GLib'; const ACTIVE_TAB = 'o-tiling-tab o-tiling-tab-active'; const INACTIVE_TAB = 'o-tiling-tab o-tiling-tab-inactive'; @@ -166,6 +168,7 @@ export class Stack { private rect: Rectangular = { width: 0, height: 0, x: 0, y: 0 }; private restacker: SignalID = (global as any).display.connect('restacked', () => this.restack()); + private _restack_id: number | null = null; private tabs_destroy: SignalID; @@ -423,6 +426,10 @@ export class Stack { /** Disconnects this stack's signal, and destroys its widgets */ destroy() { + if (this._restack_id !== null) { + lib.later_remove(this._restack_id); + this._restack_id = null; + } (global as any).display.disconnect(this.restacker); this.active_disconnect(); @@ -596,12 +603,11 @@ export class Stack { } const stack_parent = this.widgets.tabs.get_parent(); - if (stack_parent) { - stack_parent.remove_child(this.widgets.tabs); + if (stack_parent !== parent) { + stack_parent?.remove_child(this.widgets.tabs); + parent.add_child(this.widgets.tabs); } - parent.add_child(this.widgets.tabs); - // Reposition actors on the screen, being careful about not displaying over maximized windows if (!window.meta.is_fullscreen() && !window.is_maximized() && !this.ext.maximized_on_active_display()) { parent.set_child_above_sibling(this.widgets.tabs, actor); @@ -637,6 +643,17 @@ export class Stack { /** Repositions the stack, and hides all but the active window in the stack */ restack() { + // Mutter's restacked signal can precede the actor-order update. Reorder + // our tabs after compositor stack synchronization, not against stale siblings. + if (this._restack_id !== null) return; + this._restack_id = lib.later_add(Meta.LaterType.BEFORE_REDRAW, () => { + this._restack_id = null; + if (!this.is_disposed()) this.restack_now(); + return GLib.SOURCE_REMOVE; + }); + } + + private restack_now() { this.on_grab(() => { if (!this.widgets) return; diff --git a/src/extension.ts b/src/extension.ts index 01725e2..2c3afd6 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -1639,6 +1639,14 @@ export class Ext extends Ecs.System { /** Triggered when a grab operation has been ended */ on_grab_end(meta: Meta.Window, op?: any, drop_cursor?: Rect.Rectangle) { + // Floating windows do not create a tiling grab_op, but still need cleanup. + const win = this.get_window(meta); + if (win?.grab) { + win.grab = false; + // The compositor has already applied a floating drag's final frame. + win.mark_border_settling(false); + } + // Guard: Skip if grab_op is null on a non-overview drag to prevent handling a double-fired drop. if (this.grab_op === null && op !== undefined) { return; @@ -1649,12 +1657,6 @@ export class Ext extends Ecs.System { drop_cursor = cursor_rect(); } - const win = this.get_window(meta); - - if (win !== null) { - win.grab = false; - } - if (null === win || (!win.is_tilable(this) && !win.is_eligible_for_tiling(this))) { this.unset_grab_op(); return; @@ -2044,6 +2046,7 @@ export class Ext extends Ecs.System { const win = this.get_window(meta); if (win) { win.grab = true; + win.hide_border(); if (win.is_tilable(this)) { const entity = win.entity; diff --git a/src/window/window.ts b/src/window/window.ts index 70bf587..54860de 100644 --- a/src/window/window.ts +++ b/src/window/window.ts @@ -534,6 +534,10 @@ export class ShellWindow { show_border() { if (!this.border) return; + if (this.grab) { + this.hide_border(); + return; + } // Bail while the settle timer is active; mark_border_settling() will re-show once geometry is final. if (this._border_settle_id !== null) return; @@ -550,6 +554,9 @@ export class ShellWindow { const overlay_all_active = this.ext.settings.active_hint_overlay_enabled() && this.ext.settings.active_hint_overlay_all_windows(); const permitted = () => + !this.grab && + this._border_settle_id === null && + !this._border_pending_initial_tile && this.actor_exists() && this.ext.settings.active_hint() && (overlay_all_active || (this.ext.focus_window() == this && this.meta.appears_focused)) && @@ -585,7 +592,7 @@ export class ShellWindow { } /** Hides the border and defers show_border() until Mutter commits the post-tile frame rect. */ - mark_border_settling() { + mark_border_settling(wait_for_geometry_change: boolean = true) { // The initial tile pass is underway — the settle re-shows the hint at the final geometry. this._border_pending_initial_tile = false; @@ -593,7 +600,9 @@ export class ShellWindow { // Capture the pre-tile rect so the settle can tell when the window has actually moved. const rect = this.meta.get_frame_rect(); - this._border_settle_pre_rect = { x: rect.x, y: rect.y, width: rect.width, height: rect.height }; + this._border_settle_pre_rect = wait_for_geometry_change + ? { x: rect.x, y: rect.y, width: rect.width, height: rect.height } + : null; this.arm_border_settle(); } @@ -652,6 +661,10 @@ export class ShellWindow { /** Sorts the window/always-top group with each window border based on the update state (NORMAL, RAISED, WORKSPACE_CHANGED). */ restack(_updateState: RESTACK_STATE = RESTACK_STATE.NORMAL, immediate: boolean = false) { + if (this.grab) { + this.hide_border(); + return; + } this.update_border_layout(); if ( this.meta.is_fullscreen() || @@ -669,6 +682,10 @@ export class ShellWindow { } if (!this.border) return GLib.SOURCE_REMOVE; + if (this.grab) { + this.hide_border(); + return GLib.SOURCE_REMOVE; + } if (!this.actor_exists()) return GLib.SOURCE_REMOVE; @@ -1037,4 +1054,4 @@ function pointer_already_on_window(meta: Meta.Window): boolean { const cursor = lib.cursor_rect(); return cursor.intersects(meta.get_frame_rect()); -} \ No newline at end of file +} diff --git a/tests/floating-decorations.test.mjs b/tests/floating-decorations.test.mjs new file mode 100644 index 0000000..9a1cf85 --- /dev/null +++ b/tests/floating-decorations.test.mjs @@ -0,0 +1,333 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { methods } from "./shell-methods.mjs"; + +const extPath = "src/extension.ts"; +const winPath = "src/window/window.ts"; +const stackPath = "src/engine/stack.ts"; +test("floating drag hides the old border immediately", () => { + const Subject = methods(extPath, "Ext", ["on_grab_start"]); + const ext = new Subject(); + let hidden = 0; + const win = { + grab: false, + hide_border: () => hidden++, + is_tilable: () => false, + }; + ext.get_window = () => win; + ext.on_grab_start({}, 1); + assert.equal(win.grab, true); + assert.equal(hidden, 1); +}); +test("floating drag end clears grab state and refreshes the border despite no tiling grab", () => { + const Subject = methods(extPath, "Ext", ["on_grab_end"]); + const ext = new Subject(); + let settled = 0; + const win = { grab: true, mark_border_settling: () => settled++ }; + ext.get_window = () => win; + ext.grab_op = null; + ext.on_grab_end({}, 1); + assert.equal(win.grab, false); + assert.equal(settled, 1); + ext.on_grab_end({}, 1); + assert.equal(settled, 1, "duplicate drag-end must not restart settling"); +}); +function windowFixture() { + const callbacks = []; + const Subject = methods(winPath, "ShellWindow", ["show_border"], { + GLib: { + PRIORITY_DEFAULT: 0, + SOURCE_REMOVE: false, + source_remove() {}, + timeout_add(p, ms, cb) { + callbacks.push(cb); + return callbacks.length; + }, + }, + ACTIVE_HINT_SHOW_ID: null, + }); + const win = new Subject(); + win.grab = false; + win._border_settle_id = null; + win._border_pending_initial_tile = false; + win.border = { + visible: false, + show() { + this.visible = true; + }, + hide() { + this.visible = false; + }, + }; + win.hide_border = () => win.border.hide(); + win.update_border_layout = () => {}; + win.update_border_style = () => {}; + win.restack = () => {}; + win.ext = { + settings: { + active_hint: () => true, + active_hint_overlay_enabled: () => false, + }, + focus_window: () => win, + }; + win.actor_exists = () => true; + win.is_maximized = () => false; + win.smart_gapped = false; + win.meta = { + appears_focused: true, + is_fullscreen: () => false, + minimized: false, + }; + return { win, callbacks }; +} +test("focus events cannot show a border during a drag", () => { + const { win } = windowFixture(); + win.grab = true; + win.border.visible = true; + win.show_border(); + assert.equal(win.border.visible, false); +}); +test("a delayed show callback cannot revive the border after dragging begins", () => { + const { win, callbacks } = windowFixture(); + win.show_border(); + assert.equal(win.border.visible, true); + win.grab = true; + win.border.hide(); + callbacks[0](); + assert.equal(win.border.visible, false); +}); +test("stack tabs follow final window order after the compositor restacks", () => { + const callbacks = []; + const Subject = methods(stackPath, "Stack", ["restack", "restack_now"], { + lib: { + later_add(type, cb) { + callbacks.push(cb); + return callbacks.length; + }, + later_remove() {}, + }, + Meta: { LaterType: { BEFORE_REDRAW: 1 } }, + GLib: { SOURCE_REMOVE: false }, + }); + const stack = new Subject(); + stack._restack_id = null; + let aboveFloating = true; + stack.widgets = { tabs: { visible: true } }; + stack.is_disposed = () => false; + stack.permitted_to_show = () => true; + stack.on_grab = (cb) => cb(); + stack.reset_visibility = () => {}; + stack.reposition = () => { + aboveFloating = false; + }; + stack.restack(); + assert.equal( + aboveFloating, + true, + "must defer until compositor order is final", + ); + stack.restack(); + assert.equal(callbacks.length, 1, "coalesce repeated restack notifications"); + callbacks[0](); + assert.equal(aboveFloating, false); + assert.equal(stack._restack_id, null); +}); +test("tabs stay immediately above their stack window and below floating windows", () => { + const Subject = methods(stackPath, "Stack", ["reposition"]); + const stack = new Subject(); + const children = []; + const parent = { + get_children: () => [...children], + remove_child(a) { + children.splice(children.indexOf(a), 1); + a.parent = null; + }, + add_child(a) { + children.push(a); + a.parent = this; + }, + set_child_above_sibling(a, b) { + this.remove_child(a); + children.splice(children.indexOf(b) + 1, 0, a); + a.parent = this; + }, + set_child_below_sibling(a, b) { + this.remove_child(a); + children.splice(children.indexOf(b), 0, a); + a.parent = this; + }, + }; + const actor = (name) => ({ + name, + parent, + get_parent() { + return this.parent; + }, + show() {}, + }); + const stackActor = actor("stack"); + const float = actor("floating"); + const dialog = actor("dialog"); + const tabs = actor("tabs"); + children.push(stackActor, float, dialog, tabs); + stack.widgets = { tabs }; + stack.is_disposed = () => false; + stack.active = 1; + stack.ext = { + windows: new Map([ + [ + 1, + { + meta: { + get_compositor_private: () => stackActor, + is_fullscreen: () => false, + }, + is_maximized: () => false, + }, + ], + ]), + maximized_on_active_display: () => false, + }; + stack.reposition(); + assert.deepEqual( + children.map((a) => a.name), + ["stack", "tabs", "floating", "dialog"], + ); + // Calling reposition repeatedly must not append tabs at the top of the layer. + stack.reposition(); + assert.deepEqual( + children.map((a) => a.name), + ["stack", "tabs", "floating", "dialog"], + ); +}); +test("destroying a stack cancels its queued restack callback", () => { + const removed = []; + let callback; + const Subject = methods( + stackPath, + "Stack", + ["restack", "restack_now", "destroy"], + { + lib: { + later_add(t, cb) { + callback = cb; + return 9; + }, + later_remove: (id) => removed.push(id), + }, + Meta: { LaterType: { BEFORE_REDRAW: 1 } }, + GLib: { SOURCE_REMOVE: false }, + global: { display: { disconnect() {} } }, + }, + ); + const stack = new Subject(); + stack._restack_id = null; + stack.active_disconnect = () => {}; + stack.tabs = []; + stack.buttons = new Map(); + stack.widgets = { tabs: { destroy() {} } }; + stack.is_disposed = () => !stack.widgets; + stack.restack(); + stack.destroy(); + assert.deepEqual(removed, [9]); + assert.equal(stack.widgets, null); + // Even a stale callback must not touch disposed actors. + callback(); + assert.equal(stack._restack_id, null); +}); +test("queued border restack cannot show or reposition the hint during a drag", () => { + let callback; + let layouts = 0; + let hidden = 0; + const Subject = methods(winPath, "ShellWindow", ["restack"], { + RESTACK_STATE: { NORMAL: 0 }, + GLib: { SOURCE_REMOVE: false }, + lib: { + later_add(type, cb) { + callback = cb; + return 1; + }, + later_remove() {}, + }, + Meta: { LaterType: { BEFORE_REDRAW: 1 } }, + }); + const win = new Subject(); + win.grab = false; + win._restack_id = null; + win.border = {}; + win.update_border_layout = () => layouts++; + win.hide_border = () => hidden++; + win.meta = { is_fullscreen: () => false, minimized: false }; + win.is_maximized = () => false; + win.restack(); + assert.equal(layouts, 1); + win.grab = true; + callback(); + assert.equal(layouts, 1); + assert.equal(hidden, 1); +}); +test("tiled drag end still runs tiling cleanup and border settling", () => { + const Subject = methods(extPath, "Ext", ["on_grab_end"]); + const ext = new Subject(); + let settled = 0; + let dropped = 0; + let cleaned = 0; + const win = { + grab: true, + mark_border_settling: () => settled++, + is_tilable: () => true, + }; + ext.get_window = () => win; + ext.grab_op = {}; + ext.on_grab_end_ = () => dropped++; + ext.unset_grab_op = () => cleaned++; + ext.on_grab_end({}, 1, {}); + assert.equal(win.grab, false); + assert.equal(settled, 1); + assert.equal(dropped, 1); + assert.equal(cleaned, 1); +}); +test("a pre-drag show timer cannot bypass post-drag settling", () => { + const { win, callbacks } = windowFixture(); + win.show_border(); + win.border.hide(); + win.grab = false; + win._border_settle_id = 99; + callbacks[0](); + assert.equal(win.border.visible, false); +}); +test("post-drag settling accepts an already committed frame after one interval", () => { + const callbacks = []; + let shown = 0; + const rect = { x: 20, y: 30, width: 400, height: 300 }; + const Subject = methods( + winPath, + "ShellWindow", + ["mark_border_settling", "arm_border_settle"], + { + GLib: { + PRIORITY_DEFAULT: 0, + SOURCE_REMOVE: false, + SOURCE_CONTINUE: true, + source_remove() {}, + timeout_add(p, ms, cb) { + callbacks.push(cb); + return callbacks.length; + }, + }, + BORDER_SETTLE_MAX_TRIES: 8, + }, + ); + const win = new Subject(); + win._border_settle_id = null; + win.hide_border = () => {}; + win.meta = { get_frame_rect: () => rect, is_fullscreen: () => false }; + win.actor_exists = () => true; + win.border = {}; + win.ext = { focus_window: () => win }; + win.is_maximized = () => false; + win.show_border = () => shown++; + win.mark_border_settling(false); + assert.equal(callbacks[0](), false); + assert.equal(shown, 1); +}); diff --git a/tests/shell-methods.mjs b/tests/shell-methods.mjs new file mode 100644 index 0000000..cd2f3a2 --- /dev/null +++ b/tests/shell-methods.mjs @@ -0,0 +1,39 @@ +import { readFileSync } from "node:fs"; +import ts from "typescript"; + +// Compile real production methods in isolation, supplying GNOME objects at their boundary. +// This avoids importing Shell resources or reimplementing the logic under test. +export function methods(path, className, names, globals = {}) { + const source = ts.createSourceFile( + path, + readFileSync(new URL("../" + path, import.meta.url), "utf8"), + ts.ScriptTarget.Latest, + true, + ); + const cls = source.statements.find( + (n) => ts.isClassDeclaration(n) && n.name?.text === className, + ); + const selected = cls.members.filter( + (n) => n.name && names.includes(n.name.getText(source)), + ); + if (selected.length !== names.length) + throw new Error( + "Missing production method: " + + names.filter( + (name) => !selected.some((n) => n.name.getText(source) === name), + ), + ); + const code = + "class Subject {\n" + + selected.map((n) => n.getText(source)).join("\n") + + "\n}"; + const js = ts.transpileModule(code, { + compilerOptions: { + target: ts.ScriptTarget.ES2022, + module: ts.ModuleKind.None, + }, + }).outputText; + return new Function(...Object.keys(globals), js + "\nreturn Subject;")( + ...Object.values(globals), + ); +} From 5a7d47ac56b9cc0260ceade7181f0266dd17c3c5 Mon Sep 17 00:00:00 2001 From: Italo Date: Tue, 20 Oct 2026 11:16:20 -0300 Subject: [PATCH 2/2] fix: reassign stack tab owner when active window detaches --- src/engine/stack.ts | 10 ++++++ tests/floating-decorations.test.mjs | 49 +++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/src/engine/stack.ts b/src/engine/stack.ts index fa7fb86..ab088fc 100644 --- a/src/engine/stack.ts +++ b/src/engine/stack.ts @@ -545,10 +545,20 @@ export class Stack { let idx = 0; for (const c of this.tabs) { if (Ecs.entity_eq(c.entity, entity)) { + const was_active = Ecs.entity_eq(this.active, entity); + if (was_active) this.active_disconnect(); this.remove_tab_component(c, idx); if (this.active_id > idx) { this.active_id -= 1; } + if (was_active && this.tabs.length > 0) { + // A detached window may now float above this stack. Stop + // anchoring the tab bar/signals to it before the next reflow. + this.auto_activate(); + this.prev_active = null; + this.prev_active_id = 0; + this.restack(); + } return idx; } idx += 1; diff --git a/tests/floating-decorations.test.mjs b/tests/floating-decorations.test.mjs index 9a1cf85..13a0589 100644 --- a/tests/floating-decorations.test.mjs +++ b/tests/floating-decorations.test.mjs @@ -331,3 +331,52 @@ test("post-drag settling accepts an already committed frame after one interval", assert.equal(callbacks[0](), false); assert.equal(shown, 1); }); +for (const activeId of [1, 2, 3]) { + test(`removing active tab ${activeId} immediately stops anchoring the stack to its floating window`, () => { + const Subject = methods( + stackPath, + "Stack", + ["remove_tab", "auto_activate"], + { Ecs: { entity_eq: (a, b) => a === b } }, + ); + const stack = new Subject(); + stack.widgets = {}; + stack.tabs = [1, 2, 3].map((entity) => ({ entity })); + stack.active = activeId; + stack.active_id = activeId - 1; + stack.prev_active = null; + let disconnected = 0; + let restacked = 0; + stack.active_disconnect = () => disconnected++; + stack.remove_tab_component = (c, idx) => stack.tabs.splice(idx, 1); + stack.activate = (entity) => { + stack.active = entity; + stack.active_id = stack.tabs.findIndex((t) => t.entity === entity); + }; + stack.restack = () => restacked++; + assert.equal(stack.remove_tab(activeId), activeId - 1); + assert.notEqual(stack.active, activeId); + assert.ok(stack.tabs.some((t) => t.entity === stack.active)); + assert.equal(disconnected, 1); + assert.equal(restacked, 1); + }); +} +test("removing an inactive tab preserves the active owner", () => { + const Subject = methods(stackPath, "Stack", ["remove_tab", "auto_activate"], { + Ecs: { entity_eq: (a, b) => a === b }, + }); + const stack = new Subject(); + stack.widgets = {}; + stack.tabs = [1, 2, 3].map((entity) => ({ entity })); + stack.active = 3; + stack.active_id = 2; + stack.prev_active = null; + stack.remove_tab_component = (c, idx) => stack.tabs.splice(idx, 1); + stack.active_disconnect = () => + assert.fail("must not disconnect current owner"); + stack.activate = () => assert.fail("must not switch active tab"); + stack.restack = () => {}; + assert.equal(stack.remove_tab(1), 0); + assert.equal(stack.active, 3); + assert.equal(stack.active_id, 1); +});