diff --git a/docs/testing/flat-review-tables.md b/docs/testing/flat-review-tables.md new file mode 100644 index 0000000..b47c589 --- /dev/null +++ b/docs/testing/flat-review-tables.md @@ -0,0 +1,101 @@ +# Flat review in scrolling tables + +Flat review captures the visible cells of selectable tables through AT-SPI +table coordinates and component hit testing. It does not enumerate the model's +offscreen rows. Visible column headers and compound cells remain available. +The first selected cell is retained as a focus anchor if horizontal scrolling +hides its column; its offscreen text does not enter review output. + +New flat-review objects retain libatspi's event-maintained provider cache. +Clearing that cache forced redundant child-count requests even for known leaves, +which could abort capture when a provider was briefly unresponsive. Objects +already marked dirty in the owned tree still refresh their native snapshot. + +Long text geometry queries are bounded to 128 characters per provider call. +Their rectangles are combined across every chunk, including interior bidi runs. +For partially clipped lines, offscreen chunks are rejected before querying +individual character rectangles. This preserves native offsets and visible text +without measuring thousands of characters in one synchronous request. + +In the live FastSMRW investigation, measuring an entire 3,424-character line took +about 0.9 seconds, exceeding the adapter's 0.8-second provider timeout. A later +child-count query then reported that the process appeared hung, masking the +earlier text-geometry timeout. The same read-only capture probe failed before +bounded measurement and completed after it. That is provider-capture evidence; +speech, keyboard response and physical Braille still require live acceptance. +Three subsequent read-only review-worker runs completed line, word, character, +window-start and window-end commands. Initial capture took roughly 4–6 seconds; +cached commands took about one millisecond. One earlier worker probe returned a +negative child count, which did not recur in those three runs. Negative-count +errors now include the same object metadata as provider-error responses so a +recurrence can be located. Initial-capture latency remains a live acceptance +concern. + +Debug logs include one `flat review capture timing` record for each capture +attempt, including a timed-out attempt while a native call is still running. +It reports request-lock and GLib queue wait, root selection, cache preparation, +snapshot, geometry, visible-text, table-viewport, child-count, child-enumeration, +installation and reference-cleanup timings. Each stage has a total, invocation +count, and longest invocation with its bus/path/role. These are helper-stage +timings: a visible-text invocation can contain several native range queries. +`last_stage`, `stage_duration` and the object identity describe the stage at +completion or timeout; on timeout its elapsed portion is included in totals. +`root_scope` distinguishes a focused-window root from the desktop fallback. +Captured/pending object counts and text byte counts show traversal progress. +No names or text contents are added. Diagnostics allocate no per-capture trace +unless debug logging is enabled, and do not issue additional provider queries. +Late native completion cannot overwrite the timeout record. + +A descendant which becomes defunct during capture is removed from its parent's +child list. A defunct root still fails capture. Other provider failures still +fail capture rather than presenting an old snapshot. Child-count errors include +the object's bus, path, role and the number of objects already captured, without +logging its name or text. + +Run the native viewport regression on a private display and accessibility bus: + +```sh +python3 tests/native_test_session.py env CTHULHU_NATIVE_REVIEW_TABLE=1 go test ./internal/atspi -run '^TestNativeFlatReviewLargeGTKTable$' -count=1 -timeout=150s -v +python3 tests/native_test_session.py env CTHULHU_NATIVE_REVIEW_CACHE=1 go test -race ./internal/atspi -run '^TestNativeFlatReviewPreservesProviderCache$' -count=1 -timeout=30s -v +python3 tests/native_test_session.py env CTHULHU_NATIVE_REVIEW_BOUNDS=1 go test -race ./internal/atspi -run '^TestNativeFlatReviewBoundsSlowLongRanges$' -count=1 -timeout=30s -v +python3 tests/native_atspi.py +``` + +The viewport test covers a 6,000-row GTK table scrolled to its middle, a changed +viewport and updated row text, a small table, an empty table with headers, +horizontal scrolling, and cells with multiple renderers. It checks that a small +viewport captures fewer than 100 objects. Before the change, the large case +captured 6,006 objects. The existing suite covers flat-review cursor movement, +text changes, terminal viewport capture, password exclusion, and ordinary +single/multiple GTK table selections. + +The cache regression uses a GTK label whose initial cache reports zero children, +then arms a deliberately slow child-count getter. Before cache preservation, +capture failed on that redundant query. The test requires capture to succeed +without invoking the getter, then changes the label and checks that dirty text +is refreshed. The C fixture is compiled with the existing GTK development +dependencies during the test. + +The long-range regression uses a horizontally clipped 3,424-character GTK label. +Its provider deliberately stalls when a range query exceeds 128 characters. +Capture must retain visible text and never trigger that large request. Unit +tests also cover Unicode offsets, offscreen query counts, interior bidi bounds, +zero-size chunks, cancellation and provider errors. + +For live acceptance, use a user-started updated reader and: + +1. In FastSMRW, review lines, words and characters in both directions, including + window start/end. Repeat after changing the selected post and after a timeline + update. Responses should complete without a long pause or a location error. +2. Scroll the table away from its original viewport. Review should contain the + currently visible rows. Check another GTK table, its column headers, and a + horizontally scrolled table. +3. Check ordinary Up/Down selection speech, menus, preferences notebooks and + terminal flat review. Semantic browser navigation and table speech should + continue to work because their captures retain the complete semantic tree. +4. Confirm audible output and physical Braille separately. Private native tests + do not establish these outcomes or prove that the live FastSMRW timeout is + resolved. Preserve fresh logs if a failure remains; the object metadata will + identify where capture stopped. Inspect `flat review capture timing` to + compare the failed initial capture with a successful retry, including root + scope, queue wait, stage totals and the slowest object in each stage. diff --git a/internal/app/app.go b/internal/app/app.go index 0131d36..e9aafe8 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -169,7 +169,11 @@ func Run(ctx context.Context, options Options) error { settings = *options.Settings } if options.newAdapter == nil { - options.newAdapter = func() Adapter { return atspi.New() } + options.newAdapter = func() Adapter { + adapter := atspi.New() + adapter.SetCaptureLogger(logger) + return adapter + } } if options.newCommands == nil { diff --git a/internal/atspi/adapter.go b/internal/atspi/adapter.go index 1cef324..24061e7 100644 --- a/internal/atspi/adapter.go +++ b/internal/atspi/adapter.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "git.stormux.org/storm/cthulhu/internal/platform/xorg" + "log/slog" "runtime/cgo" "sync" "sync/atomic" @@ -110,6 +111,7 @@ var eventTypes = []string{ } type Adapter struct { + captureLogger atomic.Pointer[slog.Logger] libreOfficeHeaders libreOfficeHeaderStore // copied choices shared with the event loop applicationProcesses map[string]string // GLib owner; bounded provider identity cache webNotificationRoots map[ObjectID]bool // copied identities only; no native references @@ -339,12 +341,23 @@ func (adapter *Adapter) CaptureDocumentText(ctx context.Context, focus ObjectID) return adapter.capture(ctx, focus, "document text", true, true) } -func (adapter *Adapter) capture(ctx context.Context, reviewFocus ObjectID, captureName string, document, documentText bool) (*AccessibleTree, error) { +// SetCaptureLogger enables capture diagnostics when the logger accepts debug +// records. It can be updated without touching the GLib thread or native state. +func (adapter *Adapter) SetCaptureLogger(logger *slog.Logger) { + adapter.captureLogger.Store(logger) +} + +func (adapter *Adapter) capture(ctx context.Context, reviewFocus ObjectID, captureName string, document, documentText bool) (tree *AccessibleTree, err error) { if ctx == nil { return nil, fmt.Errorf("capture AT-SPI %s: nil context", captureName) } ctx, cancel := adapter.secureRequestContext(ctx) defer cancel() + var trace *captureDiagnostics + if !document && reviewFocus.Valid() { + ctx, trace = new_capture_diagnostics(ctx, adapter.captureLogger.Load(), reviewFocus) + defer func() { trace.finish(err) }() + } if err := ctx.Err(); err != nil { return nil, err } @@ -365,6 +378,7 @@ func (adapter *Adapter) capture(ctx context.Context, reviewFocus ObjectID, captu documentText: documentText, result: make(chan treeCaptureResult, 1), } + trace.begin_stage("queue-wait", reviewFocus, "") select { case adapter.treeRequests <- request: case <-ctx.Done(): diff --git a/internal/atspi/adapter_references_native.go b/internal/atspi/adapter_references_native.go index a9fa585..9ea422e 100644 --- a/internal/atspi/adapter_references_native.go +++ b/internal/atspi/adapter_references_native.go @@ -85,6 +85,9 @@ func (adapter *Adapter) captureDesktop(ctx context.Context, reviewFocus ObjectID adapter.nativeDocument = retained return tree, nil } + trace := capture_diagnostics(ctx) + trace.begin_stage("root-selection", reviewFocus, "") + rootScope := "window" var rootSource *C.AtspiAccessible if reviewFocus.Valid() { // A flat-review command only needs its owning window. Starting from @@ -98,11 +101,14 @@ func (adapter *Adapter) captureDesktop(ctx context.Context, reviewFocus ObjectID } } if rootSource == nil { + rootScope = "desktop" rootSource = C.atspi_get_desktop(0) } if rootSource == nil { return nil, errors.New("capture AT-SPI review window: root is unavailable") } + trace.set_root(nativeObjectID(rootSource), rootScope) + trace.begin_stage("cache-preparation", nativeObjectID(rootSource), "") retained := make(map[ObjectID]*C.AtspiAccessible) cache := nativeTreeCache{} if reviewFocus.BusName != "" { @@ -123,6 +129,7 @@ func (adapter *Adapter) captureDesktop(ctx context.Context, reviewFocus ObjectID } return nil, err } + trace.begin_stage("tree-installation", root, "") tree, err := adapter.installCapturedTree(root, nodes, document, documentText) if err != nil { for _, source := range retained { @@ -130,9 +137,11 @@ func (adapter *Adapter) captureDesktop(ctx context.Context, reviewFocus ObjectID } return nil, err } + trace.begin_stage("reference-cleanup", root, "") adapter.clearNativeReview() adapter.nativeReview = retained adapter.nativeReviewBus = reviewFocus.BusName + trace.begin_stage("result-delivery", root, "") return tree, nil } diff --git a/internal/atspi/adapter_tree_native.go b/internal/atspi/adapter_tree_native.go index 914f080..d54baac 100644 --- a/internal/atspi/adapter_tree_native.go +++ b/internal/atspi/adapter_tree_native.go @@ -88,6 +88,9 @@ type nativeTreeCache struct { // focused application's subtree. The latter keeps flat review independent of // unrelated AT-SPI providers that can be slow or transiently unavailable. func captureNativeTree(ctx context.Context, root *C.AtspiAccessible, reviewBusName string, document, documentText, captureSound bool, retained map[ObjectID]*C.AtspiAccessible, cache nativeTreeCache) (ObjectID, []TreeNode, error) { + trace := capture_diagnostics(ctx) + trace.begin_stage("tree-setup", ObjectID{}, "") + reviewOnly := !document && reviewBusName != "" rootID := nativeObjectID(root) if !rootID.Valid() { C.g_object_unref(C.gpointer(root)) @@ -122,17 +125,21 @@ func captureNativeTree(ctx context.Context, root *C.AtspiAccessible, reviewBusNa item := pending[last] source := item.source pending = pending[:last] - cached, known := cache.nodes[nativeObjectID(source)] + sourceID := nativeObjectID(source) + cached, known := cache.nodes[sourceID] refresh := !known || cached.snapshotInvalid || cached.textInvalid || cached.geometryInvalid || cached.childrenInvalid snapshot := cached.snapshot + trace.begin_stage("snapshot", sourceID, snapshot.Role) if refresh { // Only changed/new objects require synchronous provider queries. // Reusing live siblings matches Python's event-driven web cache and // prevents a small insertion from repeatedly timing out on a big page. - // New document nodes already have libatspi's event-maintained cache. - // Clear only entries our tree marked dirty; flushing every new node - // forces redundant provider queries across large catalogs. - if !document || known { + // New document and flat-review nodes already have libatspi's + // event-maintained cache. Clearing it forces another child-count + // request even for a cached leaf, which can fail on a busy provider. + // Known dirty entries still need a fresh provider snapshot. Preserve + // the existing full refresh for desktop/object capture. + if known || (!document && !reviewOnly) { C.atspi_accessible_clear_cache_single(source) } applicationName, knownApplication := applicationNames[nativeObjectID(source).BusName] @@ -143,13 +150,14 @@ func captureNativeTree(ctx context.Context, root *C.AtspiAccessible, reviewBusNa snapshot = snapshotFromNativeWithCaret(source, !document, false, document, document && captureSound) } } + trace.progress(snapshot.Role, len(nodes), len(pending)+1, textBytes) if _, knownApplication := applicationNames[snapshot.BusName]; !knownApplication { applicationNames[snapshot.BusName] = snapshot.ApplicationName } if snapshot.Defunct { - if document && item.parentIndex >= 0 { - // Chromium can retire a descendant after exposing it in its - // parent's child list. Drop that edge and keep the live page. + if (document || reviewOnly) && item.parentIndex >= 0 { + // Providers can retire a descendant after exposing it in its + // parent's child list. Drop that edge and keep the live window. children := nodes[item.parentIndex].Children for index, child := range children { if child == snapshot.ID() { @@ -168,6 +176,7 @@ func captureNativeTree(ctx context.Context, root *C.AtspiAccessible, reviewBusNa retained[snapshot.ID()] = (*C.AtspiAccessible)(C.g_object_ref(C.gpointer(source))) } + trace.begin_stage("geometry", snapshot.ID(), snapshot.Role) geometry := cached.geometry var geometryErr error if refresh { @@ -196,6 +205,7 @@ func captureNativeTree(ctx context.Context, root *C.AtspiAccessible, reviewBusNa // can be the displayed page below it. Preserve the parent viewport. childClip, childClipped = item.clip, item.clipped } + trace.begin_stage("visible-text", snapshot.ID(), snapshot.Role) text := VisibleText{Limited: true} if (!document || documentText) && textBytes < maxTreeTextBytes { if known && !cached.textInvalid && !cached.geometryInvalid && !item.clipChanged { @@ -220,11 +230,11 @@ func captureNativeTree(ctx context.Context, root *C.AtspiAccessible, reviewBusNa } } + trace.begin_stage("table-viewport", snapshot.ID(), snapshot.Role) // 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 @@ -238,15 +248,36 @@ func captureNativeTree(ctx context.Context, root *C.AtspiAccessible, reviewBusNa } var childCountError *C.GError childCount := len(cached.children) + var tableChildren []*C.AtspiAccessible + tableViewport := false + if reviewOnly && captureChildren { + var tableErr error + tableChildren, tableViewport, tableErr = native_review_table_children(ctx, source, clip) + if tableErr != nil { + C.g_object_unref(C.gpointer(source)) + releasePending() + return ObjectID{}, nil, fmt.Errorf("%w (bus %q path %q role %q)", tableErr, snapshot.BusName, snapshot.Path, snapshot.Role) + } + // The pending stack takes its own reference to each admitted cell. + defer func(children []*C.AtspiAccessible) { + for _, child := range children { + C.g_object_unref(C.gpointer(child)) + } + }(tableChildren) + } + trace.begin_stage("child-count", snapshot.ID(), snapshot.Role) if !captureChildren { childCount = 0 + } else if tableViewport { + childCount = len(tableChildren) } 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)) } if childCountError != nil { - err := nativeError("query AT-SPI child count", childCountError) + err := fmt.Errorf("%w (bus %q path %q role %q captured_nodes %d)", + nativeError("query AT-SPI child count", childCountError), snapshot.BusName, snapshot.Path, snapshot.Role, len(nodes)) C.g_error_free(childCountError) C.g_object_unref(C.gpointer(source)) releasePending() @@ -255,7 +286,8 @@ func captureNativeTree(ctx context.Context, root *C.AtspiAccessible, reviewBusNa if childCount < 0 { C.g_object_unref(C.gpointer(source)) releasePending() - return ObjectID{}, nil, errors.New("query AT-SPI child count: negative result") + return ObjectID{}, nil, fmt.Errorf("query AT-SPI child count: negative result (bus %q path %q role %q captured_nodes %d)", + snapshot.BusName, snapshot.Path, snapshot.Role, len(nodes)) } if reviewBusName == "" && len(scheduled)+childCount > maxTreeNodes { C.g_object_unref(C.gpointer(source)) @@ -263,6 +295,7 @@ func captureNativeTree(ctx context.Context, root *C.AtspiAccessible, reviewBusNa return ObjectID{}, nil, fmt.Errorf("%w: native tree exceeds limit %d", ErrInvalidTree, maxTreeNodes) } + trace.begin_stage("child-enumeration", snapshot.ID(), snapshot.Role) children := make([]ObjectID, 0, childCount) for index := 0; index < childCount; index++ { if err := ctx.Err(); err != nil { @@ -272,7 +305,9 @@ func captureNativeTree(ctx context.Context, root *C.AtspiAccessible, reviewBusNa } var childError *C.GError var child *C.AtspiAccessible - if cachedChildren { + if tableViewport { + child = (*C.AtspiAccessible)(C.g_object_ref(C.gpointer(tableChildren[index]))) + } else if cachedChildren { child = (*C.AtspiAccessible)(C.g_object_ref(C.gpointer(cache.sources[cached.children[index]]))) } else { child = C.atspi_accessible_get_child_at_index(source, C.gint(index), &childError) @@ -328,6 +363,7 @@ func captureNativeTree(ctx context.Context, root *C.AtspiAccessible, reviewBusNa snapshot.TextChildren = nativeTextChildren(ctx, source, snapshot.CharacterCount, children) } nodes = append(nodes, TreeNode{Snapshot: snapshot, Children: children, Geometry: geometry, Text: text}) + trace.progress(snapshot.Role, len(nodes), len(pending), textBytes) C.g_object_unref(C.gpointer(source)) } return rootID, nodes, nil @@ -344,6 +380,7 @@ func cthulhuAtspiTreeCaptureCallback(userData unsafe.Pointer) { return } request := <-adapter.treeRequests + capture_diagnostics(request.ctx).begin_stage("callback-setup", request.reviewFocus, "") adapter.applySecureReset() if request.ctx.Err() != nil { request.result <- treeCaptureResult{err: request.ctx.Err()} diff --git a/internal/atspi/capture_diagnostics.go b/internal/atspi/capture_diagnostics.go new file mode 100644 index 0000000..e607bfc --- /dev/null +++ b/internal/atspi/capture_diagnostics.go @@ -0,0 +1,132 @@ +// Copyright (c) 2026 Storm Dragon +// SPDX-License-Identifier: GPL-3.0-or-later + +package atspi + +import ( + "context" + "log/slog" + "sync" + "time" +) + +type captureDiagnosticKey struct{} + +type captureStageTiming struct { + Stage string `json:"stage"` + Calls int `json:"calls"` + Duration time.Duration `json:"duration"` + MaxDuration time.Duration `json:"max_duration"` + SlowestBus string `json:"slowest_bus"` + SlowestPath string `json:"slowest_path"` + SlowestRole string `json:"slowest_role"` +} + +// One request owns these copied diagnostics. The caller can finish them while +// the GLib thread is still inside a native call; no native references cross +// threads. A finished trace ignores later updates from that call. +type captureDiagnostics struct { + mutex sync.Mutex + logger *slog.Logger + started time.Time + stageStarted time.Time + stage string + object ObjectID + role string + root ObjectID + rootScope string + capturedNodes int + pendingNodes int + textBytes int + timings []captureStageTiming + finished bool +} + +func new_capture_diagnostics(ctx context.Context, logger *slog.Logger, focus ObjectID) (context.Context, *captureDiagnostics) { + if logger == nil || !logger.Enabled(ctx, slog.LevelDebug) { + return ctx, nil + } + now := time.Now() + trace := &captureDiagnostics{logger: logger, started: now, stageStarted: now, stage: "request-lock", object: focus} + return context.WithValue(ctx, captureDiagnosticKey{}, trace), trace +} + +func capture_diagnostics(ctx context.Context) *captureDiagnostics { + trace, _ := ctx.Value(captureDiagnosticKey{}).(*captureDiagnostics) + return trace +} + +func (trace *captureDiagnostics) record_stage(now time.Time) { + duration := now.Sub(trace.stageStarted) + index := 0 + for index < len(trace.timings) && trace.timings[index].Stage != trace.stage { + index++ + } + if index == len(trace.timings) { + trace.timings = append(trace.timings, captureStageTiming{Stage: trace.stage}) + } + timing := &trace.timings[index] + timing.Calls++ + timing.Duration += duration + if duration > timing.MaxDuration { + timing.MaxDuration = duration + timing.SlowestBus, timing.SlowestPath, timing.SlowestRole = trace.object.BusName, trace.object.Path, trace.role + } +} + +func (trace *captureDiagnostics) begin_stage(stage string, object ObjectID, role string) { + if trace == nil { + return + } + trace.mutex.Lock() + defer trace.mutex.Unlock() + if trace.finished { + return + } + now := time.Now() + trace.record_stage(now) + trace.stage, trace.object, trace.role, trace.stageStarted = stage, object, role, now +} + +func (trace *captureDiagnostics) set_root(root ObjectID, scope string) { + if trace == nil { + return + } + trace.mutex.Lock() + defer trace.mutex.Unlock() + if !trace.finished { + trace.root, trace.rootScope = root, scope + } +} + +func (trace *captureDiagnostics) progress(role string, capturedNodes, pendingNodes, textBytes int) { + if trace == nil { + return + } + trace.mutex.Lock() + defer trace.mutex.Unlock() + if !trace.finished { + trace.role = role + trace.capturedNodes, trace.pendingNodes, trace.textBytes = capturedNodes, pendingNodes, textBytes + } +} + +func (trace *captureDiagnostics) finish(err error) { + if trace == nil { + return + } + trace.mutex.Lock() + defer trace.mutex.Unlock() + if trace.finished { + return + } + now := time.Now() + trace.record_stage(now) + trace.finished = true + trace.logger.Debug("flat review capture timing", "duration", now.Sub(trace.started), "error", err, + "last_stage", trace.stage, "stage_duration", now.Sub(trace.stageStarted), + "bus", trace.object.BusName, "object_path", trace.object.Path, "role", trace.role, + "root_bus", trace.root.BusName, "root_path", trace.root.Path, "root_scope", trace.rootScope, + "captured_nodes", trace.capturedNodes, "pending_nodes", trace.pendingNodes, "text_bytes", trace.textBytes, + "stages", trace.timings) +} diff --git a/internal/atspi/capture_diagnostics_test.go b/internal/atspi/capture_diagnostics_test.go new file mode 100644 index 0000000..7819939 --- /dev/null +++ b/internal/atspi/capture_diagnostics_test.go @@ -0,0 +1,90 @@ +// Copyright (c) 2026 Storm Dragon +// SPDX-License-Identifier: GPL-3.0-or-later + +package atspi + +import ( + "bytes" + "context" + "encoding/json" + "log/slog" + "testing" + "time" + + "git.stormux.org/storm/cthulhu/internal/logging" +) + +func TestCaptureDiagnosticsTimeoutDuringNativeCall(t *testing.T) { + var output bytes.Buffer + logger := logging.New(&output, logging.Options{Level: slog.LevelDebug}) + ctx, trace := new_capture_diagnostics(context.Background(), logger, NewObjectID(":1.7", "/focus")) + if capture_diagnostics(ctx) != trace { + t.Fatal("capture context lost its diagnostics") + } + root := NewObjectID(":1.7", "/window") + cell := NewObjectID(":1.7", "/cell") + trace.set_root(root, "window") + trace.begin_stage("queue-wait", root, "frame") + started, resume, done := make(chan struct{}), make(chan struct{}), make(chan struct{}) + go func() { + defer close(done) + trace.begin_stage("visible-text", cell, "table cell") + trace.progress("table cell", 12, 3, 250) + close(started) + <-resume // The native operation is still running when its caller times out. + trace.begin_stage("child-count", cell, "table cell") + trace.progress("table cell", 13, 2, 400) + trace.set_root(NewObjectID(":1.7", "/late-root"), "desktop") + trace.finish(nil) + }() + <-started + trace.finish(context.DeadlineExceeded) + close(resume) + <-done + var record struct { + Error string `json:"error"` + Duration time.Duration `json:"duration"` + LastStage string `json:"last_stage"` + ObjectPath string `json:"object_path"` + Role string `json:"role"` + RootPath string `json:"root_path"` + RootScope string `json:"root_scope"` + CapturedNodes int `json:"captured_nodes"` + PendingNodes int `json:"pending_nodes"` + TextBytes int `json:"text_bytes"` + Stages []captureStageTiming `json:"stages"` + } + if err := json.Unmarshal(output.Bytes(), &record); err != nil { + t.Fatalf("expected one diagnostic record: %v: %s", err, output.Bytes()) + } + if record.Error != context.DeadlineExceeded.Error() || record.LastStage != "visible-text" || record.ObjectPath != cell.Path || record.Role != "table cell" { + t.Fatalf("timeout lost the in-progress operation: %+v", record) + } + if record.RootPath != root.Path || record.RootScope != "window" || record.CapturedNodes != 12 || record.PendingNodes != 3 || record.TextBytes != 250 { + t.Fatalf("late native completion changed timeout metadata: %+v", record) + } + var total time.Duration + for _, stage := range record.Stages { + total += stage.Duration + if stage.Stage == "visible-text" && (stage.SlowestPath != cell.Path || stage.MaxDuration <= 0) { + t.Fatalf("slow operation identity missing: %+v", stage) + } + } + if total != record.Duration || total <= 0 { + t.Fatalf("stage durations %v do not cover capture duration %v", total, record.Duration) + } +} + +func TestCaptureDiagnosticsRequiresDebugLogging(t *testing.T) { + ctx := context.Background() + for _, logger := range []*slog.Logger{nil, logging.New(nil, logging.Options{Level: slog.LevelInfo})} { + gotContext, trace := new_capture_diagnostics(ctx, logger, ObjectID{}) + if gotContext != ctx || trace != nil { + t.Fatal("disabled diagnostics added capture state") + } + trace.begin_stage("snapshot", ObjectID{}, "") + trace.progress("", 0, 0, 0) + trace.set_root(ObjectID{}, "") + trace.finish(nil) + } +} diff --git a/internal/atspi/review_cache_native_test.go b/internal/atspi/review_cache_native_test.go new file mode 100644 index 0000000..38eb456 --- /dev/null +++ b/internal/atspi/review_cache_native_test.go @@ -0,0 +1,167 @@ +// Copyright (c) 2026 Storm Dragon +// Authors: Storm Dragon, Angus Kola +// SPDX-License-Identifier: GPL-3.0-or-later + +package atspi + +import ( + "bufio" + "bytes" + "context" + "encoding/json" + "fmt" + "log/slog" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "git.stormux.org/storm/cthulhu/internal/logging" +) + +func TestNativeFlatReviewPreservesProviderCache(t *testing.T) { + if os.Getenv("CTHULHU_NATIVE_REVIEW_CACHE") != "1" { + t.Skip("requires private Xvfb/AT-SPI session") + } + test_native_review_provider(t, "cache") +} + +func TestNativeFlatReviewBoundsSlowLongRanges(t *testing.T) { + if os.Getenv("CTHULHU_NATIVE_REVIEW_BOUNDS") != "1" { + t.Skip("requires private Xvfb/AT-SPI session") + } + test_native_review_provider(t, "bounds") +} + +func test_native_review_provider(t *testing.T, mode string) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + flags, err := exec.CommandContext(ctx, "pkg-config", "--cflags", "--libs", "gtk+-3.0").Output() + if err != nil { + t.Fatal(err) + } + binary := filepath.Join(t.TempDir(), "review-cache-fixture") + args := append([]string{"../../tests/review_cache_fixture.c", "-o", binary}, strings.Fields(string(flags))...) + if output, err := exec.CommandContext(ctx, "cc", args...).CombinedOutput(); err != nil { + t.Fatalf("build fixture: %v: %s", err, output) + } + var diagnostics bytes.Buffer + adapter := New() + adapter.SetCaptureLogger(logging.New(&diagnostics, logging.Options{Level: slog.LevelDebug})) + if err := adapter.Start(ctx); err != nil { + t.Fatal(err) + } + defer adapter.Stop(context.Background()) + fixture := exec.CommandContext(ctx, binary, mode) + fixture.Stderr = os.Stderr + input, err := fixture.StdinPipe() + if err != nil { + t.Fatal(err) + } + output, err := fixture.StdoutPipe() + if err != nil { + t.Fatal(err) + } + if err := fixture.Start(); err != nil { + t.Fatal(err) + } + defer func() { + _ = fixture.Process.Kill() + _ = fixture.Wait() + }() + if output, err := exec.CommandContext(ctx, "xdotool", "search", "--sync", "--onlyvisible", "--name", "^Review cache probe$", "windowfocus").CombinedOutput(); err != nil { + t.Fatalf("focus fixture: %v: %s", err, output) + } + var focus ObjectID + for !focus.Valid() { + select { + case event := <-adapter.Events(): + if event.Type == WindowActivatedEvent && event.WindowTitle == "Review cache probe" { + focus = event.Source.ID() + } + case <-ctx.Done(): + t.Fatal("fixture focus missing") + } + } + scanner := bufio.NewScanner(output) + command := func(request, want string) { + t.Helper() + if _, err := fmt.Fprintln(input, request); err != nil { + t.Fatal(err) + } + if !scanner.Scan() || scanner.Text() != want { + t.Fatalf("fixture %s: %q, %v", request, scanner.Text(), scanner.Err()) + } + } + if mode == "bounds" { + command("measure", "measuring") + } else { + command("arm", "armed") + } + tree, err := adapter.CaptureReviewWindow(ctx, focus) + if err != nil { + t.Fatal(err) + } + var timing struct { + Message string `json:"message"` + RootScope string `json:"root_scope"` + CapturedNodes int `json:"captured_nodes"` + Stages []captureStageTiming `json:"stages"` + } + if err := json.Unmarshal(diagnostics.Bytes(), &timing); err != nil { + t.Fatalf("capture diagnostics: %v: %s", err, diagnostics.Bytes()) + } + if timing.Message != "flat review capture timing" || timing.RootScope != "window" || timing.CapturedNodes == 0 { + t.Fatalf("missing capture scope/progress: %+v", timing) + } + for _, want := range []string{"queue-wait", "root-selection", "snapshot", "geometry", "visible-text", "table-viewport", "child-count", "child-enumeration", "tree-installation"} { + found := false + for _, stage := range timing.Stages { + if stage.Stage == want && stage.Calls > 0 && stage.Duration > 0 { + found = true + } + } + if !found { + t.Errorf("capture timing missing stage %q: %+v", want, timing) + } + } + if mode == "bounds" { + label := findNamedReference(t, tree, "Long review label") + if !label.Valid() { + t.Fatal("long label missing from review") + } + text, err := tree.VisibleText(label) + if err != nil || !text.Known || text.Limited || len(text.Ranges) == 0 { + t.Fatalf("long label text: %+v, %v", text, err) + } + command("largest", "largest 128") + return + } + label := findNamedReference(t, tree, "Cached review text") + if !label.Valid() { + t.Fatal("cached label missing from review") + } + command("count", "count 0") + command("change", "changed") + for { + select { + case event := <-adapter.Events(): + if event.Source.ID() == label.ID() && strings.HasPrefix(event.Type, "object:text-changed:") { + goto changed + } + case <-ctx.Done(): + t.Fatal("text change missing") + } + } +changed: + tree, err = adapter.CaptureReviewWindow(ctx, focus) + if err != nil { + t.Fatal(err) + } + if !findNamedReference(t, tree, "Changed review text").Valid() { + t.Fatal("dirty label retained old provider data") + } +} diff --git a/internal/atspi/review_table_native.go b/internal/atspi/review_table_native.go new file mode 100644 index 0000000..7c49e34 --- /dev/null +++ b/internal/atspi/review_table_native.go @@ -0,0 +1,237 @@ +// Copyright (c) 2026 Storm Dragon +// Authors: Storm Dragon, Angus Kola +// SPDX-License-Identifier: GPL-3.0-or-later + +package atspi + +/* +#include "adapter_native.h" +*/ +import "C" + +import ( + "context" + "fmt" +) + +// native_review_table_children enumerates a selectable table's viewport, as +// Orca's flat review does. GTK exposes every offscreen row as a child; fetching +// that list creates thousands of transient cell proxies during timeline refresh. +// Returned references belong to the caller. All queries run on the GLib owner. +func native_review_table_children(ctx context.Context, source *C.AtspiAccessible, clip Rectangle) (children []*C.AtspiAccessible, handled bool, err error) { + if !clip.Valid() { + return nil, false, nil + } + table := C.atspi_accessible_get_table_iface(source) + if table == nil { + return nil, false, nil + } + defer C.g_object_unref(C.gpointer(table)) + selection := C.atspi_accessible_get_selection_iface(source) + if selection == nil { + return nil, false, nil + } + defer C.g_object_unref(C.gpointer(selection)) + component := C.atspi_accessible_get_component_iface(source) + if component == nil { + return nil, false, nil + } + defer C.g_object_unref(C.gpointer(component)) + defer func() { + if err != nil { + for _, child := range children { + C.g_object_unref(C.gpointer(child)) + } + children = nil + } + }() + var queryError *C.GError + rows := int(C.atspi_table_get_n_rows(table, &queryError)) + if queryError != nil { + err = nativeError("query review table rows", queryError) + C.g_error_free(queryError) + return nil, true, err + } + columns := int(C.atspi_table_get_n_columns(table, &queryError)) + if queryError != nil { + err = nativeError("query review table columns", queryError) + C.g_error_free(queryError) + return nil, true, err + } + if rows < 0 || columns < 0 { + return nil, true, fmt.Errorf("%w: negative review table dimensions", ErrInvalidTree) + } + // Native focus normalization uses the first selected cell of a uniquely + // selected row. Keep that identity as an anchor when horizontal scrolling + // hides its column; its geometry still excludes it from review output. + if rows > 0 { + if selected := C.atspi_selection_get_selected_child(selection, 0, nil); selected != nil { + children = append(children, selected) + } + } + appendHeaders := func(first, last int) error { + if last-first >= maxTreeNodes { + return fmt.Errorf("%w: visible table exceeds limit %d", ErrInvalidTree, maxTreeNodes) + } + for column := first; column <= last; column++ { + if err := ctx.Err(); err != nil { + return err + } + header := C.atspi_table_get_column_header(table, C.gint(column), nil) + if header == nil { + continue + } + geometry, geometryErr := geometryFromNative(ctx, header) + _, intersects := geometry.Bounds.Intersection(clip) + if geometryErr == nil && geometry.Visible && geometry.Showing && intersects { + children = append(children, header) + } else { + C.g_object_unref(C.gpointer(header)) + } + if geometryErr != nil { + return geometryErr + } + } + return nil + } + if rows == 0 || columns == 0 { + err = appendHeaders(0, columns-1) + return children, true, err + } + // Start just below the table's header. Hit testing avoids walking from row + // zero when the viewport is thousands of rows down the model. A header's + // bounds let us skip directly to its bottom edge. + seedRow, seedColumn := -1, -1 + bottom := int64(clip.Y) + int64(clip.Height) + for y := int64(clip.Y) + 1; y < bottom && seedRow < 0; y++ { + if err := ctx.Err(); err != nil { + return children, true, err + } + for _, x := range []int64{int64(clip.X) + 1, int64(clip.X) + int64(clip.Width)/2} { + child := C.atspi_component_get_accessible_at_point(component, C.gint(x), C.gint(y), C.ATSPI_COORD_TYPE_SCREEN, &queryError) + if queryError != nil { + err = nativeError("locate visible review table cell", queryError) + C.g_error_free(queryError) + if child != nil { + C.g_object_unref(C.gpointer(child)) + } + return children, true, err + } + if child == nil { + continue + } + row, column := -1, -1 + if C.atspi_accessible_get_role(child, nil) == C.ATSPI_ROLE_TABLE_CELL { + if cell := C.atspi_accessible_get_table_cell(child); cell != nil { + var nativeRow, nativeColumn C.gint + C.atspi_table_cell_get_position(cell, &nativeRow, &nativeColumn, &queryError) + if queryError == nil { + row, column = int(nativeRow), int(nativeColumn) + } else { + err = nativeError("query visible review cell position", queryError) + C.g_error_free(queryError) + C.g_object_unref(C.gpointer(cell)) + C.g_object_unref(C.gpointer(child)) + return children, true, err + } + C.g_object_unref(C.gpointer(cell)) + } else { + index := C.atspi_accessible_get_index_in_parent(child, nil) + row = int(C.atspi_table_get_row_at_index(table, index, nil)) + column = int(C.atspi_table_get_column_at_index(table, index, nil)) + } + } + geometry, geometryErr := geometryFromNative(ctx, child) + C.g_object_unref(C.gpointer(child)) + if geometryErr != nil { + return children, true, geometryErr + } + if row >= 0 && row < rows && column >= 0 && column < columns { + seedRow, seedColumn = row, column + break + } + if geometry.BoundsKnown { + y = max(y, int64(geometry.Bounds.Y)+int64(geometry.Bounds.Height)-1) + } + } + } + if seedRow < 0 { + // An empty or clipped body can still have visible column headers. + err = appendHeaders(0, columns-1) + return children, true, err + } + visible := func(row, column int, keep bool) (bool, error) { + if err := ctx.Err(); err != nil { + return false, err + } + child := C.atspi_table_get_accessible_at(table, C.gint(row), C.gint(column), &queryError) + if queryError != nil { + err := nativeError("query visible review table cell", queryError) + C.g_error_free(queryError) + queryError = nil + if child != nil { + C.g_object_unref(C.gpointer(child)) + } + return false, err + } + if child == nil { + return false, nil + } + geometry, geometryErr := geometryFromNative(ctx, child) + _, intersects := geometry.Bounds.Intersection(clip) + showing := geometry.Visible && geometry.Showing && geometry.BoundsKnown && intersects + if keep && showing && geometryErr == nil { + children = append(children, child) + } else { + C.g_object_unref(C.gpointer(child)) + } + return showing, geometryErr + } + firstRow, lastRow := seedRow, seedRow + firstColumn, lastColumn := seedColumn, seedColumn + for _, axis := range []struct { + first, last *int + limit int + row bool + }{{&firstRow, &lastRow, rows, true}, {&firstColumn, &lastColumn, columns, false}} { + for _, step := range []int{-1, 1} { + edge := axis.first + if step > 0 { + edge = axis.last + } + for next := *edge + step; next >= 0 && next < axis.limit; next += step { + if *axis.last-*axis.first >= maxTreeNodes { + return children, true, fmt.Errorf("%w: visible table exceeds limit %d", ErrInvalidTree, maxTreeNodes) + } + row, column := seedRow, seedColumn + if axis.row { + row = next + } else { + column = next + } + showing, err := visible(row, column, false) + if err != nil { + return children, true, err + } + if !showing { + break + } + *edge = next + } + } + } + if err := appendHeaders(firstColumn, lastColumn); err != nil { + return children, true, err + } + for row := firstRow; row <= lastRow; row++ { + for column := firstColumn; column <= lastColumn; column++ { + if len(children) >= maxTreeNodes { + return children, true, fmt.Errorf("%w: visible table exceeds limit %d", ErrInvalidTree, maxTreeNodes) + } + if _, err := visible(row, column, true); err != nil { + return children, true, err + } + } + } + return children, true, nil +} diff --git a/internal/atspi/review_table_native_test.go b/internal/atspi/review_table_native_test.go new file mode 100644 index 0000000..9003d18 --- /dev/null +++ b/internal/atspi/review_table_native_test.go @@ -0,0 +1,217 @@ +// 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" + "testing" + "time" +) + +func TestNativeFlatReviewLargeGTKTable(t *testing.T) { + if os.Getenv("CTHULHU_NATIVE_REVIEW_TABLE") != "1" { + t.Skip("requires private Xvfb/AT-SPI session") + } + for _, mode := range []string{"large", "small", "empty", "horizontal", "compound"} { + t.Run(mode, func(t *testing.T) { test_native_review_table(t, mode) }) + } +} + +func test_native_review_table(t *testing.T, mode string) { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 25*time.Second) + defer cancel() + fixture := exec.CommandContext(ctx, "/usr/bin/python3", "-c", ` +import gi, sys +gi.require_version("Gtk", "3.0") +gi.require_version("Gdk", "3.0") +from gi.repository import Gdk, GLib, Gtk +mode = sys.argv[1] +Gtk.Settings.get_default().set_property("gtk-enable-animations", False) +window = Gtk.Window(title="Flat table review probe") +window.set_default_size(400, 240) +window.connect("destroy", Gtk.main_quit) +store = Gtk.ListStore(str, str, str) +rowCount = {"empty": 0, "small": 2, "horizontal": 20, "compound": 20}.get(mode, 6000) +for index in range(rowCount): + store.append(["Row %04d" % index, "Kind %04d" % index, "Detail %04d" % index]) +view = Gtk.TreeView(model=store) +view.get_accessible().set_name("Review table") +column = Gtk.TreeViewColumn("Items", Gtk.CellRendererText(), text=0) +if mode == "compound": + icon = Gtk.CellRendererPixbuf() + icon.set_property("icon-name", "text-x-generic") + column.pack_start(icon, False) +view.append_column(column) +if mode == "horizontal": + view.append_column(Gtk.TreeViewColumn("Kind", Gtk.CellRendererText(), text=1)) + view.append_column(Gtk.TreeViewColumn("Detail", Gtk.CellRendererText(), text=2)) + for column in view.get_columns(): + column.set_sizing(Gtk.TreeViewColumnSizing.FIXED) + column.set_fixed_width(250) +scroller = Gtk.ScrolledWindow() +scroller.add(view) +window.add(scroller) +window.show_all() +def ready(): + if rowCount: + path = Gtk.TreePath.new_from_indices([rowCount//2]) + column = view.get_columns()[-1] if mode == "horizontal" else None + view.set_cursor(path, column) + view.scroll_to_cell(path, column, True, 0.5, 1) + window.present() + view.grab_focus() + acknowledge_paint("Flat table review ready") + return False +def acknowledge_paint(title): + def painted(clock): + clock.disconnect(handlerId) + window.set_title(title) + handlerId = window.get_frame_clock().connect("after-paint", painted) + window.queue_draw() +GLib.idle_add(ready) +def on_key(_window, event): + if event.keyval == Gdk.KEY_F6: + store[4500][0] = "Updated 4500" + path = Gtk.TreePath.new_from_indices([4500]) + view.set_cursor(path) + view.scroll_to_cell(path, None, True, 0.5, 0) + acknowledge_paint("Flat table review updated") + return True + return False +window.connect("key-press-event", on_key) +Gtk.main() +`, mode) + fixture.Stderr = os.Stderr + adapter := New() + if err := adapter.Start(ctx); err != nil { + t.Fatal(err) + } + defer adapter.Stop(context.Background()) + if err := fixture.Start(); err != nil { + t.Fatal(err) + } + defer func() { + _ = fixture.Process.Kill() + _ = fixture.Wait() + }() + if output, err := exec.CommandContext(ctx, "xdotool", "search", "--sync", "--onlyvisible", "--name", "^Flat table review (probe|ready)$", "windowfocus").CombinedOutput(); err != nil { + t.Fatalf("focus fixture: %v: %s", err, output) + } + var focus ObjectID + painted := false + for !focus.Valid() || !painted { + select { + case event := <-adapter.Events(): + painted = painted || event.Source.Name == "Flat table review ready" || event.WindowTitle == "Flat table review ready" + if (event.Source.Role == "table cell" || event.Source.Name == "Review table") && + (event.Type == FocusEvent || event.Type == FocusedStateEvent && event.Detail1 == 1) { + focus = event.Source.ID() + } + case <-ctx.Done(): + t.Fatal("table focus missing") + } + } + captureCtx, captureCancel := context.WithTimeout(ctx, 5*time.Second) + defer captureCancel() + tree, err := adapter.CaptureReviewWindow(captureCtx, focus) + if err != nil { + t.Fatal(err) + } + snapshot, err := tree.ReviewWindow(ctx, focus) + if err != nil { + t.Fatal(err) + } + want := "Row 3000" + switch mode { + case "small": + want = "Row 0001" + case "empty": + want = "Items" + case "horizontal": + want = "Detail 0010" + case "compound": + want = "Row 0010" + } + found := false + headerFound := false + visibleNames := make(map[string]bool) + for _, node := range snapshot.Nodes { + if node.Geometry.Visible && node.Geometry.Showing && node.Geometry.BoundsKnown { + visibleNames[node.Snapshot.Name] = true + } + if node.Snapshot.Name == want && node.Geometry.Visible && node.Geometry.Showing && node.Geometry.BoundsKnown { + found = true + } + if node.Snapshot.Role == "table column header" { + headerFound = true + } + if mode == "horizontal" && node.Snapshot.Name == "Row 0010" && len(node.Text.Ranges) != 0 { + t.Fatal("captured text from horizontally hidden focus anchor") + } + } + if !found { + for _, node := range snapshot.Nodes { + t.Logf("node: %q %q geometry=%+v", node.Snapshot.Name, node.Snapshot.Role, node.Geometry) + } + t.Fatalf("visible content %q missing", want) + } + if !headerFound { + t.Fatal("visible column header missing") + } + if mode == "large" && (!visibleNames["Row 2999"] || !visibleNames["Row 3001"]) { + t.Fatal("visible rows adjacent to focus are missing") + } + t.Logf("captured %d viewport objects", len(snapshot.Nodes)) + if len(snapshot.Nodes) > 100 { + t.Fatalf("captured %d nodes for a small table viewport", len(snapshot.Nodes)) + } + if mode != "large" { + return + } + if output, err := exec.CommandContext(ctx, "xdotool", "key", "F6").CombinedOutput(); err != nil { + t.Fatalf("scroll fixture: %v: %s", err, output) + } + updated, painted := false, false + for !updated || !painted { + select { + case event := <-adapter.Events(): + painted = painted || event.Source.Name == "Flat table review updated" || event.WindowTitle == "Flat table review updated" + if event.Source.Name == "Updated 4500" && (event.Type == ActiveDescendantEvent || + event.Type == FocusEvent || event.Type == FocusedStateEvent && event.Detail1 == 1) { + focus = event.Source.ID() + updated = true + } + case <-ctx.Done(): + t.Fatal("updated table focus missing") + } + } + tree, err = adapter.CaptureReviewWindow(ctx, focus) + if err != nil { + t.Fatal(err) + } + snapshot, err = tree.ReviewWindow(ctx, focus) + if err != nil { + t.Fatal(err) + } + found = false + visibleNames = make(map[string]bool) + for _, node := range snapshot.Nodes { + if node.Geometry.Visible && node.Geometry.Showing && node.Geometry.BoundsKnown { + visibleNames[node.Snapshot.Name] = true + } + if node.Snapshot.Name == "Updated 4500" && node.Geometry.Visible && node.Geometry.Showing && node.Geometry.BoundsKnown { + found = true + } + if node.Snapshot.Name == "Row 3000" && len(node.Text.Ranges) > 0 { + t.Fatal("recapture retained the previous viewport's text") + } + } + if !found || !visibleNames["Row 4499"] || !visibleNames["Row 4501"] || len(snapshot.Nodes) > 100 { + t.Fatalf("updated viewport: found=%t nodes=%d", found, len(snapshot.Nodes)) + } +} diff --git a/internal/atspi/text.go b/internal/atspi/text.go index f1865e1..8821b3e 100644 --- a/internal/atspi/text.go +++ b/internal/atspi/text.go @@ -20,6 +20,7 @@ package atspi import ( "context" "errors" + "math" "strings" "unicode/utf8" ) @@ -27,10 +28,11 @@ import ( const ( // A large terminal viewport can contain thousands of padded cells. // Keep the per-line query and web line-repair budgets independently small. - maxReviewCharacters = 1 << 16 - maxReviewLineCharacters = 4096 - maxReviewRanges = 256 - maxTreeTextBytes = 1 << 20 + maxReviewCharacters = 1 << 16 + maxReviewLineCharacters = 4096 + maxReviewRanges = 256 + maxTreeTextBytes = 1 << 20 + maxReviewBoundsCharacters = 128 ) var ErrStaleText = errors.New("accessible text requires recapture") @@ -123,6 +125,42 @@ type reviewTextSource interface { bounds(context.Context, int, int) (Rectangle, error) } +// bounded_review_bounds preserves the full range's bounding rectangle without +// asking a provider to measure thousands of characters in one synchronous call. +// GTK text cells can exceed libatspi's timeout on such requests. Union every +// chunk rather than inferring bounds from endpoints, which would lose bidi runs. +func bounded_review_bounds(ctx context.Context, start, end int, query func(context.Context, int, int) (Rectangle, error)) (Rectangle, error) { + var bounds Rectangle + for offset := start; offset < end; { + if err := ctx.Err(); err != nil { + return Rectangle{}, err + } + next := offset + min(end-offset, maxReviewBoundsCharacters) + chunk, err := query(ctx, offset, next) + if err != nil { + return Rectangle{}, err + } + offset = next + // Formatting characters can have no rectangle; other chunks still + // contribute to the complete range, just as in the provider's union. + if !chunk.Valid() { + continue + } + if !bounds.Valid() { + bounds = chunk + continue + } + left, top := min(bounds.X, chunk.X), min(bounds.Y, chunk.Y) + right := max(int64(bounds.X)+int64(bounds.Width), int64(chunk.X)+int64(chunk.Width)) + bottom := max(int64(bounds.Y)+int64(bounds.Height), int64(chunk.Y)+int64(chunk.Height)) + if right-int64(left) > math.MaxInt32 || bottom-int64(top) > math.MaxInt32 { + return Rectangle{}, errors.New("review range bounds exceed coordinate limits") + } + bounds = Rectangle{left, top, int32(right - int64(left)), int32(bottom - int64(top))} + } + return bounds, ctx.Err() +} + // VTE can report UTF-8 bytes as its character count while its text and line // methods use character offsets. Locate the actual end with bounded single- // character queries, without copying or scanning terminal scrollback. @@ -240,21 +278,37 @@ func captureVisibleText(ctx context.Context, source reviewTextSource, clip Recta result.Ranges = append(result.Ranges, TextRange{lineStart + start, lineStart + end, string(runes[start:end]), bounds}) } else if visible { // A clipped rectangle alone cannot hide offscreen characters. Check each - // character only for partially clipped runs, preserving disjoint spans. - for i := start; i < end; i++ { - if err := ctx.Err(); err != nil { - return unknown, err + // visible character, preserving disjoint spans. Reject offscreen blocks + // first so a horizontally clipped long line does not require thousands + // of individual provider calls. Test every block for bidi text. + for blockStart := start; blockStart < end; { + blockEnd := blockStart + min(end-blockStart, maxReviewBoundsCharacters) + if end-start > maxReviewBoundsCharacters { + blockBounds, err := source.bounds(ctx, lineStart+blockStart, lineStart+blockEnd) + if err != nil { + return unknown, err + } + if _, ok := blockBounds.Intersection(clip); !ok { + blockStart = blockEnd + continue + } } - charBounds, err := source.bounds(ctx, lineStart+i, lineStart+i+1) - if err != nil { - return unknown, err - } - if charClip, ok := charBounds.Intersection(clip); ok { - result.Ranges = append(result.Ranges, TextRange{lineStart + i, lineStart + i + 1, string(runes[i]), charClip}) - } - if len(result.Ranges) > maxReviewRanges { - return VisibleText{Limited: true}, nil + for i := blockStart; i < blockEnd; i++ { + if err := ctx.Err(); err != nil { + return unknown, err + } + charBounds, err := source.bounds(ctx, lineStart+i, lineStart+i+1) + if err != nil { + return unknown, err + } + if charClip, ok := charBounds.Intersection(clip); ok { + result.Ranges = append(result.Ranges, TextRange{lineStart + i, lineStart + i + 1, string(runes[i]), charClip}) + } + if len(result.Ranges) > maxReviewRanges { + return VisibleText{Limited: true}, nil + } } + blockStart = blockEnd } } if len(result.Ranges) > maxReviewRanges { diff --git a/internal/atspi/text_bounds_test.go b/internal/atspi/text_bounds_test.go new file mode 100644 index 0000000..80b6877 --- /dev/null +++ b/internal/atspi/text_bounds_test.go @@ -0,0 +1,90 @@ +// Copyright (c) 2026 Storm Dragon +// Authors: Storm Dragon, Angus Kola +// SPDX-License-Identifier: GPL-3.0-or-later + +package atspi + +import ( + "context" + "errors" + "strings" + "testing" +) + +type longReviewText struct { + runes []rune + queries int + largest int +} + +func (source *longReviewText) count(context.Context) (int, error) { return len(source.runes), nil } +func (source *longReviewText) line(context.Context, int) (TextRange, error) { + return TextRange{Start: 0, End: len(source.runes), Text: string(source.runes)}, nil +} +func (source *longReviewText) bounds(ctx context.Context, start, end int) (Rectangle, error) { + return bounded_review_bounds(ctx, start, end, func(_ context.Context, first, last int) (Rectangle, error) { + source.queries++ + source.largest = max(source.largest, last-first) + if last-first > 128 { + return Rectangle{}, errors.New("provider timed out measuring a long range") + } + return Rectangle{X: int32(first * 10), Width: int32((last - first) * 10), Height: 20}, nil + }) +} + +func TestCaptureVisibleTextBoundsLongHorizontalLine(t *testing.T) { + source := &longReviewText{runes: []rune(strings.Repeat("é猫", 1712))} + clip := Rectangle{X: 1000, Width: 300, Height: 20} + got, err := captureVisibleText(context.Background(), source, clip) + if err != nil || !got.Known || got.Limited { + t.Fatalf("long line: %+v, %v", got, err) + } + var spoken strings.Builder + for _, part := range got.Ranges { + if part.Start < 100 || part.End > 130 { + t.Fatalf("offscreen range retained: %+v", part) + } + spoken.WriteString(part.Text) + } + if spoken.String() != string(source.runes[100:130]) || source.largest > 128 || source.queries > 400 { + t.Fatalf("clipping: text_bytes=%d largest_query=%d queries=%d", spoken.Len(), source.largest, source.queries) + } + if !got.valid(Geometry{BoundsKnown: true, Bounds: clip}) { + t.Fatal("long clipped line violates owned text invariants") + } +} + +func TestBoundedReviewBoundsIncludesInteriorAndEmptyChunks(t *testing.T) { + rectangles := []Rectangle{{X: 500, Width: 100, Height: 20}, {X: -50, Width: 300, Height: 20}, {}, {X: 400, Width: 50, Height: 20}} + queries := 0 + got, err := bounded_review_bounds(context.Background(), 0, 512, func(_ context.Context, start, end int) (Rectangle, error) { + if end-start > 128 { + t.Fatalf("oversized query: %d", end-start) + } + queries++ + return rectangles[start/128], nil + }) + if err != nil || got != (Rectangle{X: -50, Width: 650, Height: 20}) || queries != 4 { + t.Fatalf("union lost interior bidi or empty chunks: %+v, %v, queries=%d", got, err, queries) + } +} + +func TestBoundedReviewBoundsStopsOnFailureOrCancellation(t *testing.T) { + providerError := errors.New("provider unavailable") + if _, err := bounded_review_bounds(context.Background(), 0, 512, func(context.Context, int, int) (Rectangle, error) { + return Rectangle{}, providerError + }); !errors.Is(err, providerError) { + t.Fatalf("provider error hidden: %v", err) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + queries := 0 + _, err := bounded_review_bounds(ctx, 0, 512, func(context.Context, int, int) (Rectangle, error) { + queries++ + cancel() + return Rectangle{Width: 100, Height: 20}, nil + }) + if !errors.Is(err, context.Canceled) || queries != 1 { + t.Fatalf("cancellation: %v queries=%d", err, queries) + } +} diff --git a/internal/atspi/text_native.go b/internal/atspi/text_native.go index 293d28c..bc194cc 100644 --- a/internal/atspi/text_native.go +++ b/internal/atspi/text_native.go @@ -195,6 +195,17 @@ func (source nativeReviewText) line(ctx context.Context, offset int) (TextRange, } func (source nativeReviewText) bounds(ctx context.Context, start, end int) (Rectangle, error) { + rect, err := bounded_review_bounds(ctx, start, end, source.range_bounds) + if err != nil { + return Rectangle{}, err + } + if !rect.Valid() { + return Rectangle{}, errors.New("invalid review range bounds") + } + return rect, ctx.Err() +} + +func (source nativeReviewText) range_bounds(ctx context.Context, start, end int) (Rectangle, error) { if err := ctx.Err(); err != nil { return Rectangle{}, err } @@ -204,16 +215,14 @@ func (source nativeReviewText) bounds(ctx context.Context, start, end int) (Rect defer C.g_free(C.gpointer(bounds)) } if nativeErr != nil { + err := nativeError("query review range bounds", nativeErr) C.g_error_free(nativeErr) - return Rectangle{}, errors.New("query review range bounds") + return Rectangle{}, err } if bounds == nil { return Rectangle{}, errors.New("missing review range bounds") } rect := Rectangle{int32(bounds.x), int32(bounds.y), int32(bounds.width), int32(bounds.height)} - if !rect.Valid() { - return Rectangle{}, errors.New("invalid review range bounds") - } return rect, ctx.Err() } diff --git a/tests/review_cache_fixture.c b/tests/review_cache_fixture.c new file mode 100644 index 0000000..d18c357 --- /dev/null +++ b/tests/review_cache_fixture.c @@ -0,0 +1,95 @@ +// Copyright (c) 2026 Storm Dragon +// Authors: Storm Dragon, Angus Kola +// SPDX-License-Identifier: GPL-3.0-or-later + +#include +#include +#include + +static gboolean armed; +static unsigned childQueries; +static gboolean slowRanges; +static gint largestRange; +static void (*originalRangeExtents)(AtkText *, gint, gint, AtkCoordType, AtkTextRectangle *); + +static void range_extents(AtkText *text, gint start, gint end, AtkCoordType coordinates, AtkTextRectangle *rectangle) { + if (slowRanges) { + largestRange = MAX(largestRange, end - start); + if (end - start > 128) g_usleep(1200000); + } + originalRangeExtents(text, start, end, coordinates, rectangle); +} + +static gint count_children(AtkObject *object) { + (void)object; + if (armed) { + childQueries++; + // The initial cache already reports this label as a leaf. A redundant + // property query models the temporarily unresponsive live cell. + g_usleep(1200000); + } + return 0; +} + +static gboolean command_ready(GIOChannel *channel, GIOCondition condition, gpointer label) { + if (condition & G_IO_HUP) { + gtk_main_quit(); + return FALSE; + } + gchar *line = NULL; + if (g_io_channel_read_line(channel, &line, NULL, NULL, NULL) != G_IO_STATUS_NORMAL) + return TRUE; + if (g_str_has_prefix(line, "arm")) { + armed = TRUE; + puts("armed"); + } else if (g_str_has_prefix(line, "count")) { + printf("count %u\n", childQueries); + } else if (g_str_has_prefix(line, "measure")) { + slowRanges = TRUE; + puts("measuring"); + } else if (g_str_has_prefix(line, "largest")) { + printf("largest %d\n", largestRange); + } else if (g_str_has_prefix(line, "change")) { + armed = FALSE; + gtk_label_set_text(GTK_LABEL(label), "Changed review text"); + puts("changed"); + } + g_free(line); + fflush(stdout); + return TRUE; +} + +int main(int argc, char **argv) { + gboolean boundsMode = argc > 1 && strcmp(argv[1], "bounds") == 0; + gtk_init(&argc, &argv); + GtkWidget *window = gtk_window_new(GTK_WINDOW_TOPLEVEL); + gtk_window_set_title(GTK_WINDOW(window), "Review cache probe"); + gtk_window_set_default_size(GTK_WINDOW(window), 300, 100); + g_signal_connect(window, "destroy", G_CALLBACK(gtk_main_quit), NULL); + GtkWidget *label = gtk_label_new("Cached review text"); + // This fixture has one label. Replace its class vfunc before the bridge + // builds the initial cache, then arm the delay explicitly over stdin. + ATK_OBJECT_GET_CLASS(gtk_widget_get_accessible(label))->get_n_children = count_children; + if (boundsMode) { + gchar *content = g_strnfill(3424, 'x'); + gtk_label_set_text(GTK_LABEL(label), content); + g_free(content); + gtk_label_set_xalign(GTK_LABEL(label), 0); + AtkObject *accessible = gtk_widget_get_accessible(label); + atk_object_set_name(accessible, "Long review label"); + originalRangeExtents = ATK_TEXT_GET_IFACE(accessible)->get_range_extents; + if (originalRangeExtents == NULL) g_error("fixture needs native GTK range extents"); + ATK_TEXT_GET_IFACE(accessible)->get_range_extents = range_extents; + GtkWidget *scroller = gtk_scrolled_window_new(NULL, NULL); + gtk_container_add(GTK_CONTAINER(scroller), label); + gtk_container_add(GTK_CONTAINER(window), scroller); + } else { + gtk_container_add(GTK_CONTAINER(window), label); + } + GIOChannel *input = g_io_channel_unix_new(0); + g_io_add_watch(input, G_IO_IN | G_IO_HUP, command_ready, label); + gtk_widget_show_all(window); + gtk_main(); + g_io_channel_unref(input); + return 0; +}