Name unnamed clickables from descendant text on structural landings

internal/review/document.go

@@ -584,6 +584,19 @@ func documentPosition(doc atspi.DocumentSnapshot, place documentPlace) DocumentP
 			position.LinkedHeading = &link.Snapshot
 		}
 	}
+	// Unnamed clickables often expose their readable label only in static
+	// descendants. Name the returned snapshot copy from those descendants
+	// so a structural landing speaks more than the bare role; the captured
+	// tree and caret navigation into the descendants are unchanged.
+	if place.origin == StructuralNavigation && snapshot.SupportsClick && !snapshot.Editable &&
+		snapshot.Role != "password text" && !atspi.IsWebDocumentRole(snapshot.Role) &&
+		strings.TrimSpace(snapshot.Name) == "" && strings.TrimSpace(snapshot.Text) == "" {
+		if index, ok := doc.Index(place.object.ID()); ok {
+			if label := documentDescendantLabel(doc, index); label != "" {
+				position.Snapshot.Name = label
+			}
+		}
+	}
 	return position
 }
 

internal/review/document_lists.go

@@ -140,31 +140,11 @@ func (worker *Worker) listDocument(ctx context.Context, cursor *DocumentCursor,
 		if node.Hidden {
 			continue
 		}
-		label := node.Snapshot.Name
+		var childLabels []string
-		if node.Snapshot.Role != "password text" {
+		for _, child := range node.Children {
-			if label == "" {
+			childLabels = append(childLabels, labels[child])
-				label = node.Snapshot.Text
-			}
-			if label == "" {
-				var parts []string
-				size := 0
-				for _, child := range node.Children {
-					if part := labels[child]; part != "" {
-						parts = append(parts, part)
-						size += len(part)
-					}
-					if size >= 900 {
-						break
-					}
-				}
-				label = strings.Join(parts, " ")
-			}
 		}
-		runes := []rune(label)
+		labels[node.Reference.ID()] = documentNodeLabel(node.Snapshot, childLabels)
-		if len(runes) > 225 {
-			label = string(runes[:225])
-		}
-		labels[node.Reference.ID()] = label
 	}
 	parents := make(map[atspi.ObjectID]atspi.ObjectID, len(doc.Nodes))
 	indices := make(map[atspi.ObjectID]int)
@@ -188,6 +168,57 @@ func (worker *Worker) listDocument(ctx context.Context, cursor *DocumentCursor,
 	return list, captureContext.Err()
 }
 
+// documentNodeLabel is the bounded readable label for one captured node:
+// its own name, else its text, else its children's labels in provider
+// order. Password text never contributes its text or descendants.
+func documentNodeLabel(snapshot atspi.Snapshot, childLabels []string) string {
+	label := snapshot.Name
+	if snapshot.Role != "password text" {
+		if label == "" {
+			label = snapshot.Text
+		}
+		if label == "" {
+			var parts []string
+			size := 0
+			for _, part := range childLabels {
+				if part != "" {
+					parts = append(parts, part)
+					size += len(part)
+				}
+				if size >= 900 {
+					break
+				}
+			}
+			label = strings.Join(parts, " ")
+		}
+	}
+	runes := []rune(label)
+	if len(runes) > 225 {
+		label = string(runes[:225])
+	}
+	return label
+}
+
+// documentDescendantLabel assembles the label for one node's subtree using
+// the same policy as document lists, without labeling the whole document.
+// Hidden subtrees contribute nothing.
+func documentDescendantLabel(doc atspi.DocumentSnapshot, index int) string {
+	end := doc.SubtreeEnd(index)
+	labels := make(map[atspi.ObjectID]string, end-index+1)
+	for i := end; i >= index; i-- {
+		node := doc.Nodes[i]
+		if node.Hidden {
+			continue
+		}
+		var childLabels []string
+		for _, child := range node.Children {
+			childLabels = append(childLabels, labels[child])
+		}
+		labels[node.Reference.ID()] = documentNodeLabel(node.Snapshot, childLabels)
+	}
+	return labels[doc.Nodes[index].Reference.ID()]
+}
+
 func (worker *Worker) currentDocumentList(list DocumentListSnapshot) bool {
 	worker.stateMutex.Lock()
 	defer worker.stateMutex.Unlock()

internal/review/document_lists_test.go

@@ -112,6 +112,57 @@ func TestDocumentListDescendantLabelsExcludeHiddenAndPasswordText(t *testing.T)
 	}
 }
 
+func TestDocumentClickableMoveNamesUnnamedSectionFromDescendants(t *testing.T) {
+	node := func(path, role, text string, children ...string) atspi.TreeNode {
+		snapshot := atspi.Snapshot{BusName: webID(path).BusName, Path: path, Role: role, Text: text}
+		entry := atspi.TreeNode{Snapshot: snapshot}
+		for _, child := range children {
+			entry.Children = append(entry.Children, webID(child))
+		}
+		return entry
+	}
+	nodes := []atspi.TreeNode{
+		node("/doc", "document web", "", "/start", "/clickable"),
+		node("/start", "static", "Before"),
+		node("/clickable", "section", "", "/text", "/hidden", "/password"),
+		node("/text", "static", "Clickable text"),
+		node("/hidden", "static", "Hidden content"),
+		node("/password", "password text", "private fixture password"),
+	}
+	nodes[2].Snapshot.SupportsClick = true
+	nodes[4].Geometry = atspi.Geometry{VisibilityKnown: true}
+	tree, err := atspi.NewAccessibleTree(webID("/doc"), nodes)
+	if err != nil {
+		t.Fatal(err)
+	}
+	cursor, err := NewDocumentCursor(tree)
+	if err != nil {
+		t.Fatal(err)
+	}
+	ctx := context.Background()
+	if _, err := cursor.SyncFocus(ctx, webID("/start")); err != nil {
+		t.Fatal(err)
+	}
+	move, err := cursor.Move(ctx, DocumentClickable, 0, false)
+	if err != nil || !move.Moved || move.Position.Object.ID() != webID("/clickable") {
+		t.Fatalf("clickable move: %+v %v", move, err)
+	}
+	if move.Position.Snapshot.Name != "Clickable text" {
+		t.Fatalf("landed snapshot name = %q, want descendant label", move.Position.Snapshot.Name)
+	}
+	ref, err := tree.Reference(webID("/clickable"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	captured, err := tree.Resolve(ref)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if captured.Name != "" {
+		t.Fatalf("captured snapshot mutated: name = %q", captured.Name)
+	}
+}
+
 func TestWorkerDocumentListUsesOrderedRequests(t *testing.T) {
 	tree, _ := webCursor(t)
 	worker, cancel := startWorker(t, &readAllCapturer{tree: tree}, time.Second)