Found and fixed a bug where Cthulhu could get stuck in a loop sometimes on pages with tooltips.

This commit is contained in:
Storm Dragon committed 2026-10-20 18:33:29 -04:00
1 parent 90f1411405
commit 58e4b8aa15
6 files changed
+236 -2

No files matched your search

+7
View File
@@ -288,6 +288,13 @@ line. Left/Right, Ctrl+Left/Right, and Home/End review that label by character,
word, or boundary, including links with no native text or only a numeric badge. word, or boundary, including links with no native text or only a numeric badge.
The label's review position remains separate from the browser's text caret. The label's review position remains separate from the browser's text caret.
Browse arrows, word navigation and Read All skip non-focusable, non-editable
tooltips and their contents. This avoids repeatedly entering a tooltip that
disappears when native caret placement moves focus away from its owner.
Focusable or editable tooltip contents remain navigable. Verify the native
focus-triggered case with
`CTHULHU_NATIVE_BROWSER_TESTS=tooltips python3 tests/native_browser.py`.
Enter activates the current link or clickable, including explicitly named custom Enter activates the current link or clickable, including explicitly named custom
controls and clickable ancestors of ordinary text. Activation itself is silent controls and clickable ancestors of ordinary text. Activation itself is silent
whether it succeeds or fails; the page's own focus changes and live regions still whether it succeeds or fails; the page's own focus changes and live regions still
+117
View File
@@ -0,0 +1,117 @@
// Copyright (c) 2026 Storm Dragon
// Authors: Storm Dragon, Angus Kola
// SPDX-License-Identifier: GPL-3.0-or-later
package app
import (
"context"
"fmt"
"net/http"
"net/http/httptest"
"os"
"os/exec"
"testing"
"time"
"git.stormux.org/storm/cthulhu/internal/atspi"
"git.stormux.org/storm/cthulhu/internal/review"
)
func TestNativeBrowserTooltipNavigation(t *testing.T) {
if os.Getenv("CTHULHU_NATIVE_BROWSER") != "1" {
t.Skip("requires private browser session")
}
ctx, cancel := context.WithTimeout(context.Background(), 25*time.Second)
defer cancel()
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
fmt.Fprint(w, `<!doctype html><title>Tooltip navigation regression</title><style>li {list-style: none}</style>
<ul><li><a id="project" href="#after" aria-describedby="tip"
onfocus="document.getElementById('tip').hidden=false"
onblur="document.getElementById('tip').hidden=true">Example project</a>
<span id="tip" role="tooltip" hidden><span>Example project</span></span>
<p id="after">After project</p></li></ul>`)
}))
defer server.Close()
adapter := atspi.New()
if err := adapter.Start(ctx); err != nil {
t.Fatal(err)
}
defer adapter.Stop(context.Background())
command := func(args ...string) {
t.Helper()
if output, err := exec.CommandContext(ctx, "xdotool", args...).CombinedOutput(); err != nil {
t.Fatalf("xdotool: %v: %s", err, output)
}
}
command("key", "--clearmodifiers", "ctrl+l")
command("type", "--clearmodifiers", "--delay", "0", server.URL)
command("key", "Return")
var document atspi.ObjectID
for !document.Valid() {
select {
case event := <-adapter.Events():
if event.Type == atspi.DocumentLoadCompleteEvent && event.Source.Name == "Tooltip navigation regression" {
document = event.Source.ID()
}
case <-ctx.Done():
t.Fatal(ctx.Err())
}
}
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(document)
move := func(command review.DocumentCaretCommand, want string) {
t.Helper()
if err := worker.MoveDocumentCaret(command); err != nil {
t.Fatal(err)
}
select {
case result := <-worker.Results():
if result.Err != nil || result.CaretErr != nil || !result.Moved || result.DocumentCaretText != want {
t.Fatalf("%s: text=%q want=%q err=%v caret=%v diagnostics=%+v", command,
result.DocumentCaretText, want, result.Err, result.CaretErr, result.DocumentDiagnostics)
}
case <-ctx.Done():
t.Fatal(ctx.Err())
}
}
move(review.CaretNextLine, "Example project")
// Capture after focus has exposed the tooltip. This proves the fixture
// supplies a real native tooltip subtree, rather than merely hidden text.
for {
tree, err := adapter.CaptureDocument(ctx, document)
if err != nil {
t.Fatal(err)
}
ref, _ := tree.Reference(document)
doc, err := tree.Document(ctx, ref)
found := false
if err == nil {
for _, node := range doc.Nodes {
if node.Snapshot.Role == "tool tip" && !node.Hidden && len(node.Children) > 0 {
found = true
}
}
}
if found {
break
}
select {
case <-adapter.Events():
case <-ctx.Done():
t.Fatal("visible native tooltip missing: ", ctx.Err())
}
}
move(review.CaretNextLine, "After project")
move(review.CaretPreviousLine, "Example project")
move(review.CaretLineEnd, "t")
move(review.CaretNextCharacter, "A")
move(review.CaretPreviousCharacter, "t")
}
+9 -1
View File
@@ -34,6 +34,14 @@ func wholeCaretNode(snapshot atspi.Snapshot) bool {
return false return false
} }
// Passive tooltips appear when their owner receives focus and disappear when
// caret placement moves focus away. Entering their text can strand navigation
// on a removed object and repeatedly recover to the owner's container. Match
// Orca's web caret policy while retaining focusable/editable tooltip content.
func passive_caret_tooltip(snapshot atspi.Snapshot) bool {
return snapshot.Role == "tool tip" && !snapshot.Focusable && !snapshot.Editable
}
// caret_node_indices preserves provider order and treats whole controls as one // caret_node_indices preserves provider order and treats whole controls as one
// stop. MoveCaret holds the document cursor lock while using these indices. // stop. MoveCaret holds the document cursor lock while using these indices.
func caret_node_indices(nodes []atspi.DocumentNode, object atspi.ObjectReference) ([]int, int) { func caret_node_indices(nodes []atspi.DocumentNode, object atspi.ObjectReference) ([]int, int) {
@@ -73,7 +81,7 @@ func caret_node_indices(nodes []atspi.DocumentNode, object atspi.ObjectReference
if node.Hidden || skipped[node.Reference.ID()] { if node.Hidden || skipped[node.Reference.ID()] {
continue continue
} }
if snapshot.Role == "password text" { if snapshot.Role == "password text" || passive_caret_tooltip(snapshot) {
skipChildren(node.Children) skipChildren(node.Children)
continue continue
} }
+1 -1
View File
@@ -31,7 +31,7 @@ func caretNodeRanges(nodes []atspi.DocumentNode) []caretRange {
var visit func(int) var visit func(int)
visit = func(index int) { visit = func(index int) {
node := nodes[index] node := nodes[index]
if node.Hidden || node.Snapshot.Role == "password text" { if node.Hidden || node.Snapshot.Role == "password text" || passive_caret_tooltip(node.Snapshot) {
return return
} }
children := node.Snapshot.TextChildren children := node.Snapshot.TextChildren
+101
View File
@@ -0,0 +1,101 @@
// Copyright (c) 2026 Storm Dragon
// Authors: Storm Dragon, Angus Kola
// SPDX-License-Identifier: GPL-3.0-or-later
package review
import (
"context"
"testing"
"time"
"git.stormux.org/storm/cthulhu/internal/atspi"
)
func tooltip_cursor(t *testing.T, focusable, editable, hypertext bool) (*DocumentCursor, caretTextReader) {
t.Helper()
reader := caretTextReader{webID("/before"): "Project", webID("/tiptext"): "Project", webID("/after"): "After"}
node := func(path, role string, children ...atspi.ObjectID) atspi.TreeNode {
return atspi.TreeNode{Snapshot: atspi.Snapshot{BusName: webID(path).BusName, Path: path,
Role: role, StatesKnown: true, CharacterCount: len(reader[webID(path)])}, Children: children}
}
tip := node("/tip", "tool tip", webID("/tiptext"))
tip.Snapshot.Focusable, tip.Snapshot.Editable = focusable, editable
if hypertext {
// Text around an embedded child must not bypass subtree exclusion.
tip.Snapshot.CharacterCount = 3
tip.Snapshot.TextChildren = []atspi.TextChild{{Offset: 1, Child: webID("/tiptext")}}
reader[webID("/tip")] = "x\ufffcy"
}
tree, err := atspi.NewAccessibleTree(webID("/doc"), []atspi.TreeNode{
node("/doc", "document web", webID("/before"), webID("/tip"), webID("/after")),
node("/before", "static"), tip, node("/tiptext", "static"), node("/after", "static"),
})
if err != nil {
t.Fatal(err)
}
cursor, err := NewDocumentCursor(tree)
if err != nil {
t.Fatal(err)
}
if _, err := cursor.SyncFocus(context.Background(), webID("/before")); err != nil {
t.Fatal(err)
}
return cursor, reader
}
func TestDocumentCaretSkipsPassiveTooltip(t *testing.T) {
for _, hypertext := range []bool{false, true} {
for _, command := range []DocumentCaretCommand{CaretNextLine, CaretNextCharacter, CaretNextWord} {
t.Run(command.String(), func(t *testing.T) {
cursor, reader := tooltip_cursor(t, false, false, hypertext)
if command != CaretNextLine {
cursor.places[0].offset = len("Project") - 1
}
move, err := cursor.MoveCaret(context.Background(), reader, command)
if err != nil || !move.Moved || move.Position.Object.ID() != webID("/after") {
t.Fatalf("forward: %+v, %v", move, err)
}
reverse := CaretPreviousLine
if command == CaretNextCharacter {
reverse = CaretPreviousCharacter
} else if command == CaretNextWord {
reverse = CaretPreviousWord
}
cursor.places[0].offset = 0
move, err = cursor.MoveCaret(context.Background(), reader, reverse)
if err != nil || !move.Moved || move.Position.Object.ID() != webID("/before") {
t.Fatalf("backward: %+v, %v", move, err)
}
})
}
}
}
func TestDocumentCaretPreservesInteractiveTooltip(t *testing.T) {
for _, focusable := range []bool{false, true} {
cursor, reader := tooltip_cursor(t, focusable, !focusable, false)
move, err := cursor.MoveCaret(context.Background(), reader, CaretNextLine)
if err != nil || !move.Moved || move.Position.Object.ID() != webID("/tiptext") {
t.Fatalf("interactive tooltip: %+v, %v", move, err)
}
}
}
func TestReadAllSkipsPassiveTooltip(t *testing.T) {
cursor, reader := tooltip_cursor(t, false, false, true)
worker, err := NewWorker(&readAllCapturer{tree: cursor.tree, caretTextReader: reader}, time.Second)
if err != nil {
t.Fatal(err)
}
worker.SyncFocus(webID("/before"))
result := worker.readAll(context.Background(), cursor)
if result.Err != nil || len(result.ReadAllLines) != 2 {
t.Fatalf("Read All: %+v", result)
}
for index, want := range []string{"Project", "After"} {
if got := result.ReadAllLines[index].Contents[0].Text; got != want {
t.Fatalf("line %d = %q, want %q", index, got, want)
}
}
}
+1
View File
@@ -12,6 +12,7 @@ from native_test_session import run_private_session
BROWSER_PROOFS = { BROWSER_PROOFS = {
"tooltips": ("./internal/app", "^TestNativeBrowserTooltipNavigation$", 30, 35),
"menu": ("./internal/app", "^TestNativeBrowserMenuControls$", 30, 35), "menu": ("./internal/app", "^TestNativeBrowserMenuControls$", 30, 35),
"lists": ("./internal/app", "^TestNativeBrowserLists$", 35, 40, "browser-lists.html"), "lists": ("./internal/app", "^TestNativeBrowserLists$", 35, 40, "browser-lists.html"),
"pointer": ("./internal/atspi", "^TestNativePointerInput$", 15, 20), "pointer": ("./internal/atspi", "^TestNativePointerInput$", 15, 20),