diff --git a/docs/testing/atspi-lifecycle.md b/docs/testing/atspi-lifecycle.md new file mode 100644 index 0000000..6f7031d --- /dev/null +++ b/docs/testing/atspi-lifecycle.md @@ -0,0 +1,57 @@ +# AT-SPI lifecycle and notebook flat review + +Adapter shutdown removes event subscriptions, the foreground timer and pending +capture sources on the GLib owner thread before releasing callback data and its +Go handle. Incoming owned event copies are freed after the Go callback returns. +Only one adapter may own the process-global AT-SPI loop at a time. +Sequential adapters reuse the library connection: upstream documents +[`atspi_exit()`](https://gnome.pages.gitlab.gnome.org/at-spi2-core/libatspi/func.exit.html) +as a process-exit operation. Process termination releases that connection. + +GTK notebook focus is resolved to the selected tab. This lets ordinary duplicate +focus suppression handle the notebook/tab focus replay after an X11 reader-key +grab, without suppressing real tab changes or returning from a page control. + +Run the lifecycle regressions on a disposable display and accessibility bus: + +```sh +python3 tests/native_test_session.py env CTHULHU_NATIVE_LIFECYCLE=1 go test -race ./internal/atspi -run '^TestNative(SequentialAdapterLifecycle|AdapterPendingCaptureShutdown|CanceledAdapterStart|AdapterStopRetry)$' -count=1 -v +``` + +These cover a live GTK provider across adapter replacement and restart, concurrent +adapter rejection, canceled startup, a canceled capture left pending at shutdown, +and a Stop retry after its caller times out. The original sequential test crashed +with `runtime/cgo: misuse of an invalid Handle` before the fix. + +AddressSanitizer also runs these cases with `-asan` in place of `-race`. +Pending-capture cleanup, canceled startup and Stop retries pass with leak +checking enabled. The live GTK sequential case still reports retained accessible +allocations (180 bytes in four allocations in one run; 360 bytes in eight +allocations after three repeated fixture runs), with no invalid memory-access +report. Their ownership is unresolved; this is not a clean leak check. The +earlier incoming-event-copy leaks were fixed in the callback wrapper. To +reproduce the remaining report, use the lifecycle command above with `-asan`, +`-run '^TestNativeSequentialAdapterLifecycle$'` and `-count=3`; keep leak +detection enabled. + +Build a separate binary and run the full reader/preferences regression: + +```sh +go build -o /tmp/cthulhu-lifecycle-review ./cmd/cthulhu +CTHULHU_BINARY=/tmp/cthulhu-lifecycle-review CTHULHU_PREFS_FLAT_REVIEW_ONLY=1 /usr/bin/python3 tests/native_preferences.py +``` + +This uses private Xvfb, D-Bus and recorded SSIP speech. It checks line, word and +character review on the General tab, absence of following tab announcements, +real tab changes, Tab into the page and Shift+Tab back to the tab strip. Evidence +is retained in the printed temporary directory. It does not replace listening +or physical keyboard/Braille checks. + +For live acceptance with a user-started updated build, focus the General tab +header and review several lines, words and characters in both directions. Each +review response should finish without “page tab list / General / page tab”. +Use Left/Right to change tabs, Tab into a page and Shift+Tab back: those genuine +focus changes should still speak. Check another application's notebook and a +browser tab strip, plus ordinary menus and single/multiple table selections, +because they share focus-target selection. Confirm Braille review stays at the +review position until genuine navigation changes focus. diff --git a/internal/atspi/adapter.go b/internal/atspi/adapter.go index 1ce8916..01a1274 100644 --- a/internal/atspi/adapter.go +++ b/internal/atspi/adapter.go @@ -144,6 +144,8 @@ type Adapter struct { running atomic.Bool ready atomic.Bool loopDone chan struct{} + loopError error // published by closing loopDone + stopRequested bool // stopMutex; keep Stop retries from queuing stale quit sources stopMutex sync.Mutex treeMutex sync.Mutex tree *AccessibleTree diff --git a/internal/atspi/adapter_event_callback_native.go b/internal/atspi/adapter_event_callback_native.go index 09c347e..a0265f1 100644 --- a/internal/atspi/adapter_event_callback_native.go +++ b/internal/atspi/adapter_event_callback_native.go @@ -53,7 +53,10 @@ func selectNativeEventTarget(event *C.AtspiEvent, eventType string) (*C.AtspiAcc // selection is unique. Multi-selection changes use their own events. targetRole := C.atspi_accessible_get_role(target, nil) listTable := targetRole == C.ATSPI_ROLE_LIST && C.cthulhu_is_gtk_list_table(target) != 0 - if targetRole == C.ATSPI_ROLE_MENU || targetRole == C.ATSPI_ROLE_TABLE || listTable { + // GTK notebooks report container focus before the active tab when an + // X11 key grab ends. Resolve that container to its selected tab, as + // with menus, so ordinary duplicate-focus handling preserves review. + if targetRole == C.ATSPI_ROLE_MENU || targetRole == C.ATSPI_ROLE_PAGE_TAB_LIST || targetRole == C.ATSPI_ROLE_TABLE || listTable { var child *C.AtspiAccessible if targetRole == C.ATSPI_ROLE_TABLE || listTable { child = C.cthulhu_table_selected_child(target) diff --git a/internal/atspi/adapter_events.c b/internal/atspi/adapter_events.c index 73b0db2..099210a 100644 --- a/internal/atspi/adapter_events.c +++ b/internal/atspi/adapter_events.c @@ -605,6 +605,9 @@ AtspiAccessible *cthulhu_document_ancestor(AtspiAccessible *source) { static void cthulhu_event_callback(AtspiEvent *event, void *userData) { cthulhuAtspiEventCallback(event, userData); + // libatspi passes each listener an owned boxed copy. Go retains only + // copied metadata and explicitly referenced accessibles. + if (event != NULL) g_boxed_free(ATSPI_TYPE_EVENT, event); } AtspiEventListenerCB cthulhu_event_callback_pointer(void) { @@ -629,20 +632,42 @@ gboolean cthulhu_request_event_loop_quit(void) { return source_id != 0; } +typedef struct { + cthulhu_callback_data *owner; + GSource *source; +} cthulhu_pending_capture; + static gboolean cthulhu_capture_tree_callback(gpointer userData) { - cthulhuAtspiTreeCaptureCallback(userData); + cthulhu_pending_capture *capture = userData; + cthulhuAtspiTreeCaptureCallback(capture->owner); return G_SOURCE_REMOVE; } +static void cthulhu_capture_destroy(gpointer userData) { + cthulhu_pending_capture *capture = userData; + g_mutex_lock(&capture->owner->mutex); + capture->owner->captures = g_list_remove(capture->owner->captures, capture); + g_mutex_unlock(&capture->owner->mutex); + g_source_unref(capture->source); + g_free(capture); +} + gboolean cthulhu_request_tree_capture(gpointer userData) { + cthulhu_callback_data *data = userData; GSource *source = g_idle_source_new(); if (source == NULL) { return FALSE; } + cthulhu_pending_capture *capture = g_new0(cthulhu_pending_capture, 1); + capture->owner = data; + capture->source = source; g_source_set_priority(source, G_PRIORITY_HIGH); - g_source_set_callback(source, cthulhu_capture_tree_callback, userData, NULL); + g_source_set_callback(source, cthulhu_capture_tree_callback, capture, cthulhu_capture_destroy); + g_mutex_lock(&data->mutex); + data->captures = g_list_prepend(data->captures, capture); guint source_id = g_source_attach(source, g_main_context_default()); - g_source_unref(source); + g_mutex_unlock(&data->mutex); + if (source_id == 0) g_source_destroy(source); return source_id != 0; } @@ -652,6 +677,7 @@ cthulhu_callback_data *cthulhu_callback_data_new(uintptr_t callback) { cthulhu_callback_data *data = g_new0(cthulhu_callback_data, 1); if (data != NULL) { data->callback = callback; + g_mutex_init(&data->mutex); } return data; } @@ -661,5 +687,13 @@ uintptr_t cthulhu_callback_value(const cthulhu_callback_data *data) { } void cthulhu_callback_data_free(cthulhu_callback_data *data) { + if (data == NULL) return; + // Dispatch has stopped and the Go request mutex excludes new sources. + // Destroy notifications remove each source and release its retained ref. + while (data->captures != NULL) { + cthulhu_pending_capture *capture = data->captures->data; + g_source_destroy(capture->source); + } + g_mutex_clear(&data->mutex); g_free(data); } diff --git a/internal/atspi/adapter_lifecycle_native.go b/internal/atspi/adapter_lifecycle_native.go index d69a647..3b4ae1f 100644 --- a/internal/atspi/adapter_lifecycle_native.go +++ b/internal/atspi/adapter_lifecycle_native.go @@ -17,20 +17,38 @@ import ( "git.stormux.org/storm/cthulhu/internal/platform/xorg" "runtime" "runtime/cgo" + "sync" "unsafe" ) +// libatspi and its default GLib loop are process globals. Adapters may take +// turns owning them, but must never run concurrently. atspi_exit is reserved +// for process exit: its caches do not support an exit/init cycle. +var nativeAdapterOwner sync.Mutex +var nativeAdapterInit sync.Once +var nativeAdapterInitError error + func (adapter *Adapter) Start(ctx context.Context) error { + adapter.stopMutex.Lock() + defer adapter.stopMutex.Unlock() if !adapter.running.CompareAndSwap(false, true) { return errors.New("AT-SPI adapter already started") } + if !nativeAdapterOwner.TryLock() { + adapter.running.Store(false) + return errors.New("another AT-SPI adapter is running") + } adapter.echoFocus.Store(nil) + adapter.loopError = nil + adapter.stopRequested = false adapter.loopDone = make(chan struct{}) setupDone := make(chan error, 1) go func() { runtime.LockOSThread() defer runtime.UnlockOSThread() + defer close(adapter.loopDone) + defer nativeAdapterOwner.Unlock() if err := adapter.initialize(); err != nil { setupDone <- err return @@ -51,6 +69,9 @@ func (adapter *Adapter) Start(ctx context.Context) error { if timer != 0 { C.g_source_remove(timer) } + // Deregistration can dispatch queued D-Bus events synchronously. Keep + // foreground state and retained accessibles alive until it finishes. + adapter.loopError = adapter.releaseNativeCallbacks() adapter.foregroundReader.Close() adapter.foregroundReader = nil for _, focus := range adapter.nativeFocus { @@ -67,17 +88,17 @@ func (adapter *Adapter) Start(ctx context.Context) error { adapter.clearNativeDocument() adapter.clearNativeReview() adapter.clearMouseApplication() - close(adapter.loopDone) }() select { case err := <-setupDone: if err != nil { + <-adapter.loopDone adapter.running.Store(false) return err } if err := ctx.Err(); err != nil { - C.atspi_event_quit() + C.cthulhu_request_event_loop_quit() <-adapter.loopDone adapter.running.Store(false) return err @@ -87,10 +108,11 @@ func (adapter *Adapter) Start(ctx context.Context) error { case <-ctx.Done(): err := <-setupDone if err != nil { + <-adapter.loopDone adapter.running.Store(false) return err } - C.atspi_event_quit() + C.cthulhu_request_event_loop_quit() <-adapter.loopDone adapter.running.Store(false) return ctx.Err() @@ -98,8 +120,13 @@ func (adapter *Adapter) Start(ctx context.Context) error { } func (adapter *Adapter) initialize() error { - if result := C.atspi_init(); result != 0 { - return fmt.Errorf("initialize AT-SPI: result %d", int(result)) + nativeAdapterInit.Do(func() { + if result := C.atspi_init(); result > 1 { + nativeAdapterInitError = fmt.Errorf("initialize AT-SPI: result %d", int(result)) + } + }) + if nativeAdapterInitError != nil { + return nativeAdapterInitError } // libatspi's ordinary default is 800 ms, but its startup grace period is // much longer. Keep both bounded so a newly connected or wedged provider @@ -110,7 +137,6 @@ func (adapter *Adapter) initialize() error { callbackData := C.cthulhu_callback_data_new(C.uintptr_t(callbackHandle)) if callbackData == nil { callbackHandle.Delete() - C.atspi_exit() return errors.New("allocate AT-SPI callback data") } adapter.callbackHandle = callbackHandle @@ -125,19 +151,14 @@ func (adapter *Adapter) initialize() error { adapter.callbackHandle = 0 adapter.callbackData = nil callbackHandle.Delete() - C.atspi_exit() return errors.New("allocate AT-SPI event listener") } for _, eventType := range eventTypes { if err := adapter.registerEvent(eventType); err != nil { - if cleanupErr := adapter.deregisterEvents(); cleanupErr != nil { + if cleanupErr := adapter.releaseNativeCallbacks(); cleanupErr != nil { err = errors.Join(err, cleanupErr) } - C.cthulhu_callback_data_free(callbackData) - adapter.callbackHandle = 0 - adapter.callbackData = nil - C.atspi_exit() return err } } @@ -180,6 +201,24 @@ func (adapter *Adapter) deregisterEvents() error { return stopErr } +// Called only by the GLib owner, after dispatch has stopped and request callers +// have been excluded. Destroy pending idle captures before releasing their +// callback data/handle; deregistration prevents later adapters reaching us. +func (adapter *Adapter) releaseNativeCallbacks() error { + err := adapter.deregisterEvents() + C.cthulhu_callback_data_free(adapter.callbackData) + adapter.callbackData = nil + if adapter.callbackHandle != 0 { + adapter.callbackHandle.Delete() + adapter.callbackHandle = 0 + } + for len(adapter.treeRequests) != 0 { + request := <-adapter.treeRequests + request.result <- treeCaptureResult{err: errors.New("AT-SPI adapter stopped")} + } + return err +} + func (adapter *Adapter) Stop(ctx context.Context) error { adapter.stopMutex.Lock() defer adapter.stopMutex.Unlock() @@ -192,8 +231,12 @@ func (adapter *Adapter) Stop(ctx context.Context) error { // AT-SPI documents event-loop quit as an event-handler operation. Schedule // it on the default GLib context owned by atspi_event_main instead of calling // it synchronously from this arbitrary shutdown goroutine. - if C.cthulhu_request_event_loop_quit() == 0 { - stopErr = errors.New("request AT-SPI event loop quit") + if !adapter.stopRequested { + if C.cthulhu_request_event_loop_quit() == 0 { + stopErr = errors.New("request AT-SPI event loop quit") + } else { + adapter.stopRequested = true + } } select { @@ -201,13 +244,7 @@ func (adapter *Adapter) Stop(ctx context.Context) error { case <-ctx.Done(): return errors.Join(stopErr, fmt.Errorf("wait for AT-SPI event loop: %w", ctx.Err())) } - if result := C.atspi_exit(); result != 0 { - stopErr = errors.Join(stopErr, fmt.Errorf("exit AT-SPI: result %d", int(result))) - } - C.cthulhu_callback_data_free(adapter.callbackData) - adapter.callbackData = nil - adapter.callbackHandle.Delete() - adapter.callbackHandle = 0 + stopErr = errors.Join(stopErr, adapter.loopError) adapter.running.Store(false) adapter.invalidateTree() return stopErr diff --git a/internal/atspi/adapter_native.h b/internal/atspi/adapter_native.h index 991c969..14831a4 100644 --- a/internal/atspi/adapter_native.h +++ b/internal/atspi/adapter_native.h @@ -83,6 +83,8 @@ void cthulhu_forget_menu_checkability(AtspiAccessible *source); typedef struct { uintptr_t callback; + GMutex mutex; + GList *captures; } cthulhu_callback_data; gchar *cthulhu_table_context(AtspiAccessible *source, gint *rows, gint *columns, diff --git a/internal/atspi/adapter_tree_native.go b/internal/atspi/adapter_tree_native.go index 2a80520..d28ba28 100644 --- a/internal/atspi/adapter_tree_native.go +++ b/internal/atspi/adapter_tree_native.go @@ -220,7 +220,16 @@ func captureNativeTree(ctx context.Context, root *C.AtspiAccessible, reviewBusNa } } - cachedChildren := known && !cached.childrenInvalid + // Visual review cannot use descendants of hidden controls. Inactive + // notebook pages and closed GTK combo menus can expose thousands of + // children; traversing them can exhaust the entire capture deadline. + // Keep semantic document and desktop/object capture complete. + reviewOnly := !document && reviewBusName != "" + captureChildren := !reviewOnly || !geometry.VisibilityKnown || + (geometry.Visible && geometry.Showing) || snapshot.Role == "application" || snapshot.Role == "desktop frame" + // A visibility change can reveal children omitted by an earlier review + // capture, even when the provider emits no children-changed event. + cachedChildren := known && !cached.childrenInvalid && (!reviewOnly || !cached.geometryInvalid) for _, id := range cached.children { if cache.sources[id] == nil { cachedChildren = false @@ -229,7 +238,9 @@ func captureNativeTree(ctx context.Context, root *C.AtspiAccessible, reviewBusNa } var childCountError *C.GError childCount := len(cached.children) - if !cachedChildren { + if !captureChildren { + childCount = 0 + } else if !cachedChildren { // Snapshot refresh above already cleared dirty entries. Clearing // again discards the state and interface data just fetched. childCount = int(C.atspi_accessible_get_child_count(source, &childCountError)) diff --git a/internal/atspi/lifecycle_native_test.go b/internal/atspi/lifecycle_native_test.go new file mode 100644 index 0000000..0ad5c2d --- /dev/null +++ b/internal/atspi/lifecycle_native_test.go @@ -0,0 +1,216 @@ +// Copyright (c) 2026 Storm Dragon +// Authors: Storm Dragon, Angus Kola +// SPDX-License-Identifier: GPL-3.0-or-later + +package atspi + +import ( + "context" + "os" + "os/exec" + "runtime" + "testing" + "time" +) + +func TestNativeSequentialAdapterLifecycle(t *testing.T) { + if os.Getenv("CTHULHU_NATIVE_LIFECYCLE") != "1" { + t.Skip("requires private Xvfb/AT-SPI session") + } + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + fixture := exec.CommandContext(ctx, "/usr/bin/python3", "-c", ` +import gi +gi.require_version("Gtk", "3.0") +from gi.repository import Gtk +window = Gtk.Window(title="Adapter lifecycle probe") +box = Gtk.Box() +window.add(box) +for name in ("First", "Second"): + box.pack_start(Gtk.Button(label=name), True, True, 0) +window.show_all() +Gtk.main() +`) + fixture.Stderr = os.Stderr + if err := fixture.Start(); err != nil { + t.Fatal(err) + } + defer func() { + _ = fixture.Process.Kill() + _ = fixture.Wait() + }() + var stopped []*Adapter + first := New() + // Start a fresh adapter, then restart the original instance. Events must + // reach only the current listener even though the provider stays alive. + for cycle, adapter := range []*Adapter{first, New(), first, New()} { + if err := adapter.Start(ctx); err != nil { + t.Fatalf("cycle %d start: %v", cycle, err) + } + if cycle == 0 { + other := New() + if err := other.Start(ctx); err == nil { + t.Fatal("concurrent adapter acquired the global AT-SPI loop") + } + } + func() { + defer func() { + stopCtx, stop := context.WithTimeout(context.Background(), 3*time.Second) + defer stop() + if err := adapter.Stop(stopCtx); err != nil { + t.Errorf("cycle %d stop: %v", cycle, err) + } + }() + var window ObjectReference + for !window.Valid() && ctx.Err() == nil { + tree, err := adapter.CaptureDesktop(ctx) + if err != nil { + t.Fatal(err) + } + window = findNamedReference(t, tree, "Adapter lifecycle probe") + } + if !window.Valid() { + t.Fatal("fixture window missing") + } + for _, args := range [][]string{{"search", "--sync", "--onlyvisible", "--name", "^Adapter lifecycle probe$", "windowfocus"}, {"key", "Tab"}} { + if output, err := exec.CommandContext(ctx, "xdotool", args...).CombinedOutput(); err != nil { + t.Fatalf("xdotool: %v: %s", err, output) + } + } + for { + select { + case event := <-adapter.Events(): + if event.Type == FocusedStateEvent && event.Detail1 == 1 && + (event.Source.Name == "First" || event.Source.Name == "Second") { + return + } + case <-ctx.Done(): + t.Fatal("focus event missing after adapter restart") + } + } + }() + if adapter.callbackData != nil || adapter.callbackHandle != 0 || adapter.eventListener != nil { + t.Fatal("stopped adapter retained native callbacks") + } + for _, old := range stopped { + if old == adapter { + continue + } + select { + case event := <-old.Events(): + t.Fatalf("stopped adapter received %s", event.Type) + default: + } + } + for len(adapter.events) != 0 { + <-adapter.events + } + stopped = append(stopped, adapter) + } +} + +func TestNativeAdapterPendingCaptureShutdown(t *testing.T) { + if os.Getenv("CTHULHU_NATIVE_LIFECYCLE") != "1" { + t.Skip("requires private Xvfb/AT-SPI session") + } + // Initialize on the GLib owner without starting dispatch so a canceled + // request deterministically leaves its idle callback pending at shutdown. + func() { + nativeAdapterOwner.Lock() + defer nativeAdapterOwner.Unlock() + runtime.LockOSThread() + defer runtime.UnlockOSThread() + adapter := New() + if err := adapter.initialize(); err != nil { + t.Fatal(err) + } + adapter.ready.Store(true) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Millisecond) + defer cancel() + _, err := adapter.CaptureDesktop(ctx) + if err != context.DeadlineExceeded || len(adapter.treeRequests) != 1 { + t.Fatalf("pending capture = %d, %v", len(adapter.treeRequests), err) + } + adapter.ready.Store(false) + if err := adapter.releaseNativeCallbacks(); err != nil { + t.Fatal(err) + } + if len(adapter.treeRequests) != 0 || adapter.callbackHandle != 0 { + t.Fatal("shutdown retained pending capture") + } + }() + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + adapter := New() + if err := adapter.Start(ctx); err != nil { + t.Fatal(err) + } + defer func() { + if err := adapter.Stop(ctx); err != nil { + t.Error(err) + } + }() + if _, err := adapter.CaptureDesktop(ctx); err != nil { + t.Fatal(err) + } +} + +func TestNativeCanceledAdapterStart(t *testing.T) { + if os.Getenv("CTHULHU_NATIVE_LIFECYCLE") != "1" { + t.Skip("requires private Xvfb/AT-SPI session") + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + adapter := New() + if err := adapter.Start(ctx); err != context.Canceled { + t.Fatalf("canceled start = %v", err) + } + if adapter.running.Load() || adapter.ready.Load() || adapter.callbackData != nil || adapter.eventListener != nil { + t.Fatal("canceled startup retained native resources") + } + ctx, cancel = context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if err := adapter.Start(ctx); err != nil { + t.Fatal(err) + } + if err := adapter.Stop(ctx); err != nil { + t.Fatal(err) + } +} + +func TestNativeAdapterStopRetry(t *testing.T) { + if os.Getenv("CTHULHU_NATIVE_LIFECYCLE") != "1" { + t.Skip("requires private Xvfb/AT-SPI session") + } + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + old := New() + if err := old.Start(ctx); err != nil { + t.Fatal(err) + } + expired, expire := context.WithCancel(ctx) + expire() + // The quit request can finish after this caller times out. Retrying Stop + // must not attach another global quit callback that kills a later adapter. + _ = old.Stop(expired) + select { + case <-old.loopDone: + case <-ctx.Done(): + t.Fatal("timed-out shutdown did not finish") + } + adapter := New() + if err := adapter.Start(ctx); err != nil { + t.Fatal(err) + } + defer func() { + if err := adapter.Stop(ctx); err != nil { + t.Error(err) + } + }() + if err := old.Stop(ctx); err != nil { + t.Fatal(err) + } + if _, err := adapter.CaptureDesktop(ctx); err != nil { + t.Fatal(err) + } +} diff --git a/internal/atspi/preferences_native_test.go b/internal/atspi/preferences_native_test.go index 2585103..d6f554f 100644 --- a/internal/atspi/preferences_native_test.go +++ b/internal/atspi/preferences_native_test.go @@ -150,6 +150,59 @@ func TestNativePreferencesDropdownEscape(t *testing.T) { } window := xdotool("search", "--onlyvisible", "--name", "^Cthulhu Preferences$") xdotool("windowfocus", window) + capture_review := func(name, wantName string, focus ObjectID) { + t.Helper() + captureCtx, stop := context.WithTimeout(ctx, 2*time.Second) + defer stop() + started := time.Now() + tree, err := adapter.CaptureReviewWindow(captureCtx, focus) + if err != nil { + t.Fatalf("%s review capture after %s: %v", name, time.Since(started), err) + } + snapshot, err := tree.ReviewWindow(captureCtx, focus) + if err != nil { + t.Fatalf("%s review window: %v", name, err) + } + ranges, found := 0, false + for _, node := range snapshot.Nodes { + ranges += len(node.Text.Ranges) + if node.Geometry.Visible && node.Geometry.Showing && node.Geometry.BoundsKnown && node.Snapshot.Name == wantName { + found = true + } + if node.Geometry.VisibilityKnown && (!node.Geometry.Visible || !node.Geometry.Showing) && len(node.Children) != 0 { + t.Fatalf("%s review traversed hidden %s %q", name, node.Snapshot.Role, node.Snapshot.Name) + } + } + if !found { + t.Fatalf("%s review is missing visible %q", name, wantName) + } + t.Logf("%s review: %d nodes, %d text ranges in %s", name, len(snapshot.Nodes), ranges, time.Since(started)) + } + // Match the reported workflow: focus the General tab header itself. + xdotool("key", "ctrl+Page_Down", "ctrl+Page_Up") + general := wait_event(5*time.Second, focused("General")) + capture_review("General", "Desktop", general.Source.ID()) + xdotool("key", "ctrl+Page_Down", "ctrl+Page_Down") + speech := wait_event(5*time.Second, focused("Speech")) + capture_review("Speech", "Enable speech", speech.Source.ID()) + xdotool("key", "Tab", "Tab") + wait_event(5*time.Second, focused("Speech system")) + xdotool("key", "alt+Down") + popup := wait_event(5*time.Second, focused("Speech Dispatcher")) + capture_review("open dropdown", "Speech Dispatcher", popup.Source.ID()) + xdotool("key", "Escape") + combo := wait_event(1500*time.Millisecond, focused("Speech system")) + capture_review("closed dropdown", "Speech system", combo.Source.ID()) + xdotool("key", "alt+Down") + popup = wait_event(5*time.Second, focused("Speech Dispatcher")) + capture_review("reopened dropdown", "Speech Dispatcher", popup.Source.ID()) + xdotool("key", "Escape") + wait_event(1500*time.Millisecond, focused("Speech system")) + xdotool("key", "ctrl+Page_Up", "ctrl+Page_Up") + general = wait_event(5*time.Second, focused("Desktop")) + capture_review("returned General", "Desktop", general.Source.ID()) + xdotool("key", "shift+Tab") + wait_event(5*time.Second, focused("General")) xdotool("key", "--clearmodifiers", "ctrl+Page_Down", "ctrl+Page_Down") wait_event(5*time.Second, focused("Speech")) xdotool("key", "Tab", "Tab") diff --git a/tests/native_echo_preview.py b/tests/native_echo_preview.py index e5800a8..df207db 100644 --- a/tests/native_echo_preview.py +++ b/tests/native_echo_preview.py @@ -140,7 +140,52 @@ def check_radio_buttons(controls, tabs, server, readerLog): assert not errors, errors -def check_runtime(radioOnly=False): +def check_flat_review(tabs, server, readerLog): + def records(): + return [json.loads(line) for line in readerLog.read_text().splitlines() if line.startswith("{")] + + def key(*keys): + subprocess.run(["xdotool", "key", "--clearmodifiers", *keys], check=True) + + assert tabs.querySelection().selectChild(0) + key("ctrl+Page_Down", "ctrl+Page_Up") + wait_for(lambda: any(record["text"] == "General" for record in server.snapshot()) and + server.snapshot()[-1]["finished"], "General tab was not announced") + time.sleep(0.4) + for command in ("KP_Up", "KP_Page_Up", "KP_Right", "KP_Page_Down", "KP_End", "KP_Home"): + logOffset = len(records()) + speechOffset = len(server.snapshot()) + key(command) + def review_delivered(): + return any(record.get("message") == "delivering accessible presentation" and + record.get("cause") == "flat-review" for record in records()[logOffset:]) + wait_for(review_delivered, "flat review did not speak: " + command) + wait_for(lambda: len(server.snapshot()) > speechOffset and server.snapshot()[-1]["finished"], + "review speech did not finish: " + command) + # Include the focus replay after the key grab has been released. + time.sleep(0.4) + deliveries = [record for record in records()[logOffset:] + if record.get("message") == "delivering accessible presentation"] + assert deliveries and all(record.get("cause") == "flat-review" for record in deliveries), deliveries + assert not any(record["canceled"] for record in server.snapshot()[speechOffset:]) + assert tabs.querySelection().getSelectedChild(0).name == "General" + + speechOffset = len(server.snapshot()) + nextTab = tabs[1].name + key("Right") + wait_for(lambda: any(record["text"] == nextTab for record in server.snapshot()[speechOffset:]) and + server.snapshot()[-1]["finished"], "new tab focus was suppressed") + speechOffset = len(server.snapshot()) + key("Tab") + wait_for(lambda: len(server.snapshot()) > speechOffset and server.snapshot()[-1]["finished"], + "Tab into the page did not speak") + speechOffset = len(server.snapshot()) + key("shift+Tab") + wait_for(lambda: any(record["text"] == nextTab for record in server.snapshot()[speechOffset:]) and + server.snapshot()[-1]["finished"], "return to tab header was suppressed") + + +def check_runtime(radioOnly=False, flatReviewOnly=False): import pyatspi directory = Path(tempfile.mkdtemp(prefix="cthulhu-echo-runtime-")) binary = os.environ["CTHULHU_BINARY"] @@ -212,6 +257,10 @@ lang = "en" xid = subprocess.check_output(["xdotool", "search", "--onlyvisible", "--name", "^Cthulhu Preferences$"], text=True).splitlines()[0] subprocess.run(["xdotool", "windowactivate", "--sync", xid], check=True) tabs = next(node for node in walk(window) if node.getRole() == pyatspi.ROLE_PAGE_TAB_LIST) + if flatReviewOnly: + check_flat_review(tabs, server, directory / "reader.log") + print(f"PASS: notebook flat review has no repeated tab speech; tab changes and focus returns speak; evidence: {directory}") + return if radioOnly: check_radio_buttons(controls, tabs, server, directory / "reader.log") print(f"PASS: real GTK Desktop/Laptop arrows play only the final selected sound and retain labels; evidence: {directory}") diff --git a/tests/native_preferences.py b/tests/native_preferences.py index 7501267..f28663a 100644 --- a/tests/native_preferences.py +++ b/tests/native_preferences.py @@ -46,6 +46,10 @@ class ResponseReader: def inside(): + if os.environ.get("CTHULHU_PREFS_FLAT_REVIEW_ONLY") == "1": + from native_echo_preview import check_runtime + check_runtime(flatReviewOnly=True) + return if os.environ.get("CTHULHU_PREFS_RADIO_RUNTIME_ONLY") == "1": from native_echo_preview import check_runtime check_runtime(radioOnly=True)