x11: fix client left at (0,0) inside its frame after WM restart (titlebar drawn behind the client)

[science]

The bug

When muffin manages an already-mapped window — every window on the desktop after cinnamon --replace or a crash-recovery restart — the client can be left at (0,0) inside its frame instead of at (borders.total.left, borders.total.top). The frame is created and sized correctly and _NET_FRAME_EXTENTS advertises the titlebar, but the client covers it: the window looks like it lost its titlebar, with black strips on the right and bottom where the oversized frame shows through. The state never self-heals — later moves only move the frame, and later resizes send CWWidth|CWHeight without CWX|CWY.

Trigger: the client's root coordinates at manage time are exactly (borders.total.left, borders.total.top) — e.g. (10, 45) with the default theme. That sounds exotic, but it is precisely where the X save-set drops any window whose frame sat at the screen's top-left corner when the previous WM died: frames are destroyed with the old WM's connection, and save-set reparenting preserves absolute coordinates (frame at (0,0) + child offset (10,45) = client at root (10,45)). In practice, a window the user keeps parked in the top-left corner (sidebars, docks) gets this on every Cinnamon restart, while every other window re-frames fine.

Root cause

meta_window_x11_manage seeds the client-rect cache with root coordinates after meta_window_ensure_frame has already reparented the client to (0,0) inside the frame:

  if (window->decorated)
    meta_window_ensure_frame (window);      /* XReparentWindow(..., child_x = 0, child_y = 0) */
  ...
  priv->client_rect = window->rect;         /* ROOT coordinates from XGetWindowAttributes */

