Fix Wayland internal UI lifecycle edge cases

internal/input/commands.go

@@ -484,3 +484,14 @@ func (router *CommandRouter) ReplaceBindings(bindings CommandBindings) {
 		router.bindings[chord] = command
 	}
 }
+
+// ReplaceConfiguration updates the modifier set and bindings without losing
+// held-key or multi-press state. Grab-only transitions need that continuity
+// when capture changes while the initiating chord is still held.
+func (router *CommandRouter) ReplaceConfiguration(modifiers []uint32, bindings CommandBindings) {
+	router.modifiers = make(map[uint32]bool, len(modifiers))
+	for _, symbol := range modifiers {
+		router.modifiers[symbol] = true
+	}
+	router.ReplaceBindings(bindings)
+}

internal/input/commands_test.go

@@ -68,6 +68,17 @@ func TestBindingTableRoutesPreferencesOnPress(t *testing.T) {
 	}
 }
 
+func TestReplaceConfigurationPreservesHeldModifier(t *testing.T) {
+	router := NewCommandRouter([]uint32{0xff63}, DefaultCommandBindings(DesktopKeyboardLayout))
+	if consumed, command := router.Handle(0xff63, 118, 0, true); !consumed || command != "" {
+		t.Fatalf("modifier press = %v %q", consumed, command)
+	}
+	router.ReplaceConfiguration([]uint32{0xff63}, DefaultCommandBindings(DesktopKeyboardLayout))
+	if consumed, command := router.Handle('t', 20, 0, true); !consumed || command != PresentTime {
+		t.Fatalf("held modifier was lost across reconfiguration: %v %q", consumed, command)
+	}
+}
+
 func TestApplicationPreferencesShortcut(t *testing.T) {
 	router := NewCommandRouter([]uint32{0xff63}, DefaultCommandBindings(DesktopKeyboardLayout))
 	if consumed, command := router.Handle(0xff63, 90, 0, true); !consumed || command != "" {

internal/platform/wayland/keyboard.go

@@ -61,6 +61,7 @@ type Keyboard struct {
 	active           atomic.Bool
 	learn            atomic.Bool
 	learnEcho        atomic.Bool
+	uiEcho           atomic.Bool
 	uiSession        atomic.Uint64
 	uiSequence       atomic.Uint64
 	uiClose          atomic.Uint64

internal/platform/wayland/protocol.go

@@ -65,6 +65,8 @@ type keyboardOwner struct {
 	captured      map[uint16]bool
 	learning      bool
 	uiSession     uint64
+	grabbed       bool
+	grabKnown     bool
 	started       time.Time
 	sequence      uint32
 	generation    uint64
@@ -114,22 +116,34 @@ func (owner *keyboardOwner) configure(ctx context.Context, configuration keyboar
 	if err := owner.call(ctx, "SetKeyGrabs", symbols, grabs); err != nil {
 		return err
 	}
-	grabKeyboard := policy.learn || policy.ui
-	if grabKeyboard != (owner.policy.learn || owner.policy.ui) {
+	grabKeyboard := policy.enabled && !policy.secure && (policy.learn || policy.ui)
+	if !owner.grabKnown || grabKeyboard != owner.grabbed {
 		method := "UngrabKeyboard"
 		if grabKeyboard {
 			method = "GrabKeyboard"
 		}
 		if err := owner.call(ctx, method); err != nil {
+			// A timeout can mean the compositor applied the transition
+			// anyway. Keep the applied grab state unknown so the next
+			// policy sync forces the compositor back to the desired state
+			// instead of trusting a diff against stale policy.
+			owner.grabKnown = false
 			return err
 		}
+		owner.grabbed = grabKeyboard
+		owner.grabKnown = true
 	}
 	owner.configuration, owner.policy, owner.bindings = configuration, policy, bindings
-	owner.router = input.NewCommandRouter(symbols, bindings)
+	if owner.router == nil {
+		owner.router = input.NewCommandRouter(symbols, bindings)
+	} else {
+		owner.router.ReplaceConfiguration(symbols, bindings)
+	}
 	owner.keyboard.applied.Store(policy.enabled && !policy.secure)
 	owner.keyboard.browseApplied.Store(policy.browse && !policy.bypass && policy.enabled && !policy.secure)
 	owner.keyboard.bypassApplied.Store(policy.bypass && policy.enabled && !policy.secure)
 	owner.keyboard.learnEcho.Store(policy.learn)
+	owner.keyboard.uiEcho.Store(policy.ui && owner.grabbed)
 	return nil
 }
 
@@ -139,6 +153,7 @@ func (keyboard *Keyboard) run(ctx context.Context, configuration keyboardConfigu
 	defer keyboard.browseApplied.Store(false)
 	defer keyboard.bypassApplied.Store(false)
 	defer keyboard.uiSession.Store(0)
+	defer keyboard.uiEcho.Store(false)
 	defer keyboard.active.Store(false)
 	defer keyboard.learn.Store(false)
 	defer keyboard.learnEcho.Store(false)
@@ -149,7 +164,7 @@ func (keyboard *Keyboard) run(ctx context.Context, configuration keyboardConfigu
 		return
 	}
 	defer connection.Close() // Also releases grabs if setup only partly succeeded.
-	owner := &keyboardOwner{keyboard: keyboard, connection: connection, configuration: configuration,
+	owner := &keyboardOwner{keyboard: keyboard, connection: connection, configuration: configuration, grabKnown: true,
 		held: make(map[uint16]uint32), captured: make(map[uint16]bool), started: time.Now()}
 	deadline, cancel := context.WithTimeout(ctx, callTimeout)
 	err = connection.BusObject().CallWithContext(deadline, "org.freedesktop.DBus.GetNameOwner", 0, managerName).Store(&owner.owner)
@@ -237,15 +252,16 @@ func (owner *keyboardOwner) loop(ctx context.Context, queue *signalQueue) error
 			if err := owner.call(ctx, method); err != nil {
 				return err
 			}
-		}
-		// A close request revokes the session even if its acknowledgement
-		// times out waiting for held keys, mirroring the Xorg owner. The
-		// pending update (if any) then just confirms the ungrab.
-		if owner.uiSession != 0 && keyboard.uiClose.Load() >= owner.uiSession {
-			owner.dismiss_internal_ui()
-			if err := owner.configure(ctx, owner.configuration, owner.requested_policy()); err != nil {
+			if err := owner.configure(ctx, owner.configuration, policy); err != nil {
 				return err
 			}
+		}
+		// A close request revokes the session identity promptly, even if
+		// its acknowledgement must wait for held keys. The held==0 policy
+		// sync below retains the compositor grab until those releases
+		// arrive, mirroring the Escape path and the Xorg owner.
+		if owner.uiSession != 0 && keyboard.uiClose.Load() >= owner.uiSession {
+			owner.dismiss_internal_ui()
 			policy = owner.requested_policy()
 		}
 		if !stopping && len(owner.held) == 0 {
@@ -266,9 +282,6 @@ func (owner *keyboardOwner) loop(ctx context.Context, queue *signalQueue) error
 					}
 				}
 				update.result <- reply
-				if reply.err != nil && update.closeSession != 0 {
-					return reply.err
-				}
 			} else if pending != nil {
 				update := pending
 				pending = nil
@@ -289,7 +302,7 @@ func (owner *keyboardOwner) loop(ctx context.Context, queue *signalQueue) error
 					keyboard.mutex.Unlock()
 				}
 				update.result <- err
-			} else if policy != owner.policy && (policy.bypass == owner.policy.bypass || len(owner.held) == 0) {
+			} else if policy != owner.policy || !owner.grabKnown {
 				if err := owner.configure(ctx, owner.configuration, policy); err != nil {
 					return err
 				}

internal/platform/wayland/protocol_test.go

@@ -26,12 +26,18 @@ type MonitorSnapshot struct {
 // FakeKeyboardMonitor verifies the real D-Bus wire contract without pretending
 // to prove physical key suppression by a compositor.
 type FakeKeyboardMonitor struct {
-	connection  *dbus.Conn
-	mutex       sync.Mutex
-	state       MonitorSnapshot
-	changes     chan MonitorSnapshot
-	rejectNext  bool
-	rejectWatch bool
+	connection      *dbus.Conn
+	mutex           sync.Mutex
+	state           MonitorSnapshot
+	changes         chan MonitorSnapshot
+	rejectNext      bool
+	rejectWatch     bool
+	failGrabNext    bool
+	grabDelay       time.Duration
+	grabStarted     chan struct{}
+	grabCalls       int
+	ungrabCalls     int
+	setKeyGrabCalls int
 }
 
 func (monitor *FakeKeyboardMonitor) publish() {
@@ -59,15 +65,33 @@ func (monitor *FakeKeyboardMonitor) UnwatchKeyboard() *dbus.Error {
 	return nil
 }
 func (monitor *FakeKeyboardMonitor) GrabKeyboard() *dbus.Error {
+	monitor.mutex.Lock()
+	monitor.grabCalls++
+	failApplied, delay, started := monitor.failGrabNext, monitor.grabDelay, monitor.grabStarted
+	monitor.failGrabNext = false
+	monitor.mutex.Unlock()
+	if started != nil {
+		select {
+		case started <- struct{}{}:
+		default:
+		}
+	}
+	if delay != 0 {
+		time.Sleep(delay)
+	}
 	monitor.mutex.Lock()
 	defer monitor.mutex.Unlock()
 	monitor.state.grabbed = true
 	monitor.publish()
+	if failApplied {
+		return dbus.NewError("org.freedesktop.DBus.Error.Timeout", []any{"grab applied but timed out"})
+	}
 	return nil
 }
 func (monitor *FakeKeyboardMonitor) UngrabKeyboard() *dbus.Error {
 	monitor.mutex.Lock()
 	defer monitor.mutex.Unlock()
+	monitor.ungrabCalls++
 	monitor.state.grabbed = false
 	monitor.publish()
 	return nil
@@ -75,6 +99,7 @@ func (monitor *FakeKeyboardMonitor) UngrabKeyboard() *dbus.Error {
 func (monitor *FakeKeyboardMonitor) SetKeyGrabs(modifiers []uint32, grabs []keyGrab) *dbus.Error {
 	monitor.mutex.Lock()
 	defer monitor.mutex.Unlock()
+	monitor.setKeyGrabCalls++
 	if monitor.rejectNext {
 		monitor.rejectNext = false
 		return dbus.NewError("org.freedesktop.DBus.Error.Failed", []any{"grab update denied"})
@@ -84,6 +109,37 @@ func (monitor *FakeKeyboardMonitor) SetKeyGrabs(modifiers []uint32, grabs []keyG
 	return nil
 }
 
+func (monitor *FakeKeyboardMonitor) grab_state() (grabbed bool, grabCalls, ungrabCalls, keyGrabs int) {
+	monitor.mutex.Lock()
+	defer monitor.mutex.Unlock()
+	return monitor.state.grabbed, monitor.grabCalls, monitor.ungrabCalls, len(monitor.state.grabs)
+}
+
+func (monitor *FakeKeyboardMonitor) fail_next_grab() {
+	monitor.mutex.Lock()
+	defer monitor.mutex.Unlock()
+	monitor.failGrabNext = true
+}
+
+func (monitor *FakeKeyboardMonitor) delay_next_grab(delay time.Duration) <-chan struct{} {
+	started := make(chan struct{}, 1)
+	monitor.mutex.Lock()
+	defer monitor.mutex.Unlock()
+	monitor.grabDelay, monitor.grabStarted = delay, started
+	return started
+}
+
+func wait_for(t *testing.T, check func() bool, message string) {
+	t.Helper()
+	deadline := time.Now().Add(3 * time.Second)
+	for !check() {
+		if time.Now().After(deadline) {
+			t.Fatal(message)
+		}
+		time.Sleep(time.Millisecond)
+	}
+}
+
 func start_monitor(t *testing.T) *FakeKeyboardMonitor {
 	t.Helper()
 	if os.Getenv("CTHULHU_NATIVE_DBUS_TEST") != "1" {
@@ -417,6 +473,17 @@ func TestNativeBypassAndInternalUIGrabLifecycle(t *testing.T) {
 	keyboard := start_keyboard(t, monitor)
 	keyboard.SetBrowseNavigation(true)
 	wait_snapshot(t, monitor, func(state MonitorSnapshot) bool { return len(state.grabs) != 0 })
+	monitor.key(t, 'h', 43, 0, false)
+	await_command(t, keyboard, input.DocumentNextHeading)
+	keyboard.SetCommandBypass(true)
+	heldDeadline := time.Now().Add(75 * time.Millisecond)
+	for time.Now().Before(heldDeadline) {
+		if _, _, _, keyGrabs := monitor.grab_state(); keyGrabs == 0 {
+			t.Fatal("bypass was applied while a key was held")
+		}
+		time.Sleep(time.Millisecond)
+	}
+	monitor.key(t, 'h', 43, 0, true)
 	keyboard.SetCommandBypass(true)
 	wait_snapshot(t, monitor, func(state MonitorSnapshot) bool { return len(state.grabs) == 0 })
 	deadline := time.Now().Add(3 * time.Second)
@@ -459,3 +526,192 @@ func TestNativeBypassAndInternalUIGrabLifecycle(t *testing.T) {
 		t.Fatal("internal UI session survived end")
 	}
 }
+
+func TestNativeInternalUISecureTransitionRevokesSession(t *testing.T) {
+	monitor := start_monitor(t)
+	keyboard := start_keyboard(t, monitor)
+	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
+	defer cancel()
+	session, err := keyboard.BeginInternalUI(ctx)
+	if err != nil || session == 0 {
+		t.Fatalf("begin internal UI: session %d, %v", session, err)
+	}
+	wait_snapshot(t, monitor, func(state MonitorSnapshot) bool { return state.grabbed })
+
+	keyboard.SetSecureInputActive(true)
+	if event := await_command(t, keyboard, input.InternalUIDismiss); event.UISession != session {
+		t.Fatalf("secure dismiss session = %d, want %d", event.UISession, session)
+	}
+	wait_snapshot(t, monitor, func(state MonitorSnapshot) bool { return !state.watching && !state.grabbed })
+	if keyboard.InternalUIActive() || keyboard.InternalUISession() != 0 {
+		t.Fatal("internal UI session survived secure transition")
+	}
+	_, grabCalls, _, _ := monitor.grab_state()
+	if _, err := keyboard.BeginInternalUI(ctx); err == nil {
+		t.Fatal("internal UI began while secure")
+	}
+	if _, afterCalls, _, _ := monitor.grab_state(); afterCalls != grabCalls {
+		t.Fatal("rejected secure begin still grabbed the keyboard")
+	}
+
+	keyboard.SetSecureInputActive(false)
+	wait_snapshot(t, monitor, func(state MonitorSnapshot) bool { return state.watching && !state.grabbed })
+	session, err = keyboard.BeginInternalUI(ctx)
+	if err != nil || session == 0 {
+		t.Fatalf("begin internal UI after unlock: session %d, %v", session, err)
+	}
+	wait_snapshot(t, monitor, func(state MonitorSnapshot) bool { return state.grabbed })
+	monitor.key(t, 0xff1b, 9, 0, false)
+	event := await_command(t, keyboard, input.InternalUIKey)
+	if event.UISession != session || event.Chord.Symbol != 0xff1b {
+		t.Fatalf("internal UI Escape = %+v", event)
+	}
+	monitor.key(t, 0xff1b, 9, 0, true)
+	wait_snapshot(t, monitor, func(state MonitorSnapshot) bool { return !state.grabbed })
+	if err := keyboard.EndInternalUI(ctx, session); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestNativeInternalUIEndWaitsForHeldKeyRelease(t *testing.T) {
+	monitor := start_monitor(t)
+	keyboard := start_keyboard(t, monitor)
+	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
+	defer cancel()
+	session, err := keyboard.BeginInternalUI(ctx)
+	if err != nil || session == 0 {
+		t.Fatalf("begin internal UI: session %d, %v", session, err)
+	}
+	wait_snapshot(t, monitor, func(state MonitorSnapshot) bool { return state.grabbed })
+	monitor.key(t, 0xff0d, 36, 0, false)
+	await_command(t, keyboard, input.InternalUIKey)
+
+	closed := make(chan error, 1)
+	go func() { closed <- keyboard.EndInternalUI(ctx, session) }()
+	wait_for(t, func() bool { return !keyboard.InternalUIActive() }, "session identity was not revoked promptly")
+	if grabbed, _, _, _ := monitor.grab_state(); !grabbed {
+		t.Fatal("keyboard was ungrabbed while Enter was held")
+	}
+	if !keyboard.InternalUIEchoActive() {
+		t.Fatal("UI echo suppression ended before held key release")
+	}
+	select {
+	case err := <-closed:
+		t.Fatalf("EndInternalUI returned before held key release: %v", err)
+	default:
+	}
+	monitor.key(t, 0xff0d, 36, 0, true)
+	select {
+	case err := <-closed:
+		if err != nil {
+			t.Fatal(err)
+		}
+	case <-time.After(3 * time.Second):
+		t.Fatal("EndInternalUI did not return after key release")
+	}
+	wait_snapshot(t, monitor, func(state MonitorSnapshot) bool { return !state.grabbed })
+	if keyboard.InternalUIEchoActive() {
+		t.Fatal("UI echo suppression survived ungrab")
+	}
+}
+
+func TestNativeShowActionsCapturePreservesHeldReaderModifier(t *testing.T) {
+	monitor := start_monitor(t)
+	keyboard := start_keyboard(t, monitor)
+	monitor.key(t, 0xff63, 118, 0, false)
+	monitor.key(t, 0xffe1, 50, 1, false)
+	monitor.key(t, 'A', 38, 1, false)
+	opened := await_command(t, keyboard, input.ShowActions)
+	if opened.UISession == 0 || opened.UISession != keyboard.InternalUISession() {
+		t.Fatalf("ShowActions did not capture atomically: %+v", opened)
+	}
+	wait_snapshot(t, monitor, func(state MonitorSnapshot) bool { return state.grabbed })
+	// Insert is still held from the opening chord. Its router state must
+	// survive the synchronous capture so this cannot become field text.
+	monitor.key(t, 'x', 53, 1, false)
+	monitor.key(t, 'x', 53, 1, true)
+	monitor.key(t, 'A', 38, 1, true)
+	monitor.key(t, 0xffe1, 50, 0, true)
+	monitor.key(t, 0xff63, 118, 0, true)
+	monitor.key(t, 'a', 38, 0, false)
+	event := await_command(t, keyboard, input.InternalUIKey)
+	if event.UISession != opened.UISession || event.Text != "a" {
+		t.Fatalf("first internal UI key = %+v", event)
+	}
+	monitor.key(t, 'a', 38, 0, true)
+	monitor.key(t, 0xff1b, 9, 0, false)
+	await_command(t, keyboard, input.InternalUIKey)
+	monitor.key(t, 0xff1b, 9, 0, true)
+	wait_snapshot(t, monitor, func(state MonitorSnapshot) bool { return !state.grabbed })
+}
+
+func TestNativeShowActionsSecureTransitionDoesNotBlockLock(t *testing.T) {
+	monitor := start_monitor(t)
+	keyboard := start_keyboard(t, monitor)
+	started := monitor.delay_next_grab(300 * time.Millisecond)
+	monitor.key(t, 0xff63, 118, 0, false)
+	monitor.key(t, 0xffe1, 50, 1, false)
+	monitor.key(t, 'A', 38, 1, false)
+	select {
+	case <-started:
+	case <-time.After(3 * time.Second):
+		t.Fatal("GrabKeyboard was not started")
+	}
+	secureDone := make(chan struct{})
+	go func() {
+		keyboard.SetSecureInputActive(true)
+		close(secureDone)
+	}()
+	select {
+	case <-secureDone:
+	case <-time.After(100 * time.Millisecond):
+		t.Fatal("secure transition blocked behind ShowActions capture")
+	}
+	if event := await_command(t, keyboard, input.InternalUIDismiss); event.UISession == 0 {
+		t.Fatalf("secure dismiss event = %+v", event)
+	}
+	wait_snapshot(t, monitor, func(state MonitorSnapshot) bool { return !state.watching && !state.grabbed })
+	keyboard.SetSecureInputActive(false)
+	deadline := time.Now().Add(3 * time.Second)
+	for {
+		monitor.mutex.Lock()
+		state := monitor.state
+		setKeyGrabCalls, snapshotQueue := monitor.setKeyGrabCalls, len(monitor.changes)
+		monitor.mutex.Unlock()
+		if state.watching && !state.grabbed {
+			break
+		}
+		if time.Now().After(deadline) {
+			select {
+			case err := <-keyboard.Failures():
+				t.Fatalf("keyboard did not resume watching after unlock: watching=%v grabbed=%v secure=%v enabled=%v uiSession=%d setKeyGrabCalls=%d snapshotQueue=%d failure=%v", state.watching, state.grabbed, keyboard.secure.Load(), keyboard.enabled.Load(), keyboard.InternalUISession(), setKeyGrabCalls, snapshotQueue, err)
+			default:
+				t.Fatalf("keyboard did not resume watching after unlock: watching=%v grabbed=%v secure=%v enabled=%v uiSession=%d setKeyGrabCalls=%d snapshotQueue=%d", state.watching, state.grabbed, keyboard.secure.Load(), keyboard.enabled.Load(), keyboard.InternalUISession(), setKeyGrabCalls, snapshotQueue)
+			}
+		}
+		time.Sleep(time.Millisecond)
+	}
+}
+
+func TestNativeInternalUIRecoversAmbiguousGrabFailure(t *testing.T) {
+	monitor := start_monitor(t)
+	keyboard := start_keyboard(t, monitor)
+	monitor.fail_next_grab()
+	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
+	defer cancel()
+	if session, err := keyboard.BeginInternalUI(ctx); err == nil || session != 0 {
+		t.Fatalf("ambiguous grab begin = session %d, %v", session, err)
+	}
+	wait_for(t, func() bool {
+		grabbed, _, ungrabCalls, _ := monitor.grab_state()
+		return !grabbed && ungrabCalls != 0
+	}, "ambiguous grab was not reconciled with UngrabKeyboard")
+	session, err := keyboard.BeginInternalUI(ctx)
+	if err != nil || session == 0 {
+		t.Fatalf("begin after grab recovery: session %d, %v", session, err)
+	}
+	wait_snapshot(t, monitor, func(state MonitorSnapshot) bool { return state.grabbed })
+	if err := keyboard.EndInternalUI(ctx, session); err != nil {
+		t.Fatal(err)
+	}
+}

internal/platform/wayland/routing.go

@@ -291,7 +291,22 @@ func (owner *keyboardOwner) handle_key(key keyEvent) error {
 			// Capture exclusively before reporting the command, as on Xorg.
 			// An unavailable modal grab is a failed command, not a failed
 			// reader: session zero makes dispatch announce unavailability.
-			if session, err := owner.begin_internal_ui(owner.ctx); err == nil {
+			// Do not hold the secure mutex across compositor IPC; a secure
+			// transition during the calls retires the just-created session
+			// instead of reporting a command from before the lock.
+			generation := owner.generation
+			keyboard.secureMutex.Unlock()
+			session, err := owner.begin_internal_ui(owner.ctx)
+			keyboard.secureMutex.Lock()
+			if err == nil {
+				if generation != keyboard.secureGeneration.Load() {
+					owner.retire_secure_generation()
+					return nil
+				}
+				if !keyboard.enabled.Load() {
+					owner.dismiss_internal_ui()
+					return nil
+				}
 				uiSession = session
 			}
 		}

internal/platform/wayland/routing_test.go

@@ -367,6 +367,18 @@ func TestInternalUIKeyRoutingAndEscapeDismiss(t *testing.T) {
 		t.Fatalf("UI tab event = %+v", event)
 	}
 	send_key(t, owner, 0xff09, 23, 0, true)
+	send_key(t, owner, 'A', 38, 1, false)
+	event = next_command(t, owner.keyboard, input.InternalUIKey)
+	if event.UISession != 7 || event.Text != "A" {
+		t.Fatalf("UI shifted letter event = %+v", event)
+	}
+	send_key(t, owner, 'A', 38, 1, true)
+	send_key(t, owner, '#', 12, 1, false)
+	event = next_command(t, owner.keyboard, input.InternalUIKey)
+	if event.UISession != 7 || event.Text != "#" {
+		t.Fatalf("UI shifted digit event = %+v", event)
+	}
+	send_key(t, owner, '3', 12, 0, true)
 	// Reader chords never become field contents.
 	send_key(t, owner, 0xff63, 118, 0, false)
 	send_key(t, owner, 'x', 53, 0, false)

internal/platform/wayland/ui.go

@@ -25,11 +25,10 @@ type uiUpdate struct {
 func (keyboard *Keyboard) InternalUISession() uint64 { return keyboard.uiSession.Load() }
 func (keyboard *Keyboard) InternalUIActive() bool    { return keyboard.uiSession.Load() != 0 }
 
-// InternalUIEchoActive follows the session itself; the owner retains the
-// compositor grab until the session's last held key is released and the
-// ungrab is applied, so echo suppression never outlives key capture by
-// more than one policy cycle.
-func (keyboard *Keyboard) InternalUIEchoActive() bool { return keyboard.uiSession.Load() != 0 }
+// InternalUIEchoActive follows the applied compositor grab for a UI
+// session. It remains active after the session identity is revoked until
+// the session's last held key is released and the ungrab is applied.
+func (keyboard *Keyboard) InternalUIEchoActive() bool { return keyboard.uiEcho.Load() }
 
 // BeginInternalUI acknowledges exclusive capture through the shared
 // interface's GrabKeyboard. EndInternalUI waits for physical releases,
@@ -75,8 +74,6 @@ func (keyboard *Keyboard) update_internal_ui(ctx context.Context, closeSession u
 	select {
 	case result := <-update.result:
 		return result.session, result.err
-	case <-ctx.Done():
-		return 0, ctx.Err()
 	case <-done:
 		return 0, errors.New("Wayland keyboard stopped")
 	}
@@ -85,7 +82,7 @@ func (keyboard *Keyboard) update_internal_ui(ctx context.Context, closeSession u
 // begin_internal_ui runs on the owner goroutine. The compositor grab is
 // applied before the session is acknowledged, mirroring the Xorg owner.
 func (owner *keyboardOwner) begin_internal_ui(ctx context.Context) (uint64, error) {
-	if !owner.policy.enabled || owner.policy.secure || owner.learning || owner.uiSession != 0 {
+	if !owner.policy.enabled || owner.policy.secure || !owner.keyboard.enabled.Load() || owner.keyboard.secure.Load() || owner.learning || owner.uiSession != 0 {
 		return 0, errors.New("internal UI input unavailable")
 	}
 	policy := owner.policy
@@ -141,7 +138,7 @@ func (owner *keyboardOwner) handle_ui_key(key keyEvent, symbol uint32, chord inp
 // falls back to the keysym's Unicode mapping. Control characters never
 // become field text.
 func key_text(key keyEvent, symbol uint32) string {
-	for _, candidate := range []uint32{key.character, symbol_utf32(symbol), symbol_utf32(key.symbol)} {
+	for _, candidate := range []uint32{key.character, symbol_utf32(key.symbol), symbol_utf32(symbol)} {
 		if candidate >= 0x20 && candidate != 0x7f && candidate <= 0x10ffff {
 			return string(rune(candidate))
 		}

internal/platform/wayland/ui_test.go

@@ -0,0 +1,40 @@
+// Copyright (c) 2026 Storm Dragon
+// Authors: Storm Dragon, Angus Kola
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package wayland
+
+import (
+	"context"
+	"testing"
+)
+
+func TestUpdateInternalUIWaitsForSentUpdateResult(t *testing.T) {
+	keyboard := &Keyboard{
+		done:      make(chan struct{}),
+		uiUpdates: make(chan uiUpdate),
+	}
+	ctx, cancel := context.WithCancel(context.Background())
+	type outcome struct {
+		session uint64
+		err     error
+	}
+	outcomes := make(chan outcome, 1)
+	go func() {
+		session, err := keyboard.update_internal_ui(ctx, 0)
+		outcomes <- outcome{session: session, err: err}
+	}()
+
+	update := <-keyboard.uiUpdates
+	cancel()
+	select {
+	case got := <-outcomes:
+		t.Fatalf("sent update was abandoned on cancellation: %+v", got)
+	default:
+	}
+	update.result <- uiReply{session: 9}
+	got := <-outcomes
+	if got.err != nil || got.session != 9 {
+		t.Fatalf("sent update outcome = %+v", got)
+	}
+}

internal/review/document_lists.go

@@ -172,15 +172,16 @@ func (worker *Worker) listDocument(ctx context.Context, cursor *DocumentCursor,
 // its own name, else its text, else its children's labels in provider
 // order. Password text never contributes its text or descendants.
 func documentNodeLabel(snapshot atspi.Snapshot, childLabels []string) string {
-	label := snapshot.Name
+	label := strings.TrimSpace(snapshot.Name)
 	if snapshot.Role != "password text" {
 		if label == "" {
-			label = snapshot.Text
+			label = strings.TrimSpace(snapshot.Text)
 		}
 		if label == "" {
 			var parts []string
 			size := 0
 			for _, part := range childLabels {
+				part = strings.TrimSpace(part)
 				if part != "" {
 					parts = append(parts, part)
 					size += len(part)
@@ -192,6 +193,7 @@ func documentNodeLabel(snapshot atspi.Snapshot, childLabels []string) string {
 			label = strings.Join(parts, " ")
 		}
 	}
+	label = strings.TrimSpace(label)
 	runes := []rune(label)
 	if len(runes) > 225 {
 		label = string(runes[:225])

internal/review/document_lists_test.go

@@ -113,53 +113,76 @@ func TestDocumentListDescendantLabelsExcludeHiddenAndPasswordText(t *testing.T)
 }
 
 func TestDocumentClickableMoveNamesUnnamedSectionFromDescendants(t *testing.T) {
-	node := func(path, role, text string, children ...string) atspi.TreeNode {
-		snapshot := atspi.Snapshot{BusName: webID(path).BusName, Path: path, Role: role, Text: text}
-		entry := atspi.TreeNode{Snapshot: snapshot}
-		for _, child := range children {
-			entry.Children = append(entry.Children, webID(child))
-		}
-		return entry
-	}
-	nodes := []atspi.TreeNode{
-		node("/doc", "document web", "", "/start", "/clickable"),
-		node("/start", "static", "Before"),
-		node("/clickable", "section", "", "/text", "/hidden", "/password"),
-		node("/text", "static", "Clickable text"),
-		node("/hidden", "static", "Hidden content"),
-		node("/password", "password text", "private fixture password"),
-	}
-	nodes[2].Snapshot.SupportsClick = true
-	nodes[4].Geometry = atspi.Geometry{VisibilityKnown: true}
-	tree, err := atspi.NewAccessibleTree(webID("/doc"), nodes)
-	if err != nil {
-		t.Fatal(err)
-	}
-	cursor, err := NewDocumentCursor(tree)
-	if err != nil {
-		t.Fatal(err)
-	}
-	ctx := context.Background()
-	if _, err := cursor.SyncFocus(ctx, webID("/start")); err != nil {
-		t.Fatal(err)
-	}
-	move, err := cursor.Move(ctx, DocumentClickable, 0, false)
-	if err != nil || !move.Moved || move.Position.Object.ID() != webID("/clickable") {
-		t.Fatalf("clickable move: %+v %v", move, err)
-	}
-	if move.Position.Snapshot.Name != "Clickable text" {
-		t.Fatalf("landed snapshot name = %q, want descendant label", move.Position.Snapshot.Name)
-	}
-	ref, err := tree.Reference(webID("/clickable"))
-	if err != nil {
-		t.Fatal(err)
-	}
-	captured, err := tree.Resolve(ref)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if captured.Name != "" {
-		t.Fatalf("captured snapshot mutated: name = %q", captured.Name)
+	for _, tc := range []struct {
+		name     string
+		rootName string
+		rootText string
+		editable bool
+		wantName string
+	}{
+		{name: "descendant label", wantName: "Clickable text"},
+		{name: "whitespace name and text", rootName: " \t", rootText: " ", wantName: "Clickable text"},
+		{name: "editable clickable", editable: true},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			node := func(path, role, text string, children ...string) atspi.TreeNode {
+				snapshot := atspi.Snapshot{BusName: webID(path).BusName, Path: path, Role: role, Text: text}
+				entry := atspi.TreeNode{Snapshot: snapshot}
+				for _, child := range children {
+					entry.Children = append(entry.Children, webID(child))
+				}
+				return entry
+			}
+			nodes := []atspi.TreeNode{
+				node("/doc", "document web", "", "/start", "/clickable"),
+				node("/start", "static", "Before"),
+				node("/clickable", "section", tc.rootText, "/text", "/hidden", "/password"),
+				node("/text", "static", "Clickable text"),
+				node("/hidden", "static", "Hidden content"),
+				node("/password", "password text", "private fixture password"),
+			}
+			nodes[2].Snapshot.Name = tc.rootName
+			nodes[2].Snapshot.SupportsClick = true
+			nodes[2].Snapshot.Editable = tc.editable
+			nodes[4].Geometry = atspi.Geometry{VisibilityKnown: true}
+			tree, err := atspi.NewAccessibleTree(webID("/doc"), nodes)
+			if err != nil {
+				t.Fatal(err)
+			}
+			cursor, err := NewDocumentCursor(tree)
+			if err != nil {
+				t.Fatal(err)
+			}
+			ctx := context.Background()
+			focused, err := cursor.SyncFocus(ctx, webID("/clickable"))
+			if err != nil {
+				t.Fatal(err)
+			}
+			if focused.Snapshot.Name != tc.rootName {
+				t.Fatalf("non-structural snapshot name = %q, want captured name %q", focused.Snapshot.Name, tc.rootName)
+			}
+			if _, err := cursor.SyncFocus(ctx, webID("/start")); err != nil {
+				t.Fatal(err)
+			}
+			move, err := cursor.Move(ctx, DocumentClickable, 0, false)
+			if err != nil || !move.Moved || move.Position.Object.ID() != webID("/clickable") {
+				t.Fatalf("clickable move: %+v %v", move, err)
+			}
+			if move.Position.Snapshot.Name != tc.wantName {
+				t.Fatalf("landed snapshot name = %q, want %q", move.Position.Snapshot.Name, tc.wantName)
+			}
+			ref, err := tree.Reference(webID("/clickable"))
+			if err != nil {
+				t.Fatal(err)
+			}
+			captured, err := tree.Resolve(ref)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if captured.Name != tc.rootName {
+				t.Fatalf("captured snapshot mutated: name = %q, want %q", captured.Name, tc.rootName)
+			}
+		})
 	}
 }