Fix some issues with clickables on some websites.

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")
+}

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)

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