meta_window_x11_move_resize_internal then compares frame-relative target coordinates against that root-coordinate seed (the code comment even notes the coordinate-space switch right above the comparison):

  if (window->frame)
    {
      client_rect.x = borders.total.left;
      client_rect.y = borders.total.top;
    }

  if (client_rect.x != priv->client_rect.x ||
      client_rect.y != priv->client_rect.y)
    {
      need_move_client = TRUE;
      ...

When the pre-manage root position equals borders.total, the two compare equal, need_move_client stays FALSE, and the client stays at (0,0) where the reparent put it. Since the cache now claims the client is at the "correct" offset, nothing ever moves it again.

The fix

Seed client_rect.x/y with frame->child_x/child_y — where XReparentWindow actually placed the client — when the window was framed. One hunk in meta_window_x11_manage.

For comparison: mutter no longer has this pattern since the frames rework — meta-x11-frame.c queries the frame borders before reparenting and passes the real child offset to XReparentWindow, so there is no fix-up move to skip.

Validation

Visual demo

Screenshot filmstrip — healthy → WM killed → re-adopted, on 6.0.1 / unpatched master / patched master (5 frames, unedited scrot captures)

Two identical xterms; the only difference is position. "victim" (red) is parked with its frame at the screen's top-left corner, "control" (green) sits mid-screen. muffin is SIGKILLed and restarted. On 6.0.1 and unpatched master the victim's client is left at (0,0) inside its frame — titlebar behind the client, black strips where the oversized frame shows through — while the control re-frames correctly. With this patch both re-frame correctly.

Regression test

Self-contained regression test (attached: muffin-restart-reframe-test.sh; runs on a nested Xvfb, no effect on the host session). It starts muffin, maps two xterms, reads the healthy child offset (theme-independent), SIGKILLs muffin, parks the "victim" xterm at exactly that offset in root coordinates, restarts muffin, and compares both windows' child offsets:

muffin	result
6.0.1 (Ubuntu 24.04 package)	FAIL — victim at (0,0), control at (10,45)
master b332877, unpatched, built locally	FAIL — victim at (0,0), control at (10,45)
master b332877 + this patch	PASS — victim at (10,45), control at (10,45)

=== unpatched master ===
[info] healthy frame child offset: (10, 45)
[info] victim parked at root (10, 45); starting muffin #2
[info] after re-adoption: victim child offset: (0 0)  control: (10 45)
FAIL: victim left at (0 0) inside its frame — titlebar is behind the client

=== patched master ===
[info] healthy frame child offset: (10, 45)
[info] victim parked at root (10, 45); starting muffin #2
[info] after re-adoption: victim child offset: (10 45)  control: (10 45)
PASS: victim re-framed correctly at (10 45)

muffin-restart-reframe-test.sh (run with MUFFIN_BIN=/path/to/muffin)

#!/bin/bash
# Regression test for the WM-restart re-adoption bug: a client whose root
# position at adoption time equals (borders.total.left, borders.total.top)
# is left at (0,0) inside its new frame, covering its own titlebar.
#
# Self-contained on a nested Xvfb display (:97) with a throwaway HOME.
# Requires: Xvfb, xterm, xdotool, x11-utils (xwininfo/xprop/xdpyinfo),
# dbus-run-session, and a muffin binary.
#
#   MUFFIN_BIN=/path/to/muffin ./muffin-restart-reframe-test.sh
#
# Flow:
#   1. muffin #1 manages two xterms ("victim", "control").
#   2. Read the frame's interior child offset (CHILD_X, CHILD_Y) from the
#      healthy state — this makes the test theme-independent.
#   3. SIGKILL muffin #1: frames die with its connection; the X save-set
#      returns clients to the root PRESERVING ABSOLUTE COORDINATES.
#   4. Park the victim at root (CHILD_X, CHILD_Y) — exactly where the
#      save-set drops any window whose frame sat at the screen origin.
#   5. Start muffin #2 and compare both windows' child offsets.
#
# PASS: victim re-framed at (CHILD_X, CHILD_Y) like the control.
# FAIL: victim left at (0,0) inside its frame.
set -uo pipefail
export DISPLAY=:97
MUFFIN_BIN="${MUFFIN_BIN:-/usr/bin/muffin}"
FAKEHOME="$(mktemp -d)"
PIDS=()
cleanup() {
  for p in "${PIDS[@]:-}"; do kill "$p" 2>/dev/null; done
  sleep 0.5
  for p in "${PIDS[@]:-}"; do kill -9 "$p" 2>/dev/null; done
}
trap cleanup EXIT

command -v "$MUFFIN_BIN" >/dev/null || { echo "FATAL: muffin binary not found: $MUFFIN_BIN"; exit 2; }

Xvfb :97 -screen 0 1280x900x24 2>/dev/null &
PIDS+=($!)
for i in $(seq 50); do xdpyinfo >/dev/null 2>&1 && break; sleep 0.2; done
xdpyinfo >/dev/null 2>&1 || { echo "FATAL: Xvfb didn't come up"; exit 2; }

export FAKEHOME MUFFIN_BIN
dbus-run-session -- bash -c '
set -u
export DISPLAY=:97
export HOME="$FAKEHOME"
export XDG_CACHE_HOME="$HOME/.cache" XDG_CONFIG_HOME="$HOME/.config"
export XDG_RUNTIME_DIR="$HOME/xdgrt"; mkdir -p "$XDG_RUNTIME_DIR"; chmod 700 "$XDG_RUNTIME_DIR"
export LIBGL_ALWAYS_SOFTWARE=1 CLUTTER_BACKEND=x11

# child_offset <name> → "X Y" of the named client inside its frame,
# from the relative geometry field of xwininfo -root -tree.
child_offset() {
  xwininfo -root -tree \
    | grep -oP "\"$1\": \(\"xterm\" \"XTerm\"\)\s+\d+x\d+\+\K\d+\+\d+" \
    | head -1 | tr "+" " "
}
wait_for_wm() {
  for i in $(seq 60); do
    xprop -root _NET_SUPPORTING_WM_CHECK 2>/dev/null | grep -q "window id" && return 0
    sleep 0.5
  done
  return 1
}

"$MUFFIN_BIN" >"$HOME/muffin1.log" 2>&1 &
M1=$!
wait_for_wm || { echo "FATAL: muffin #1 never managed"; tail -5 "$HOME/muffin1.log"; exit 2; }
sleep 2

xterm -T victim  & V=$!
xterm -T control & X=$!
sleep 3

read -r CHILD_X CHILD_Y <<<"$(child_offset victim)"
if [ -z "${CHILD_X:-}" ] || [ "$CHILD_X$CHILD_Y" = "00" ]; then
  echo "FATAL: could not read a sane healthy child offset (got: ${CHILD_X:-?},${CHILD_Y:-?})"
  exit 2
fi
echo "[info] healthy frame child offset: ($CHILD_X, $CHILD_Y)"

# WM "crash": frames are destroyed with the connection; save-set returns
# clients to root at unchanged absolute coordinates.
kill -9 $M1
sleep 2

VICTIM_WID=$(xwininfo -root -tree | grep -oP "0x[0-9a-f]+(?= \"victim\")" | head -1)
xdotool windowmove "$VICTIM_WID" "$CHILD_X" "$CHILD_Y"
sleep 1
echo "[info] victim parked at root ($CHILD_X, $CHILD_Y); starting muffin #2"

"$MUFFIN_BIN" >"$HOME/muffin2.log" 2>&1 &
M2=$!
wait_for_wm || { echo "FATAL: muffin #2 never managed"; tail -5 "$HOME/muffin2.log"; exit 2; }
sleep 4

VICTIM_OFF=$(child_offset victim)
CONTROL_OFF=$(child_offset control)
echo "[info] after re-adoption: victim child offset: ($VICTIM_OFF)  control: ($CONTROL_OFF)"

kill $V $X $M2 2>/dev/null

if [ "$VICTIM_OFF" != "$CHILD_X $CHILD_Y" ]; then
  echo "FAIL: victim left at ($VICTIM_OFF) inside its frame — titlebar is behind the client"
  exit 1
fi
echo "PASS: victim re-framed correctly at ($VICTIM_OFF)"
exit 0
'

Note on a related corner

The other two meta_window_ensure_frame callers (MOTIF-hints reload in window-props.c, window-type change in window.c) have the same theoretical exposure: the cache holds the unframed root coordinates when the reparent-to-(0,0) happens. Hitting it requires a client to toggle decorations while positioned exactly on borders.total root coordinates, which doesn't happen organically — and fixing those paths would need window-x11 private access from other modules (or mutter's approach of reparenting at the real offset from the start). This PR takes the minimal fix for the path users actually hit; happy to extend it if you'd prefer the broader change.

Environment

Observed on Cinnamon 6.0.4 / muffin 6.0.1 (Ubuntu 24.04.4) on two machines; root-caused and fixed against master.

🤖 Generated with Claude Code