From ea184ba45d7a780d8d8936ee627179a8f3930534 Mon Sep 17 00:00:00 2001 From: Andrei Dziahel Date: Thu, 16 Jul 2026 14:59:24 +0200 Subject: [PATCH 1/9] Make popup windows fully visible on focus and attention-demand Popups (transients and non-NORMAL dialogs) are real MetaWindows that mutter positions itself; PaperWM does not scroll them via the clone container, so they were often left clamped at a monitor edge, partially off-screen. Two entry points now reposition them fully on-screen via move_frame: - Case A (popup gains focus): focus_handler repositions popup-class windows instead of early-returning. Predicate widened from isTransient to isPopupClass to also cover non-transient dialogs (MODAL_DIALOG/UTILITY without a parent). - Case B (popup demands attention, focus denied by focus-stealing prevention): new window-demands-attention / window-marked-urgent handler makes the popup visible WITHOUT stealing focus, by design. PaperWM connected neither before. Pure helpers (workAreaToBounds, computeClampedPosition, classifyPopup) live in a shell-free popuputil.js so they are unit-testable with standalone GJS. The coordinate reconciliation (workArea is monitor-relative; frame/move_frame are screen-absolute) is folded into workAreaToBounds to avoid the multi-monitor wrong-monitor bug. tests/test-popup-visibility.js covers it (25 checks, gjs -m). --- imports.js | 1 + popuputil.js | 66 +++++++++++++ tests/test-popup-visibility.js | 171 +++++++++++++++++++++++++++++++++ tiling.js | 57 ++++++++++- 4 files changed, 293 insertions(+), 2 deletions(-) create mode 100644 popuputil.js create mode 100644 tests/test-popup-visibility.js diff --git a/imports.js b/imports.js index 94c5a9605..0f975c7c9 100644 --- a/imports.js +++ b/imports.js @@ -17,3 +17,4 @@ export * as Topbar from './topbar.js'; export * as Utils from './utils.js'; export * as Workspace from './workspace.js'; export * as OverviewLayout from './overviewlayout.js'; +export * as PopupUtil from './popuputil.js'; diff --git a/popuputil.js b/popuputil.js new file mode 100644 index 000000000..e1360da1a --- /dev/null +++ b/popuputil.js @@ -0,0 +1,66 @@ +/** + * Pure (shell-free) helpers for positioning popup/transient windows. + * + * Deliberately free of `gi://` and gnome-shell imports so it can be unit-tested + * with standalone GJS (tests/test-popup-visibility.js). `tiling.js`'s + * `ensureVisibleInWorkArea` is a thin adapter that feeds live MetaWindow / + * Space objects into these functions. + */ + +/** + * Reconcile a Space's monitor-relative workArea into screen-absolute bounds. + * + * `Space.workArea()` returns monitor-relative coordinates (it subtracts + * `monitor.x` / `monitor.y`), while `MetaWindow.get_frame_rect()` and + * `move_frame()` are screen-absolute. Folding the monitor origin back in here + * avoids relocating windows to the wrong monitor on multi-head. + * + * @param {{x:number,y:number}} monitor the monitor's screen-absolute origin + * @param {{x:number,y:number,width:number,height:number}} workArea monitor-relative + * @returns {{x:number,y:number,width:number,height:number}} + */ +export function workAreaToBounds(monitor, workArea) { + return { + x: monitor.x + workArea.x, + y: monitor.y + workArea.y, + width: workArea.width, + height: workArea.height, + }; +} + +/** + * Clamp a frame rectangle into bounds, returning the target {x, y}. + * + * A window that fits is shifted minimally so it is fully inside bounds. One + * that is larger than bounds on an axis is pinned to the leading edge + * (minX / minY) rather than allowed to go negative. + * + * @param {{x:number,y:number,width:number,height:number}} frame + * @param {{x:number,y:number,width:number,height:number}} bounds + * @returns {{x:number,y:number}} + */ +export function computeClampedPosition(frame, bounds) { + const minX = bounds.x; + const maxX = bounds.x + bounds.width - frame.width; + const minY = bounds.y; + const maxY = bounds.y + bounds.height - frame.height; + const x = Math.max(minX, Math.min(frame.x, maxX)); + const y = Math.max(minY, Math.min(frame.y, maxY)); + return { x, y }; +} + +/** + * Whether a window is "popup-class" for visibility purposes: a real, non-tiled + * surface that mutter positions itself (transients plus non-NORMAL dialog / + * modal / utility windows), excluding sticky and scratch windows which have + * their own positioning. Takes plain booleans so it is testable without + * MetaWindow / Scratch. + * + * @param {{isTransient:boolean,isNormalType:boolean,onAllWorkspaces:boolean,isScratch:boolean}} w + * @returns {boolean} + */ +export function classifyPopup({ isTransient, isNormalType, onAllWorkspaces, isScratch }) { + if (onAllWorkspaces || isScratch) + return false; + return isTransient || !isNormalType; +} diff --git a/tests/test-popup-visibility.js b/tests/test-popup-visibility.js new file mode 100644 index 000000000..f95880d83 --- /dev/null +++ b/tests/test-popup-visibility.js @@ -0,0 +1,171 @@ +#!/usr/bin/env gjs +/* + * Unit tests for the pure popup-positioning helpers (popuputil.js). + * + * Run: gjs -m tests/test-popup-visibility.js + * + * These cover the shell-free pieces only — the coordinate reconciliation + * (workArea is monitor-relative, frame/move_frame are screen-absolute), the + * clamp arithmetic (incl. the oversize edge case), and the popup-class + * predicate. The shell-facing wrappers in tiling.js (ensureVisibleInWorkArea, + * the focus / demands-attention entry points) need a live gnome-shell session + * and are exercised manually. + */ + +import { workAreaToBounds, computeClampedPosition, classifyPopup } from '../popuputil.js'; +import system from 'system'; + +let _passed = 0, _failed = 0; +const _failures = []; + +function check(name, cond, detail) { + if (cond) { + _passed++; + } else { + _failed++; + _failures.push(detail ? `${name} — ${detail}` : name); + print(` FAIL: ${name}${detail ? ` — ${detail}` : ''}`); + } +} + +function rectEq(a, b) { + return Math.round(a.x) === Math.round(b.x) && Math.round(a.y) === Math.round(b.y); +} + +// ─── workAreaToBounds (the coordinate bug: monitor-relative vs screen-absolute) ─ + +print('\n== workAreaToBounds (coordinate reconciliation) =='); + +{ + const mon = { x: 0, y: 0 }; + const wa = { x: 0, y: 12, width: 1920, height: 1068 }; + const b = workAreaToBounds(mon, wa); + check('primary monitor keeps origin', b.x === 0 && b.y === 12, + `got x=${b.x} y=${b.y}`); + check('primary width/height pass through', b.width === 1920 && b.height === 1068); +} + +{ + // THE BUG SCENARIO: secondary monitor at x=1920. A naive clamp comparing + // screen-absolute frame.x against monitor-relative workArea.x (0) would + // relocate the window onto the primary monitor. Reconciliation adds 1920. + const mon = { x: 1920, y: 0 }; + const wa = { x: 0, y: 0, width: 1920, height: 1080 }; + const b = workAreaToBounds(mon, wa); + check('secondary monitor bounds.x == 1920', b.x === 1920, + `got ${b.x} (wrong monitor if 0)`); + check('secondary monitor right edge == 3840', b.x + b.width === 3840, + `got ${b.x + b.width}`); +} + +// ─── computeClampedPosition ──────────────────────────────────────────────────── + +print('\n== computeClampedPosition (clamp math) =='); + +{ + const bounds = { x: 0, y: 0, width: 1920, height: 1080 }; + + let r = computeClampedPosition({ x: 100, y: 100, width: 800, height: 600 }, bounds); + check('already visible: unchanged', rectEq(r, { x: 100, y: 100 }), + `got x=${r.x} y=${r.y}`); + + r = computeClampedPosition({ x: -200, y: 100, width: 800, height: 600 }, bounds); + check('off left: clamped to minX', r.x === 0, `got ${r.x}`); + + r = computeClampedPosition({ x: 1500, y: 100, width: 800, height: 600 }, bounds); + check('off right: clamped to maxX - width', r.x === 1920 - 800, `got ${r.x}`); + + r = computeClampedPosition({ x: 100, y: -50, width: 800, height: 600 }, bounds); + check('off top: clamped to minY', r.y === 0, `got ${r.y}`); + + r = computeClampedPosition({ x: 100, y: 700, width: 800, height: 600 }, bounds); + check('off bottom: clamped to maxY - height', r.y === 1080 - 600, `got ${r.y}`); + + // oversize: wider AND taller than the workarea must pin to the leading + // edge, not go negative (this case is why computeClampedPosition uses + // Math.max(minX, …) rather than a bare right-edge rule). + r = computeClampedPosition({ x: 500, y: 500, width: 3000, height: 2000 }, bounds); + check('oversize: clamped to minX', r.x === 0, `got ${r.x}`); + check('oversize: clamped to minY', r.y === 0, `got ${r.y}`); +} + +{ + // Multi-monitor: a popup half-off the RIGHT edge of the secondary monitor + // must stay on the secondary monitor, not jump to the primary. + const bounds = { x: 1920, y: 0, width: 1920, height: 1080 }; + const frame = { x: 1920 + 1500, y: 100, width: 800, height: 600 }; + const r = computeClampedPosition(frame, bounds); + check('multi-monitor: stays on secondary', + r.x === 1920 + (1920 - 800), `got ${r.x}, expected ${1920 + (1920 - 800)}`); + check('multi-monitor: not relocated to primary', r.x >= 1920, + `got ${r.x} (< 1920 would be the primary-monitor bug)`); +} + +// ─── classifyPopup (the predicate — incl. the non-transient-dialog gap) ──────── + +print('\n== classifyPopup (predicate) =='); + +check('transient (normal-type) -> true', + classifyPopup({ isTransient: true, isNormalType: true, onAllWorkspaces: false, isScratch: false }) === true); +check('transient MODAL_DIALOG -> true', + classifyPopup({ isTransient: true, isNormalType: false, onAllWorkspaces: false, isScratch: false }) === true); +check('non-transient MODAL_DIALOG -> true (the gap isTransient alone misses)', + classifyPopup({ isTransient: false, isNormalType: false, onAllWorkspaces: false, isScratch: false }) === true); +check('non-transient UTILITY -> true', + classifyPopup({ isTransient: false, isNormalType: false, onAllWorkspaces: false, isScratch: false }) === true); +check('normal tiled window -> false', + classifyPopup({ isTransient: false, isNormalType: true, onAllWorkspaces: false, isScratch: false }) === false); +check('sticky window (on_all_workspaces) -> false', + classifyPopup({ isTransient: true, isNormalType: false, onAllWorkspaces: true, isScratch: false }) === false); +check('scratch window -> false', + classifyPopup({ isTransient: true, isNormalType: false, onAllWorkspaces: false, isScratch: true }) === false); +check('normal sticky -> false', + classifyPopup({ isTransient: false, isNormalType: true, onAllWorkspaces: true, isScratch: false }) === false); + +// ─── end-to-end (workArea + frame + clamp, multi-monitor) ────────────────────── + +print('\n== end-to-end (workArea + frame + clamp, multi-monitor) =='); + +{ + // Reproduces the reported symptom: popup of an off-screen parent, partially + // visible (clamped to the monitor edge by mutter). Should land fully + // on-screen AND on the correct monitor. + const monitor = { x: 1920, y: 0 }; + const workArea = { x: 0, y: 0, width: 1920, height: 1080 }; + const bounds = workAreaToBounds(monitor, workArea); + const frame = { x: 3500, y: 100, width: 800, height: 600 }; + const target = computeClampedPosition(frame, bounds); + const fullyOnScreen = + target.x >= bounds.x && + target.y >= bounds.y && + target.x + frame.width <= bounds.x + bounds.width && + target.y + frame.height <= bounds.y + bounds.height; + check('end-to-end: popup fully on-screen', fullyOnScreen, + `target x=${target.x} y=${target.y}`); + check('end-to-end: on correct (secondary) monitor', target.x >= 1920, + `got ${target.x}`); + check('end-to-end: actually moved (not a no-op)', + !(target.x === frame.x && target.y === frame.y)); +} + +{ + // No-op case: popup already visible should not move (idempotency for + // repeated demands-attention signals). + const monitor = { x: 0, y: 0 }; + const workArea = { x: 0, y: 0, width: 1920, height: 1080 }; + const bounds = workAreaToBounds(monitor, workArea); + const frame = { x: 500, y: 400, width: 800, height: 600 }; + const target = computeClampedPosition(frame, bounds); + check('no-op when already visible', + target.x === frame.x && target.y === frame.y, + `expected unchanged, got x=${target.x} y=${target.y}`); +} + +// ─── summary ─────────────────────────────────────────────────────────────────── + +print(`\n━━━ ${_passed} passed, ${_failed} failed ━━━`); +if (_failed > 0) { + print('Failures:'); + _failures.forEach(f => print(` • ${f}`)); +} +system.exit(_failed > 0 ? 1 : 0); diff --git a/tiling.js b/tiling.js index a56ce3c8d..6fce5bfa4 100644 --- a/tiling.js +++ b/tiling.js @@ -13,6 +13,7 @@ import { import { Easer, DispatcherMode } from './utils.js'; import { ClickOverlay } from './stackoverlay.js'; import { WorkspaceSettings } from './workspace.js'; +import { workAreaToBounds, computeClampedPosition, classifyPopup } from './popuputil.js'; const { signals: Signals } = imports; const workspaceManager = global.workspace_manager; @@ -2304,6 +2305,11 @@ export const Spaces = class Spaces extends Map { this.signals.connect(display, 'window-created', (display, metaWindow, _user_data) => this.window_created(metaWindow)); + this.signals.connect(display, 'window-demands-attention', + (_display, metaWindow) => this.positionTransientOnDemand(metaWindow)); + this.signals.connect(display, 'window-marked-urgent', + (_display, metaWindow) => this.positionTransientOnDemand(metaWindow)); + this.signals.connect(display, 'grab-op-begin', (display, mw, type) => grabBegin(mw, type)); this.signals.connect(display, 'grab-op-end', (display, mw, type) => grabEnd(mw, type)); @@ -3396,6 +3402,18 @@ export const Spaces = class Spaces extends Map { return out; } + /** + * Handle a popup that demanded attention without getting focus (focus was + * denied by mutter's focus-stealing prevention). Make it fully visible + * without stealing focus — by design, the user's current window keeps focus. + * Non-popup demands-attention (a tiled window wanting attention) is left to + * gnome-shell's default handler. + */ + positionTransientOnDemand(metaWindow) { + if (isPopupClass(metaWindow)) + ensureVisibleInWorkArea(metaWindow); + } + /** * @param display * @param metaWindow {import("@gi-types/meta").Window} @@ -4683,6 +4701,37 @@ export function getDefaultFocusMode() { } // `MetaWindow::focus` handling +/** + * Whether `metaWindow` is popup-class: a real (non-tiled) surface mutter + * positions itself — transients and non-NORMAL dialogs/modals, but not sticky + * or scratch windows (which have their own positioning). Thin adapter over the + * shell-free `classifyPopup` (popuputil.js). + */ +export function isPopupClass(metaWindow) { + return classifyPopup({ + isTransient: !!metaWindow.get_transient_for(), + isNormalType: metaWindow.window_type === Meta.WindowType.NORMAL, + onAllWorkspaces: metaWindow.is_on_all_workspaces(), + isScratch: Scratch.isScratchWindow(metaWindow), + }); +} + +/** + * Reposition a popup-class window fully inside its space's workArea. + * + * Unlike tiled windows, popups are real MetaWindows positioned by mutter (not + * in the clone container), so they can't be scrolled via `ensureViewport`. We + * move them directly with `move_frame`. No-op if already fully on-screen. + */ +export function ensureVisibleInWorkArea(metaWindow) { + const space = spaces.spaceOfWindow(metaWindow); + const bounds = workAreaToBounds(space.monitor, space.workArea()); + const frame = metaWindow.get_frame_rect(); + const { x, y } = computeClampedPosition(frame, bounds); + if (x !== frame.x || y !== frame.y) + metaWindow.move_frame(true, x, y); +} + export function focus_handler(metaWindow) { console.debug("focus:", metaWindow?.title); if (Scratch.isScratchWindow(metaWindow)) { @@ -4692,9 +4741,13 @@ export function focus_handler(metaWindow) { return; } - // If metaWindow is a transient window, return (after deselecting tiled focus indicators) - if (isTransient(metaWindow)) { + // Popup-class window (transient / dialog): it's a real MetaWindow mutter + // positions itself, not in the clone container, so ensureViewport can't + // scroll it. Move it fully on-screen instead, then bail out of the tiled + // focus logic (after deselecting tiled focus indicators). + if (isPopupClass(metaWindow)) { setAllWorkspacesInactive(); + ensureVisibleInWorkArea(metaWindow); return; } From 5a66a442aee85db7ece5b25f4931df129cea8bd6 Mon Sep 17 00:00:00 2001 From: Andrei Dziahel Date: Thu, 16 Jul 2026 15:13:27 +0200 Subject: [PATCH 2/9] refactor(lowy): derive isPopupClass from add_filter (single source of truth) isPopupClass re-read the four tiling-eligibility booleans (transient / window_type / on_all_workspaces / scratch) that add_filter already inspects. Delegate to add_filter instead so the predicate follows automatically if tiling eligibility changes. classifyPopup stays in popuputil.js as the pure testable spec. Also adds the monitor-agreement precondition note to ensureVisibleInWorkArea. --- tiling.js | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tiling.js b/tiling.js index 6fce5bfa4..a419756b3 100644 --- a/tiling.js +++ b/tiling.js @@ -13,7 +13,7 @@ import { import { Easer, DispatcherMode } from './utils.js'; import { ClickOverlay } from './stackoverlay.js'; import { WorkspaceSettings } from './workspace.js'; -import { workAreaToBounds, computeClampedPosition, classifyPopup } from './popuputil.js'; +import { workAreaToBounds, computeClampedPosition } from './popuputil.js'; const { signals: Signals } = imports; const workspaceManager = global.workspace_manager; @@ -4704,16 +4704,15 @@ export function getDefaultFocusMode() { /** * Whether `metaWindow` is popup-class: a real (non-tiled) surface mutter * positions itself — transients and non-NORMAL dialogs/modals, but not sticky - * or scratch windows (which have their own positioning). Thin adapter over the - * shell-free `classifyPopup` (popuputil.js). + * or scratch windows (which have their own positioning). Derived from the + * canonical `add_filter` (single source of truth for tiling eligibility) minus + * sticky/scratch. The pure `classifyPopup` in popuputil.js stays as the + * testable spec. */ export function isPopupClass(metaWindow) { - return classifyPopup({ - isTransient: !!metaWindow.get_transient_for(), - isNormalType: metaWindow.window_type === Meta.WindowType.NORMAL, - onAllWorkspaces: metaWindow.is_on_all_workspaces(), - isScratch: Scratch.isScratchWindow(metaWindow), - }); + if (metaWindow.is_on_all_workspaces() || Scratch.isScratchWindow(metaWindow)) + return false; + return !add_filter(metaWindow); } /** @@ -4722,6 +4721,9 @@ export function isPopupClass(metaWindow) { * Unlike tiled windows, popups are real MetaWindows positioned by mutter (not * in the clone container), so they can't be scrolled via `ensureViewport`. We * move them directly with `move_frame`. No-op if already fully on-screen. + * + * Precondition: `metaWindow`'s space agrees with its physical monitor — + * cross-workspace popups are redirected by `insertWindow` before this runs. */ export function ensureVisibleInWorkArea(metaWindow) { const space = spaces.spaceOfWindow(metaWindow); From 4ef2b1d0c8db46eec6cec6159aae7eaad314df95 Mon Sep 17 00:00:00 2001 From: Andrei Dziahel Date: Thu, 16 Jul 2026 15:14:20 +0200 Subject: [PATCH 3/9] refactor(hickey): route cycleWindowWidthDirection through workAreaToBounds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The monitor-relative→screen-absolute reconciliation (workArea.x += monitor.x) is exactly what workAreaToBounds encapsulates. centerWindow is NOT converted here: it straddles two coordinate spaces (monitor-relative for move_to, screen-absolute for easeScratch), so forcing it through the single-space helper would regress clarity. --- tiling.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tiling.js b/tiling.js index a419756b3..04b05c514 100644 --- a/tiling.js +++ b/tiling.js @@ -5081,8 +5081,7 @@ export function cycleWindowWidthBackwards(metawindow) { export function cycleWindowWidthDirection(metaWindow, direction) { let frame = metaWindow.get_frame_rect(); let space = spaces.spaceOfWindow(metaWindow); - let workArea = space.workArea(); - workArea.x += space.monitor.x; + let workArea = workAreaToBounds(space.monitor, space.workArea()); let findFn = direction === CycleWindowSizesDirection.FORWARD ? Lib.findNext : Lib.findPrev; From f21345ba11da340965afd95a35b06d792901bb39 Mon Sep 17 00:00:00 2001 From: Andrei Dziahel Date: Thu, 16 Jul 2026 15:15:33 +0200 Subject: [PATCH 4/9] refactor(hickey): hoist+rename positionTransientOnDemand to module-level positionPopupOnDemand The handler used no instance state (lived on Spaces.prototype only by authoring convenience) and its name said 'Transient' while the body gates on isPopupClass (transients + non-NORMAL dialogs). Moved to a module-level free function next to ensureVisibleInWorkArea and renamed to match the widened predicate; connect sites drop the decorative this-binding. --- tiling.js | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/tiling.js b/tiling.js index 04b05c514..5abf68427 100644 --- a/tiling.js +++ b/tiling.js @@ -2306,9 +2306,9 @@ export const Spaces = class Spaces extends Map { (display, metaWindow, _user_data) => this.window_created(metaWindow)); this.signals.connect(display, 'window-demands-attention', - (_display, metaWindow) => this.positionTransientOnDemand(metaWindow)); + (_display, metaWindow) => positionPopupOnDemand(metaWindow)); this.signals.connect(display, 'window-marked-urgent', - (_display, metaWindow) => this.positionTransientOnDemand(metaWindow)); + (_display, metaWindow) => positionPopupOnDemand(metaWindow)); this.signals.connect(display, 'grab-op-begin', (display, mw, type) => grabBegin(mw, type)); this.signals.connect(display, 'grab-op-end', (display, mw, type) => grabEnd(mw, type)); @@ -3402,18 +3402,6 @@ export const Spaces = class Spaces extends Map { return out; } - /** - * Handle a popup that demanded attention without getting focus (focus was - * denied by mutter's focus-stealing prevention). Make it fully visible - * without stealing focus — by design, the user's current window keeps focus. - * Non-popup demands-attention (a tiled window wanting attention) is left to - * gnome-shell's default handler. - */ - positionTransientOnDemand(metaWindow) { - if (isPopupClass(metaWindow)) - ensureVisibleInWorkArea(metaWindow); - } - /** * @param display * @param metaWindow {import("@gi-types/meta").Window} @@ -4734,6 +4722,17 @@ export function ensureVisibleInWorkArea(metaWindow) { metaWindow.move_frame(true, x, y); } +/** + * Make a popup that demanded attention fully visible WITHOUT stealing focus + * (focus was denied by mutter's focus-stealing prevention, or never requested). + * By design the user's current window keeps focus. Non-popup demands-attention + * (a tiled window wanting attention) is left to gnome-shell's default handler. + */ +export function positionPopupOnDemand(metaWindow) { + if (isPopupClass(metaWindow)) + ensureVisibleInWorkArea(metaWindow); +} + export function focus_handler(metaWindow) { console.debug("focus:", metaWindow?.title); if (Scratch.isScratchWindow(metaWindow)) { From bdc6d92e7c04464343e5222ae66780c9e7d3de06 Mon Sep 17 00:00:00 2001 From: Andrei Dziahel Date: Thu, 16 Jul 2026 15:16:09 +0200 Subject: [PATCH 5/9] refactor(hickey): collapse duplicate non-NORMAL-type test cases classifyPopup takes a boolean isNormalType, so MODAL_DIALOG and UTILITY collapse to the same input (isNormalType:false). The two separately-labelled checks were byte-identical and overclaimed distinct window-type coverage. Merged into one 'any non-NORMAL type' assertion. --- tests/test-popup-visibility.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test-popup-visibility.js b/tests/test-popup-visibility.js index f95880d83..c7a995acc 100644 --- a/tests/test-popup-visibility.js +++ b/tests/test-popup-visibility.js @@ -109,9 +109,7 @@ check('transient (normal-type) -> true', classifyPopup({ isTransient: true, isNormalType: true, onAllWorkspaces: false, isScratch: false }) === true); check('transient MODAL_DIALOG -> true', classifyPopup({ isTransient: true, isNormalType: false, onAllWorkspaces: false, isScratch: false }) === true); -check('non-transient MODAL_DIALOG -> true (the gap isTransient alone misses)', - classifyPopup({ isTransient: false, isNormalType: false, onAllWorkspaces: false, isScratch: false }) === true); -check('non-transient UTILITY -> true', +check('any non-NORMAL type (dialog/modal/utility) -> true (the gap isTransient alone misses)', classifyPopup({ isTransient: false, isNormalType: false, onAllWorkspaces: false, isScratch: false }) === true); check('normal tiled window -> false', classifyPopup({ isTransient: false, isNormalType: true, onAllWorkspaces: false, isScratch: false }) === false); From 38d7689ca7df29afc3acdc8d059e74f41fd3fc58 Mon Sep 17 00:00:00 2001 From: Andrei Dziahel Date: Thu, 16 Jul 2026 15:27:05 +0200 Subject: [PATCH 6/9] =?UTF-8?q?fix(police):=20fact-check=20=E2=80=94=20sco?= =?UTF-8?q?pe=20focus=20repositioning=20to=20transients,=20not=20all=20pop?= =?UTF-8?q?up-class=20windows?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Widening focus_handler's branch from isTransient to isPopupClass (hickey-lowy pass) skipped the tiled-focus side effects (topbar restore, overlay, selection) that a non-transient non-NORMAL window (standalone DIALOG/UTILITY/SPLASHSCREEN) previously got via the fallthrough — a UI-state regression for e.g. a GIMP toolbox gaining focus on a fullscreen space. Revert the focus branch to isTransient (original behavior + the new repositioning). The broader isPopupClass lives only in the new demands-attention handler (Case B), which has no prior behavior to regress. --- tiling.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tiling.js b/tiling.js index 5abf68427..da66008fe 100644 --- a/tiling.js +++ b/tiling.js @@ -4742,11 +4742,11 @@ export function focus_handler(metaWindow) { return; } - // Popup-class window (transient / dialog): it's a real MetaWindow mutter - // positions itself, not in the clone container, so ensureViewport can't - // scroll it. Move it fully on-screen instead, then bail out of the tiled - // focus logic (after deselecting tiled focus indicators). - if (isPopupClass(metaWindow)) { + // Transient window: it's a real MetaWindow mutter positions itself, not in + // the clone container, so ensureViewport can't scroll it. Move it fully + // on-screen instead, then bail out of the tiled focus logic (after + // deselecting tiled focus indicators). + if (isTransient(metaWindow)) { setAllWorkspacesInactive(); ensureVisibleInWorkArea(metaWindow); return; From 8a64b3fa87aa97d75ed7205e0ef525f2a7e6e58c Mon Sep 17 00:00:00 2001 From: Andrei Dziahel Date: Thu, 16 Jul 2026 15:27:54 +0200 Subject: [PATCH 7/9] =?UTF-8?q?fix(police):=20fact-check=20=E2=80=94=20gua?= =?UTF-8?q?rd=20popup=20positioning=20(monitor-match=20+=20lifecycle)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ensureVisibleInWorkArea asserted (in its docstring) that the window's space agrees with its physical monitor, but that can be violated for existing windows whose workspace changed post-creation (D-Bus move, drag) — spaceOfWindow returns the workspace's monitor, so bounds could come from the wrong head and move_frame would yank the popup across monitors. Add an explicit get_monitor() guard. Also guard positionPopupOnDemand against demands-attention firing mid-teardown on an unmapped/actorless window. --- tiling.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tiling.js b/tiling.js index da66008fe..685cd1b02 100644 --- a/tiling.js +++ b/tiling.js @@ -4715,6 +4715,11 @@ export function isPopupClass(metaWindow) { */ export function ensureVisibleInWorkArea(metaWindow) { const space = spaces.spaceOfWindow(metaWindow); + // Bail if the window's physical monitor doesn't match its workspace's space + // (can desync on cross-workspace moves) — repositioning from the wrong + // monitor's workArea would yank the popup across heads. + if (metaWindow.get_monitor() !== space.monitor.index) + return; const bounds = workAreaToBounds(space.monitor, space.workArea()); const frame = metaWindow.get_frame_rect(); const { x, y } = computeClampedPosition(frame, bounds); @@ -4729,6 +4734,10 @@ export function ensureVisibleInWorkArea(metaWindow) { * (a tiled window wanting attention) is left to gnome-shell's default handler. */ export function positionPopupOnDemand(metaWindow) { + // Demands-attention / urgent can fire mid-teardown on a window whose actor + // is gone or frame not yet computed — bail before touching it. + if (!metaWindow || metaWindow.unmapped || !metaWindow.get_compositor_private()) + return; if (isPopupClass(metaWindow)) ensureVisibleInWorkArea(metaWindow); } From de35f3c7bae4a88f4a9e61ec43c694caa83d76e2 Mon Sep 17 00:00:00 2001 From: Andrei Dziahel Date: Thu, 16 Jul 2026 15:30:04 +0200 Subject: [PATCH 8/9] =?UTF-8?q?fix(police):=20no-dead-code=20=E2=80=94=20r?= =?UTF-8?q?emove=20test-only=20classifyPopup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After isPopupClass was refactored to delegate to add_filter (single source of truth), classifyPopup became a parallel pure encoding used only by the test — a stale 'spec' the implementation no longer transcribed, which would stay green while silently diverging from production (false-confidence generator). Removed classifyPopup from popuputil.js and the predicate test block; updated the isPopupClass docstring to drop the false spec claim. The geometry tests (the coordinate bug + oversize clamp, the genuinely valuable ones) remain. test now 17 checks. --- popuputil.js | 16 ---------------- tests/test-popup-visibility.js | 31 ++++++------------------------- tiling.js | 3 +-- 3 files changed, 7 insertions(+), 43 deletions(-) diff --git a/popuputil.js b/popuputil.js index e1360da1a..6f6a569dc 100644 --- a/popuputil.js +++ b/popuputil.js @@ -48,19 +48,3 @@ export function computeClampedPosition(frame, bounds) { const y = Math.max(minY, Math.min(frame.y, maxY)); return { x, y }; } - -/** - * Whether a window is "popup-class" for visibility purposes: a real, non-tiled - * surface that mutter positions itself (transients plus non-NORMAL dialog / - * modal / utility windows), excluding sticky and scratch windows which have - * their own positioning. Takes plain booleans so it is testable without - * MetaWindow / Scratch. - * - * @param {{isTransient:boolean,isNormalType:boolean,onAllWorkspaces:boolean,isScratch:boolean}} w - * @returns {boolean} - */ -export function classifyPopup({ isTransient, isNormalType, onAllWorkspaces, isScratch }) { - if (onAllWorkspaces || isScratch) - return false; - return isTransient || !isNormalType; -} diff --git a/tests/test-popup-visibility.js b/tests/test-popup-visibility.js index c7a995acc..c243a8eca 100644 --- a/tests/test-popup-visibility.js +++ b/tests/test-popup-visibility.js @@ -5,14 +5,14 @@ * Run: gjs -m tests/test-popup-visibility.js * * These cover the shell-free pieces only — the coordinate reconciliation - * (workArea is monitor-relative, frame/move_frame are screen-absolute), the - * clamp arithmetic (incl. the oversize edge case), and the popup-class - * predicate. The shell-facing wrappers in tiling.js (ensureVisibleInWorkArea, - * the focus / demands-attention entry points) need a live gnome-shell session - * and are exercised manually. + * (workArea is monitor-relative, frame/move_frame are screen-absolute) and the + * clamp arithmetic (incl. the oversize edge case). The shell-facing wrappers in + * tiling.js (ensureVisibleInWorkArea, the focus / demands-attention entry + * points, the isPopupClass predicate) need a live gnome-shell session and are + * exercised manually. */ -import { workAreaToBounds, computeClampedPosition, classifyPopup } from '../popuputil.js'; +import { workAreaToBounds, computeClampedPosition } from '../popuputil.js'; import system from 'system'; let _passed = 0, _failed = 0; @@ -101,25 +101,6 @@ print('\n== computeClampedPosition (clamp math) =='); `got ${r.x} (< 1920 would be the primary-monitor bug)`); } -// ─── classifyPopup (the predicate — incl. the non-transient-dialog gap) ──────── - -print('\n== classifyPopup (predicate) =='); - -check('transient (normal-type) -> true', - classifyPopup({ isTransient: true, isNormalType: true, onAllWorkspaces: false, isScratch: false }) === true); -check('transient MODAL_DIALOG -> true', - classifyPopup({ isTransient: true, isNormalType: false, onAllWorkspaces: false, isScratch: false }) === true); -check('any non-NORMAL type (dialog/modal/utility) -> true (the gap isTransient alone misses)', - classifyPopup({ isTransient: false, isNormalType: false, onAllWorkspaces: false, isScratch: false }) === true); -check('normal tiled window -> false', - classifyPopup({ isTransient: false, isNormalType: true, onAllWorkspaces: false, isScratch: false }) === false); -check('sticky window (on_all_workspaces) -> false', - classifyPopup({ isTransient: true, isNormalType: false, onAllWorkspaces: true, isScratch: false }) === false); -check('scratch window -> false', - classifyPopup({ isTransient: true, isNormalType: false, onAllWorkspaces: false, isScratch: true }) === false); -check('normal sticky -> false', - classifyPopup({ isTransient: false, isNormalType: true, onAllWorkspaces: true, isScratch: false }) === false); - // ─── end-to-end (workArea + frame + clamp, multi-monitor) ────────────────────── print('\n== end-to-end (workArea + frame + clamp, multi-monitor) =='); diff --git a/tiling.js b/tiling.js index 685cd1b02..b7a488cff 100644 --- a/tiling.js +++ b/tiling.js @@ -4694,8 +4694,7 @@ export function getDefaultFocusMode() { * positions itself — transients and non-NORMAL dialogs/modals, but not sticky * or scratch windows (which have their own positioning). Derived from the * canonical `add_filter` (single source of truth for tiling eligibility) minus - * sticky/scratch. The pure `classifyPopup` in popuputil.js stays as the - * testable spec. + * sticky/scratch, so it follows automatically if `add_filter` widens. */ export function isPopupClass(metaWindow) { if (metaWindow.is_on_all_workspaces() || Scratch.isScratchWindow(metaWindow)) From 87c2978b7cbce2a9d7faf5dc14eca03572cfe043 Mon Sep 17 00:00:00 2001 From: Andrei Dziahel Date: Fri, 17 Jul 2026 13:47:48 +0200 Subject: [PATCH 9/9] Add manual-verification trigger for the popup-visibility fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GJS/GTK4 script that opens a parent window, waits a delay (so the user can scroll it off-screen in PaperWM), then pops up a transient modal dialog — reproducing both Case A (popup gets focus) and Case B (demands-attention, focus denied). Verified manually to reproduce the bug pre-fix and confirm the fix post-fix. Companion to tests/test-popup-visibility.js, which covers only the pure shell-free geometry. --- tests/trigger-popup.js | 82 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 tests/trigger-popup.js diff --git a/tests/trigger-popup.js b/tests/trigger-popup.js new file mode 100644 index 000000000..7a3d23d42 --- /dev/null +++ b/tests/trigger-popup.js @@ -0,0 +1,82 @@ +#!/usr/bin/env gjs +/* + * Manual-verification trigger for the popup-visibility fix — the live-window + * companion to tests/test-popup-visibility.js (which covers only the pure, + * shell-free geometry). Verified to reproduce the bug pre-fix and confirm it + * post-fix. + * + * Run: gjs -m tests/trigger-popup.js [delay_secs] + * + * 1. Opens a parent window (PaperWM tiles it). + * 2. Waits seconds (default 10) — scroll the parent OUT OF VIEW. + * 3. Pops up a transient (modal) dialog for the parent. + * + * fix works ⇒ the dialog lands fully on-screen (Case A if it got focus, + * Case B / demands-attention if focus was denied — no focus steal + * either way, it's just repositioned). + * bug present ⇒ dialog clamped at the monitor edge / partially off-screen, + * or (Case B + skip_taskbar) not visible at all. + * + * To exercise Case A (popup gets focus), click the parent window right before + * the timer fires. To exercise Case B (focus denied — the common background + * case), just don't touch the parent after it opens; the popup arrives "stale" + * and mutter's focus-stealing prevention denies focus. + */ + +import Gtk from 'gi://Gtk?version=4.0'; +import GLib from 'gi://GLib'; + +const delay = parseInt(ARGV[0] ?? '10', 10); + +const app = new Gtk.Application({ application_id: 'org.paperwm.trigger' }); + +let parent; + +app.connect('activate', () => { + parent = new Gtk.ApplicationWindow({ + application: app, + title: 'PaperWM popup trigger — parent', + default_width: 520, + default_height: 350, + }); + parent.set_child(new Gtk.Label({ + label: `Parent window.\n\nIn ${delay}s a transient dialog will pop up.\n` + + '→ scroll me OUT OF VIEW in PaperWM now.', + })); + parent.connect('close-request', () => app.quit()); + parent.present(); + + print(`parent up — popup in ${delay}s (scroll the parent off-screen now)`); + + GLib.timeout_add_seconds(GLib.PRIORITY_DEFAULT, delay, () => { + const dlg = new Gtk.Window({ + transient_for: parent, + modal: true, + title: 'Transient popup', + default_width: 360, + default_height: 160, + destroy_with_parent: true, + }); + const box = new Gtk.Box({ + orientation: Gtk.Orientation.VERTICAL, + spacing: 12, + margin_top: 14, margin_bottom: 14, margin_start: 14, margin_end: 14, + }); + box.append(new Gtk.Label({ + label: 'Readable + fully on-screen ⇒ the fix works.\n' + + 'Clamped at the edge / invisible ⇒ bug.', + })); + const btn = new Gtk.Button({ label: 'Close' }); + btn.connect('clicked', () => { + dlg.destroy(); + app.quit(); + }); + box.append(btn); + dlg.set_child(box); + dlg.present(); + print('popup shown'); + return GLib.SOURCE_REMOVE; + }); +}); + +app.run([]);