From 4c9a267897738e55c7a702915e3fb5aeb4f16ce9 Mon Sep 17 00:00:00 2001 From: Storm Dragon Date: Fri, 6 Nov 2026 02:13:00 -0500 Subject: [PATCH] Dialogue fixes and tests. --- internal/reader/changes.go | 15 +-- internal/reader/diagnostics_test.go | 5 +- internal/reader/dialog_test.go | 148 ++++++++++++++++++++++++++++ internal/reader/tracking_test.go | 3 +- tests/native_dialogs.py | 112 +++++++++++++++++++++ 5 files changed, 274 insertions(+), 9 deletions(-) create mode 100644 internal/reader/dialog_test.go create mode 100644 tests/native_dialogs.py diff --git a/internal/reader/changes.go b/internal/reader/changes.go index 5012fb53..ddd23f10 100644 --- a/internal/reader/changes.go +++ b/internal/reader/changes.go @@ -26,12 +26,7 @@ import ( "git.stormux.org/storm/fenrir/internal/screen" ) -func changes(previous, next screen.Snapshot, cursorEcho bool) []presentation.Message { - // TUI navigation can move over unchanged text while a status line also - // changes. That status delta must not hide the newly focused row (mutt). - if cursorEcho && next.Y != previous.Y && strings.TrimSpace(next.Text(next.Y)) != "" && slices.Equal(previous.Line(next.Y), next.Line(next.Y)) { - return []presentation.Message{presentation.Line_message(next, next.Y)} - } +func changes(previous, next screen.Snapshot, cursorEcho bool) (output []presentation.Message) { if slices.Equal(previous.Cells, next.Cells) { if cursorEcho && next.Y != previous.Y { return []presentation.Message{presentation.Line_message(next, next.Y)} @@ -41,6 +36,14 @@ func changes(previous, next screen.Snapshot, cursorEcho bool) []presentation.Mes } return nil } + // Keep navigation over unchanged text audible even when other rows change + // (mutt), but first read those changes. A dialog can finish drawing its + // body while moving the cursor onto an already drawn button row. + if cursorEcho && next.Y != previous.Y && strings.TrimSpace(next.Text(next.Y)) != "" && slices.Equal(previous.Line(next.Y), next.Line(next.Y)) { + defer func() { + output = append(output, presentation.Line_message(next, next.Y)) + }() + } // Match a scroll as one shift so old lines aren't read a second time. // Find the first non-space cell once. Successively shorter overlaps // cannot be meaningful scroll matches once they contain only whitespace. diff --git a/internal/reader/diagnostics_test.go b/internal/reader/diagnostics_test.go index 2f4ac2bd..15d22010 100644 --- a/internal/reader/diagnostics_test.go +++ b/internal/reader/diagnostics_test.go @@ -70,12 +70,13 @@ func TestReaderDiagnosticsCursorAndStatusWithoutText(t *testing.T) { next := fixture("PRIVATE first", "PRIVATE second", "2/2 PRIVATE") next.Y = 1 r.Note_input(true, false, false) - if got := r.Update(next); !reflect.DeepEqual(got, []presentation.Message{{Text: "PRIVATE second"}}) { + want := []presentation.Message{presentation.Character_message('2'), {Text: "PRIVATE second"}} + if got := r.Update(next); !reflect.DeepEqual(got, want) { t.Fatal("diagnostics changed cursor feedback", got) } check_update_record(t, log, map[string]string{ "cursor": "[0,1]", "previous_cursor": "[0,0]", "mode": `"cursor"`, - "text_rows": "[2]", "text_cells": "1", "messages": "1", "message_lengths": "[14]", + "text_rows": "[2]", "text_cells": "1", "messages": "2", "message_lengths": "[1,14]", "path": `"screen-changes"`, "review": "false", "cursor_echo": "true", }) log.Reset() diff --git a/internal/reader/dialog_test.go b/internal/reader/dialog_test.go new file mode 100644 index 00000000..edaa2ff5 --- /dev/null +++ b/internal/reader/dialog_test.go @@ -0,0 +1,148 @@ +// Fenrir console screen reader. +// Copyright (c) 2026 Storm Dragon +// SPDX-License-Identifier: GPL-3.0-or-later + +package reader + +import ( + "reflect" + "testing" + + "git.stormux.org/storm/fenrir/internal/command" + "git.stormux.org/storm/fenrir/internal/presentation" + "git.stormux.org/storm/fenrir/internal/screen" +) + +func dialog_snapshot(lines map[int]string, x, y int) screen.Snapshot { + snapshot := screen.Snapshot{Terminal: 1, Active: true, Rows: 25, Cols: 80, X: x, Y: y} + snapshot.Cells = make([]rune, snapshot.Rows*snapshot.Cols) + for index := range snapshot.Cells { + snapshot.Cells[index] = ' ' + } + for row, text := range lines { + copy(snapshot.Line(row), []rune(text)) + } + return snapshot +} + +func TestDialogBodyWithUnchangedCursorRow(t *testing.T) { + // These are the two split redraws captured during the I38 audit: the + // button row is already present before the dialog body finishes drawing. + for _, testCase := range []struct { + name string + before, after screen.Snapshot + want []presentation.Message + }{ + { + name: "saved preferences body", + before: dialog_snapshot(map[int]string{ + 8: "I38", 14: "< OK >", + }, 0, 19), + after: dialog_snapshot(map[int]string{ + 8: "I38", 9: "Loaded saved preferences from", 10: "/tmp/I38_preferences.conf", + 12: "Missing or invalid values will be prompted.", 14: "< OK >", + }, 2, 14), + want: []presentation.Message{ + {Text: "Loaded saved preferences from"}, {Text: "/tmp/I38_preferences.conf"}, + {Text: "Missing or invalid values will be prompted."}, {Text: "< OK >"}, + }, + }, + { + name: "last warning line", + before: dialog_snapshot(map[int]string{ + 0: "I38", 1: "I38Lock is not available with the current screen reader setup.", + 2: "Requires i38lock and either Cthulhu or Orca 50 or newer.", 22: "< OK >", + }, 2, 3), + after: dialog_snapshot(map[int]string{ + 0: "I38", 1: "I38Lock is not available with the current screen reader setup.", + 2: "Requires i38lock and either Cthulhu or Orca 50 or newer.", + 3: "Please choose another screen lock method.", 22: "< OK >", + }, 2, 22), + want: []presentation.Message{ + {Text: "Please choose another screen lock method."}, {Text: "< OK >"}, + }, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + r := New() + r.Update(testCase.before) + if got := r.Update(testCase.after); !reflect.DeepEqual(got, testCase.want) { + t.Fatalf("got %v, want %v", got, testCase.want) + } + if got := r.Update(testCase.after); len(got) != 0 { + t.Fatal("unchanged dialog repeated speech", got) + } + }) + } +} + +func TestCursorFeedbackPreservesConcurrentOutput(t *testing.T) { + for _, testCase := range []struct { + name string + before, after screen.Snapshot + want []presentation.Message + }{ + { + "output below focus", fixture("first", "focus", ""), fixture("first", "focus", "new output"), + []presentation.Message{{Text: "new output"}, {Text: "focus"}}, + }, + { + "scroll with fixed focus", fixture("old", "keep one", "keep two", "last", "focus"), + fixture("keep one", "keep two", "last", "new output", "focus"), + []presentation.Message{{Text: "new output"}, {Text: "focus"}}, + }, + { + "changed focused row", fixture("first", "old focus"), fixture("first", "new focus"), + []presentation.Message{{Text: "new"}}, + }, + { + "cleared body", fixture("old body", "focus"), fixture("", "focus"), + []presentation.Message{{Text: "focus"}}, + }, + } { + t.Run(testCase.name, func(t *testing.T) { + testCase.after.Y = 1 + if testCase.name == "scroll with fixed focus" { + testCase.after.Y = 4 + } + if got := changes(testCase.before, testCase.after, true); !reflect.DeepEqual(got, testCase.want) { + t.Fatalf("got %v, want %v", got, testCase.want) + } + }) + } +} + +func TestDialogRedrawRespectsReadingModes(t *testing.T) { + for _, mode := range []string{"cursor", "cursor off", "table", "highlight", "review", "automatic off"} { + t.Run(mode, func(t *testing.T) { + r := New() + before := fixture("I38", "", "< OK >") + r.Update(before) + switch mode { + case "cursor off": + r.Configure(true, false) + case "table", "highlight": + r.Configure_tracking(mode) + case "review": + r.Command(command.CurrentLine) + case "automatic off": + r.Configure(false, true) + } + after := fixture("I38", "New dialog body", "< OK >") + after.Y = 2 + var want []presentation.Message + if mode != "automatic off" { + want = []presentation.Message{{Text: "New dialog body"}} + if mode == "cursor" { + want = append(want, presentation.Message{Text: "< OK >"}) + } + } + if got := r.Update(after); !reflect.DeepEqual(got, want) { + t.Fatalf("got %v, want %v", got, want) + } + if mode == "review" && (!r.review.Active() || r.Command(command.CurrentLine).Text != "I38") { + t.Fatal("passive dialog redraw displaced review") + } + }) + } +} diff --git a/internal/reader/tracking_test.go b/internal/reader/tracking_test.go index c7935eb1..a51fb563 100644 --- a/internal/reader/tracking_test.go +++ b/internal/reader/tracking_test.go @@ -37,7 +37,8 @@ func TestCursorNavigationWithStatusChanges(t *testing.T) { after := fixture("Message one", "Message two", "2/2 messages") after.Y = 1 r.Note_input(true, false, false) - if got := r.Update(after); !reflect.DeepEqual(got, []presentation.Message{{Text: "Message two"}}) { + want := []presentation.Message{presentation.Character_message('2'), {Text: "Message two"}} + if got := r.Update(after); !reflect.DeepEqual(got, want) { t.Fatal("status line hid focused message", got) } if r.review.Active() { diff --git a/tests/native_dialogs.py b/tests/native_dialogs.py new file mode 100644 index 00000000..99e674e8 --- /dev/null +++ b/tests/native_dialogs.py @@ -0,0 +1,112 @@ +#!/usr/bin/env python3 +# Copyright (c) 2026 Storm Dragon +# SPDX-License-Identifier: GPL-3.0-or-later +"""Replay split dialog redraws through a real PTY and capture automatic SSIP. + +Run: xvfb-run -a env FENRIR_PRIVATE_X11=1 python3 tests/native_dialogs.py +No installed I38, dialog, GNU Screen, live audio, or live keyboard is used. +""" + +import json +import os +from pathlib import Path +import signal +import subprocess +import sys +import tempfile + +from native_terminal import KeyboardFixture, SpeechServer, wait_for + + +def main(): + if not os.environ.get("DISPLAY") or os.environ.get("FENRIR_PRIVATE_X11") != "1": + raise SystemExit("Use xvfb-run -a env FENRIR_PRIVATE_X11=1 python3 tests/native_dialogs.py") + repository = Path(__file__).resolve().parents[1] + fixture = str(repository / "tests/native_terminal.py") + binary = os.environ.get("FENRIR_TEST_BINARY", str(repository / "fenrir")) + with tempfile.TemporaryDirectory(prefix="fenrir-dialogs-", dir="/tmp") as directory: + root = Path(directory) + (root / "home").mkdir() + (root / "keyboard").mkdir() + (root / "keyboard/custom.toml").write_bytes((repository / "config/keyboard/desktop.toml").read_bytes()) + (root / "settings.toml").write_text( + 'version=3\n[presentation.sound]\nenabled=false\n' + '[presentation.progress]\nenabled=false\n' + '[reading]\npunctuation_level="none"\nspellcheck=false\n' + f'[remote]\nsocket_file="{root / "remote.sock"}"\n') + shell = root / "shell" + shell.write_text(f'#!{sys.executable}\nimport runpy, sys\nsys.argv=[{fixture!r}, "--fixture-shell"]\nrunpy.run_path({fixture!r}, run_name="__main__")\n') + shell.chmod(0o700) + server = SpeechServer(root / "speech.sock") + keyboard = KeyboardFixture() + environment = dict(os.environ, FENRIR_TEST_BINARY=binary, FENRIR_FIXTURE_DIR=str(root), + HOME=str(root / "home"), TMPDIR=str(root), LOGNAME="Username", USER="Username", + SPEECHD_ADDRESS="unix_socket:" + str(root / "speech.sock"), LC_ALL="C", LANGUAGE="C") + terminal = subprocess.Popen(["xterm", "-geometry", "80x25", "-e", sys.executable, + fixture, "--outer", str(root)], env=environment, cwd=root) + + def spoken(text, start): + wait_for(lambda: any(text in item for item in server.events[start:]), + f"automatic speech {text!r}; recent={server.events[-12:]}") + + def emit_output(data): + start = len(server.events) + (root / "prompt-output").write_bytes(data) + os.kill(int((root / "shell-pid").read_text()), signal.SIGUSR1) + return start + + try: + wait_for(lambda: (root / "started").exists(), "reader startup") + state = json.loads((root / "started").read_text()) + subprocess.run(["xdotool", "windowfocus", "--sync", state["window"]], check=True) + spoken("prompt", 0) + for name, initial, redraw, expected in ( + ("saved preferences body", + b"\x1b[2J\x1b[9;1HI38\x1b[15;1H< OK >\x1b[20;1H", + b"\x1b[10;1HLoaded saved preferences\x1b[11;1HPreferences file loaded" + b"\x1b[13;1HMissing or invalid values will be prompted\x1b[15;3H", + ["Loaded saved preferences", "Preferences file loaded", "Missing or invalid values will be prompted"]), + ("last warning line", + b"\x1b[2J\x1b[1;1HI38\x1b[2;1HI38Lock is not available" + b"\x1b[3;1HRequires i38lock and a supported reader\x1b[23;1H< OK >\x1b[4;3H", + b"\x1b[4;1HPlease choose another screen lock method\x1b[23;3H", + ["Please choose another screen lock method"]), + ): + start = emit_output(initial) + spoken("OK", start) # The button is captured before the body. + start = emit_output(redraw) + for text in expected: + spoken(text, start) + spoken("OK", start) + utterances = [item for item in server.events[start:] if item != "CANCEL"] + joined = " ".join(utterances) + for text in expected: + assert joined.count(text) == 1, (name, text, utterances) + assert "OK" in utterances[-1], (name, utterances) + # No review key was used to obtain the missing text. + assert not (root / "input").exists() or not (root / "input").read_bytes() + print(f"PASS: {name}: automatic body speech followed by focused button", flush=True) + keyboard.key("Insert+q") + wait_for(lambda: (root / "finished").exists(), "reader shutdown") + result = json.loads((root / "finished").read_text()) + assert result == {"status": 0, "restored": True, "flagsRestored": True}, result + assert not server.errors, server.errors + except BaseException: + print("SSIP:", server.events[-25:], file=sys.stderr) + for name in ("reader-errors", "timing.log"): + if (root / name).exists(): + print(name + ":", (root / name).read_text()[-5000:], file=sys.stderr) + raise + finally: + terminal.terminate() + try: + terminal.wait(timeout=3) + except subprocess.TimeoutExpired: + terminal.kill() + terminal.wait() + keyboard.close() + server.close() + + +if __name__ == "__main__": + main()