Make popup windows fully visible on focus and attention-demand

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';

popuputil.js

@@ -0,0 +1,50 @@
+/**
+ * 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 };
+}

tests/test-popup-visibility.js

@@ -0,0 +1,150 @@
+#!/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) 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 } 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)`);
+}
+
+// ─── 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);

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 <delay> 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([]);

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 } 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) => positionPopupOnDemand(metaWindow));
+        this.signals.connect(display, 'window-marked-urgent',
+            (_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));
 
@@ -4683,6 +4689,58 @@ 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). Derived from the
+ * canonical `add_filter` (single source of truth for tiling eligibility) minus
+ * sticky/scratch, so it follows automatically if `add_filter` widens.
+ */
+export function isPopupClass(metaWindow) {
+    if (metaWindow.is_on_all_workspaces() || Scratch.isScratchWindow(metaWindow))
+        return false;
+    return !add_filter(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.
+ *
+ * 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);
+    // 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);
+    if (x !== frame.x || y !== frame.y)
+        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) {
+    // 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);
+}
+
 export function focus_handler(metaWindow) {
     console.debug("focus:", metaWindow?.title);
     if (Scratch.isScratchWindow(metaWindow)) {
@@ -4692,9 +4750,13 @@ export function focus_handler(metaWindow) {
         return;
     }
 
-    // If metaWindow is a transient window, return (after deselecting tiled focus indicators)
+    // 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;
     }
 
@@ -5026,8 +5088,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;