Make popup windows fully visible on focus and attention-demand#1173
Open
develop7 wants to merge 9 commits into
Open
Make popup windows fully visible on focus and attention-demand#1173develop7 wants to merge 9 commits into
develop7 wants to merge 9 commits into
Conversation
Author
Hickey/Lowy AnalysisPost-implement structural review (two parallel sub-agents). Both lenses converged on the same three headline findings.
Notes
|
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).
… 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.
…ounds 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.
…vel 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.
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.
…t all popup-class windows 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.
…fecycle) 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.
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.
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.
develop7
force-pushed
the
fix/popup-visibility-demands-attention
branch
from
July 20, 2026 13:36
c5c09cc to
1447601
Compare
develop7
marked this pull request as ready for review
July 20, 2026 15:12
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1174
UPD: I've tested the fix and it works for me just fine, am using it daily.
Transient/popup windows are now repositioned fully on-screen when they appear, instead of being left wherever mutter clamped them — usually half-off the monitor edge, partially visible.
Popups (transients and dialogs) are real
MetaWindows that mutter positions itself. PaperWM never scrolled them — they're rejected byadd_filterso they're not in the clone container, meaningensureViewport/move_to(which movecloneContainer) physically can't reach them. Two entry points now reposition them withmove_frame:focus_handlerearly-returned; popup stayed clampedwindow-demands-attentionhandlerThe demands-attention path is the common one for background-spawned popups (most are
skip_taskbar, so gnome-shell's defaultWindowAttentionHandlerbails on them too — they previously disappeared entirely).How it's structured
The pure pieces live in a new shell-free
popuputil.jsso they're unit-testable with standalone GJS (tiling.jsimportsgi://Meta+ the live shell, so it can't be loaded in plaingjs):workAreaToBounds— reconcilesSpace.workArea()(monitor-relative) to the screen-absolute coordsget_frame_rect/move_frameuse. Folding the monitor origin in here is the load-bearing detail: without it the popup jumps to the wrong monitor on multi-head.computeClampedPosition— clamps a frame into bounds; an oversize window pins to the leading edge rather than going negative.tiling.jsholds the thin shell-facing adapters:isPopupClass(derived from the canonicaladd_filter, minus sticky/scratch),ensureVisibleInWorkArea(clamp +move_frame), andpositionPopupOnDemand(the demands-attention handler, no focus steal).Caveats
isTransient); the broaderisPopupClass(transients + non-NORMAL dialogs) is used only on the new demands-attention path. Widening the focus branch too regressed side-effects for standalone non-NORMAL windows (topbar/overlay/selection), caught in review.move_frameon Wayland may be re-clamped by mutter's transient constraints on a later pass — the pure positioning logic is unit-tested, but the livemove_framebehaviour needs manual verification in a gnome-shell session (can't be automated without a running compositor).tests/test-popup-visibility.js(17 checks,gjs -m) covers the coordinate reconciliation, the clamp arithmetic including the oversize edge case, and a multi-monitor end-to-end.Generated by
/doon opencode (modelzai-coding-plan/glm-5.2).