From 20578ada406e66e1d0fc15efdb60597fcd136f5f Mon Sep 17 00:00:00 2001 From: Michael Taboada Date: Sat, 7 Nov 2026 10:58:48 -0800 Subject: [PATCH] Fix some issues with clickables on some websites. --- docs/testing/browser-clickables.md | 55 ++++++ internal/app/browser_actions_native_test.go | 191 ++++++++++++++++++++ internal/atspi/adapter_actions_native.go | 10 + internal/atspi/adapter_document_native.go | 14 +- testdata/parity-ledger.csv | 1 + tests/browser-actions.html | 34 ++++ tests/native_browser.py | 1 + 7 files changed, 304 insertions(+), 2 deletions(-) create mode 100644 docs/testing/browser-clickables.md diff --git a/docs/testing/browser-clickables.md b/docs/testing/browser-clickables.md new file mode 100644 index 0000000..32259ef --- /dev/null +++ b/docs/testing/browser-clickables.md @@ -0,0 +1,55 @@ +# Clickable disclosures + +A successful accessible action can reveal options without a usable +children-changed event. The Actions list must invalidate the same document +ancestry as Enter activation. Both paths also invalidate the target snapshot: +caret navigation deliberately tolerates stale child order on ticking pages, +so child-order invalidation alone can leave the newly opened options skipped. +The next move uses the existing bounded recapture, retaining live object +identities and the saved browse position. Action ownership checks remain strict. + +Text caret placement also reveals its native range. Explicitly scrolling the +whole accessible object first can scroll the outer document while reading an +inner popup and cause the page to dismiss that popup. Text targets now rely on +native caret placement, as Python does; non-text stops retain component scrolling. + +## Automated checks + +```sh +CTHULHU_BROWSER=/usr/bin/chromium CTHULHU_NATIVE_BROWSER_TESTS=actions,disclosure,reveal,positions,ticking,document python3 tests/native_browser.py +``` + +The disclosure proof opens a generic scrollable popup through the label's +ancestor action and through Enter. It reads characters and both options in +both directions before closing it. The popup dismisses on blur or outer-document +scrolling. Selection-change markers wait for two browser animation frames so +queued scrolling and dismissal run before checking that the option remains +available. Restoring unconditional component scrolling fails this check. + +The completion marker is separate from the reader's focus anchor so it cannot +accidentally force recapture and hide the earlier activation regression. +Viewport intersection acknowledgements also verify that distant text and +non-text images are revealed. The other proofs cover iframe actions, revealed +headings, list-position metadata, continuously replaced text, and browser +document commands. All use private Xorg and D-Bus. + +The original public page was also checked in a private Chromium profile, with +browser animation-frame acknowledgements between moves. Before the scroll fix, +the next character after the first tag returned to the closed filter label. +With the fix, character reading and moving to the next tag preserved the popup +and its focused scroll container. + +The separate `link-sounds` proof failed its initial native Tab-position check +with pans `[0 0 0]` both with the original unconditional scrolling and with the +fix. That baseline failure remains unresolved; it is not a passing check here. + +## Live acceptance + +With a rebuilt reader on the originally reported page, open each filter using +Enter and the Actions list. Read its options with Down and Up, select an option, +then reopen it and verify that it remains usable. Also check ordinary links, +buttons, Tab/Shift+Tab, and reading through a continuously updating page. + +The user's installed Brave session, physical input, audible speech, and physical +Braille remain live acceptance checks. Human diff review and live testing +precede submission. diff --git a/internal/app/browser_actions_native_test.go b/internal/app/browser_actions_native_test.go index 6148c80..040d87a 100644 --- a/internal/app/browser_actions_native_test.go +++ b/internal/app/browser_actions_native_test.go @@ -6,6 +6,7 @@ package app import ( "context" + "fmt" "os" "os/exec" "strings" @@ -250,3 +251,193 @@ func TestNativeBrowserActionOwnership(t *testing.T) { previousContent = mode + " mode content" } } + +func TestNativeBrowserDisclosureFocus(t *testing.T) { + if os.Getenv("CTHULHU_NATIVE_BROWSER") != "1" { + t.Skip("requires private browser disclosure fixture") + } + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + adapter := atspi.New() + if err := adapter.Start(ctx); err != nil { + t.Fatal(err) + } + defer adapter.Stop(context.Background()) + var focus atspi.ObjectID + for attempt := 0; attempt < 25 && !focus.Valid(); attempt++ { + if out, err := exec.CommandContext(ctx, "xdotool", "key", "--clearmodifiers", "Tab").CombinedOutput(); err != nil { + t.Fatalf("fixture focus: %v %s", err, out) + } + timer := time.NewTimer(150 * time.Millisecond) + drain: + for { + select { + case event := <-adapter.Events(): + if isFocusGainEvent(event) && event.Source.Name == "Action fixture start" { + focus = event.Source.ID() + break drain + } + case <-timer.C: + break drain + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + } + timer.Stop() + } + if !focus.Valid() { + t.Fatal("fixture heading focus missing") + } + worker, err := review.NewWorker(adapter, 2*time.Second) + if err != nil { + t.Fatal(err) + } + if err := worker.Start(ctx); err != nil { + t.Fatal(err) + } + defer worker.Stop(context.Background()) + worker.SyncFocus(focus) + testBrowserDisclosureActions(t, ctx, adapter, worker) +} + +func testBrowserDisclosureActions(t *testing.T, ctx context.Context, adapter *atspi.Adapter, worker *review.Worker) { + t.Helper() + move := func(command review.DocumentCaretCommand) review.Result { + t.Helper() + if err := worker.MoveDocumentCaret(command); err != nil { + t.Fatal(err) + } + select { + case result := <-worker.Results(): + if result.Err != nil || result.CaretErr != nil { + t.Fatalf("disclosure navigation failed: %+v", result) + } + return result + case <-ctx.Done(): + t.Fatal(ctx.Err()) + return review.Result{} + } + } + waitToggle := func(want string) { + t.Helper() + for { + select { + case event := <-adapter.Events(): + if event.Type == atspi.AccessibleNameEvent && event.Source.Name == want { + return + } + case <-ctx.Done(): + t.Fatalf("disclosure did not report %q: %v", want, ctx.Err()) + } + } + } + for _, method := range []string{"actions", "enter"} { + var label atspi.ObjectReference + for step := 0; step < 60; step++ { + command := review.CaretNextLine + if step == 0 { + command = review.CaretDocumentStart + } + result := move(command) + if strings.TrimSpace(result.DocumentCaretText) == "Choose category" { + label = result.DocumentPosition.Object + break + } + } + if !label.Valid() { + t.Fatal("disclosure label missing") + } + waitToggle("Caret settled:Choose category:0:closed") + perform := func() { + t.Helper() + actions, err := adapter.AccessibleActions(ctx, label.ID()) + if err != nil { + t.Fatalf("%s list actions: %v", method, err) + } + if err := adapter.PerformAccessibleAction(ctx, actions.Target, "clickAncestor"); err != nil { + current, currentErr := adapter.AccessibleActions(ctx, label.ID()) + t.Fatalf("%s perform action: %v; before=%+v after=%+v (%v)", method, err, actions.Target, current.Target, currentErr) + } + } + if method == "actions" { + perform() + } else { + if err := worker.ActivateDocumentCaret(); err != nil { + t.Fatal(err) + } + select { + case result := <-worker.Results(): + if result.Err != nil { + t.Fatalf("%s Enter activation: %v", method, result.Err) + } + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + } + waitToggle("Categories opened") + // A real dropdown's scroll container can acquire browser focus when + // its first option receives the native caret. Reading within it must + // not blur that container and dismiss the rest of the options. + optionAvailable := func(result review.Result) { + t.Helper() + prefix := fmt.Sprintf("Caret settled:%s:%d:", result.DocumentPosition.Snapshot.Name, result.DocumentPosition.Offset) + for { + select { + case event := <-adapter.Events(): + if event.Type == atspi.AccessibleNameEvent && strings.HasPrefix(event.Source.Name, prefix) { + if event.Source.Name != prefix+"open" { + t.Fatalf("%s popup closed after %s", method, result.DocumentCaretCommand) + } + goto settled + } + case <-ctx.Done(): + t.Fatalf("missing %s acknowledgement: %v", prefix, ctx.Err()) + } + } + settled: + if _, err := adapter.AccessibleActions(ctx, result.DocumentPosition.Object.ID()); err != nil { + t.Fatalf("%s popup option disappeared after %s: %v", method, result.DocumentCaretCommand, err) + } + } + first := move(review.CaretNextLine) + if strings.TrimSpace(first.DocumentCaretText) != "First category" { + t.Fatalf("%s first disclosure option: %q", method, first.DocumentCaretText) + } + optionAvailable(first) + character := move(review.CaretNextCharacter) + if character.DocumentCaretText != "i" { + t.Fatalf("%s disclosure character: got %q, want i", method, character.DocumentCaretText) + } + optionAvailable(character) + optionAvailable(move(review.CaretLineStart)) + for _, want := range []string{"Second category"} { + result := move(review.CaretNextLine) + if strings.TrimSpace(result.DocumentCaretText) != want { + t.Fatalf("%s disclosure navigation: got %q, want %q", method, result.DocumentCaretText, want) + } + optionAvailable(result) + } + // Both options must remain available when browsing backwards too. + for _, want := range []string{"First category"} { + result := move(review.CaretPreviousLine) + if strings.TrimSpace(result.DocumentCaretText) != want { + t.Fatalf("%s reverse disclosure navigation: got %q, want %q", method, result.DocumentCaretText, want) + } + optionAvailable(result) + } + perform() + waitToggle("Categories closed") + } + // Text caret placement must still reveal offscreen text, and the + // explicit component scroll remains necessary for a non-text image. + end := move(review.CaretDocumentEnd) + if end.DocumentPosition.Snapshot.Name != "Distant image" { + t.Fatalf("document end = %q, want distant image", end.DocumentPosition.Snapshot.Name) + } + waitToggle("Viewport:distant-image") + previous := move(review.CaretPreviousLine) + if strings.TrimSpace(previous.DocumentCaretText) != "Distant text" { + t.Fatalf("offscreen previous line = %q", previous.DocumentCaretText) + } + waitToggle("Viewport:distant-text") +} diff --git a/internal/atspi/adapter_actions_native.go b/internal/atspi/adapter_actions_native.go index adff6f9..f98540e 100644 --- a/internal/atspi/adapter_actions_native.go +++ b/internal/atspi/adapter_actions_native.go @@ -139,6 +139,16 @@ func (adapter *Adapter) native_actions(ctx context.Context, object ObjectID, exp C.g_error_free(nativeErr) return ActionSet{}, errors.New("accessible action failed") } + // Actions can reveal document branches without a children-changed + // event. Refresh the same ancestry as browse-caret activation so + // the next navigation discovers the newly opened options. + adapter.treeMutex.Lock() + tree := adapter.documentTree + adapter.treeMutex.Unlock() + if tree != nil { + adapter.treeInvalidations.Add(uint64(tree.markAncestorChildrenChanged(object))) + adapter.treeInvalidations.Add(uint64(tree.InvalidateSnapshot(object))) + } return result, nil } labelRaw := C.atspi_action_get_localized_name(action, C.gint(index), nil) diff --git a/internal/atspi/adapter_document_native.go b/internal/atspi/adapter_document_native.go index 3fc5806..3c87fac 100644 --- a/internal/atspi/adapter_document_native.go +++ b/internal/atspi/adapter_document_native.go @@ -269,6 +269,10 @@ func (adapter *Adapter) activateNativeDocumentObject(ctx context.Context, positi } if activated != 0 { adapter.treeInvalidations.Add(uint64(tree.markAncestorChildrenChanged(position.ID()))) + // Caret navigation tolerates stale child order on ticking pages. + // A completed action must instead refresh before leaving its + // target, or it can skip a newly revealed popup entirely. + adapter.treeInvalidations.Add(uint64(tree.InvalidateSnapshot(position.ID()))) return nil } } @@ -551,15 +555,21 @@ func (adapter *Adapter) setNativeDocumentPosition(ctx context.Context, position for _, id := range echoIDs { adapter.navigationEcho[id] = 15 } + text := C.atspi_accessible_get_text_iface(source) component := C.atspi_accessible_get_component_iface(source) if component != nil { if documentTargetGrabsFocus(tree, position, snapshot) { C.atspi_component_grab_focus(component, nil) } - C.atspi_component_scroll_to(component, C.ATSPI_SCROLL_ANYWHERE, nil) + // Setting a text caret reveals its range. An additional whole-object + // scroll can move the outer document while reading a popup's inner + // scroller, causing the page to dismiss the popup. Python likewise + // relies on caret placement for text; non-text stops still need scroll. + if text == nil { + C.atspi_component_scroll_to(component, C.ATSPI_SCROLL_ANYWHERE, nil) + } C.g_object_unref(C.gpointer(component)) } - text := C.atspi_accessible_get_text_iface(source) if text == nil { clear(adapter.navigationEcho) // Images and other whole-object stops have a valid virtual position diff --git a/testdata/parity-ledger.csv b/testdata/parity-ledger.csv index 8816d37..8655204 100644 --- a/testdata/parity-ledger.csv +++ b/testdata/parity-ledger.csv @@ -74,3 +74,4 @@ speech-interruption-preferences,src/cthulhu/input_event.py,input,"Choose all key pidgin-message-history,src/cthulhu/chat.py;src/cthulhu/scripts/apps/pidgin/chat.py;src/cthulhu/scripts/apps/pidgin/script.py,messaging,Announce verified Pidgin transcript insertions and review or copy nine recent messages,Storm Dragon,internal/messaging internal/atspi internal/app internal/input,implemented,go test ./...; go test -race ./internal/messaging ./internal/input ./internal/atspi ./internal/app; see docs/testing/messaging.md for private native checks,Private GTK transcript and X11 command checks pass; actual Pidgin and physical shortcuts pending.,,Recorded presentation tested; actual Pidgin listening pending.,Single flash recorded; physical Braille pending.,"Legacy defaults only: combined per-provider history, all conversations, no room prefix or typing announcements. Legacy chat preference controls remain unported. Cthulhu+H is the sole default learn-mode chord; F1-F9 review and Shift+F1-F9 copy. Existing explicit assignments and modal OCR F5 refresh take precedence. Steam notification history remains separate. See docs/testing/messaging.md." atspi-announcements,Orca src/orca/script.py;src/orca/scripts/default.py;src/orca/event_manager.py,focus,"Present object:announcement payloads from any provider, politely or assertively, without requiring focus or foreground ownership",,internal/atspi internal/app,implemented,"go test ./internal/atspi ./internal/app -run Announcement -count=1; native GTK4 fixture delivers polite detail1=1 and assertive detail1=2 payloads through the adapter; live Niri 26.04 event capture recorded the workspace-switch announcement shape",Native fixture runs on private Xvfb; Xorg reader behavior otherwise unchanged,"Live Niri 26.04 capture on 2026-10-31: workspace switch emits accessible-name then object:announcement with the workspace label about 10ms before window activation. Maintainer confirms workspace switching, AltTab announcements and the exit dialog read out with the built reader on Niri on 2026-10-31.","Maintainer live listening on Niri passed for workspace switching, AltTab and the exit dialog on 2026-10-31.",Braille flash follows the system-role output queue; physical display acceptance pending.,"Announcements bypass background-event filtering and foreground epochs because the provider explicitly requests speech and may never own focus (a compositor's own accessible tree under Niri via AccessKit). Screen-lock and secure-input gates still apply. Trimmed repeats from one provider inside 300ms are deduped; Niri alternates live-node identities and may append a space to force re-emission. There is no Orca workspace-specific behavior to mirror: Orca only presents the generic announcement. See docs/testing/announcements.md." web-message-article-focus,src/cthulhu/scripts/web/speech_generator.py;src/cthulhu/formatting.py,web,Announce named message articles when focus enters their controls,,internal/atspi/speech_metadata_native.go;internal/app/reading_context.go;internal/presentation/reading.go,in-progress,Speech replay covers browse and focus modes plus pending browser discovery; private Chromium article capture regression. See docs/testing/messaging.md.,Private Chromium fixture uses isolated X11 and D-Bus.,,Recorded speech only; live Discord listening pending.,Physical display acceptance pending.,Shared named article handling; no Discord identifiers or child scans. Preserve ordinary container context policy; announce article labels on entry without repeating them on exit. +web-disclosure-refresh,src/cthulhu/action_presenter.py; src/cthulhu/scripts/web/script_utilities.py,web,Discover and keep disclosure options open during Enter or Actions-list activation and caret navigation,,internal/atspi internal/app,implemented,"CTHULHU_BROWSER=/usr/bin/chromium CTHULHU_NATIVE_BROWSER_TESTS=actions,disclosure,reveal,positions,ticking,document python3 tests/native_browser.py",Private Chromium/Xorg fixture covers both activation paths; character and line reading; blur and document-scroll dismissal; offscreen text and images. Original public page also checked in a private Chromium profile.,,Original-page audible speech acceptance pending.,Physical Braille acceptance pending.,Preserve target generations and strict ownership; refresh action snapshots and ancestor order. Text caret placement reveals its range without an extra whole-object scroll that dismisses popups; non-text scrolling retained. User installed Brave acceptance pending. Link-sounds native Tab pan baseline fails before and after this fix. See docs/testing/browser-clickables.md. diff --git a/tests/browser-actions.html b/tests/browser-actions.html index 4fe6e27..79e39a7 100644 --- a/tests/browser-actions.html +++ b/tests/browser-actions.html @@ -9,7 +9,41 @@ Ordinary link

Destination

+
Choose category
+ +

After categories

+
+
+

Distant text

+
+Distant image