mirror of
https://github.com/priyanshujain/sanderling.git
synced 2026-10-02 19:17:10 +00:00
fix(web-runtime): answer clickable for an element reached through ax
The handle hardcoded true, so every text node and container a spec reached through state.ax claimed to be a tap target while the enumeration and the hierarchy dump both resolved it through the tappable selector. The parity test now compares the handle against the enumeration element by element in a real browser, which is where the three answers have to agree. Claude-Session: https://claude.ai/code/session_01A5KmftdEJ49A9z5mF5ESrX
This commit is contained in:
1 parent
9b9c7cf6e2
commit
7066b23a22
4 files changed
+128
-22
No files matched your search
@@ -45,12 +45,17 @@ import (
|
||||
// elementFacts is one element as a producer reports it: the tag, for readable
|
||||
// failures, and every fact acceptsTarget consults.
|
||||
type elementFacts struct {
|
||||
tag string
|
||||
clickable bool
|
||||
enabled bool
|
||||
editable bool
|
||||
scrollable bool
|
||||
positiveBounds bool
|
||||
tag string
|
||||
clickable bool
|
||||
enabled bool
|
||||
editable bool
|
||||
scrollable bool
|
||||
hintText string
|
||||
// handleClickable is the clickability of the ax element a spec reaches
|
||||
// through state.ax.find, a third place the fact is computed and the one that
|
||||
// answered a hardcoded true while the other two resolved a selector.
|
||||
handleClickable bool
|
||||
positiveBounds bool
|
||||
}
|
||||
|
||||
// factRow pairs an element's facts with the id both producers key on, kept in
|
||||
@@ -91,6 +96,7 @@ func TestHierarchy_DerivesTheSameFactsAsTheWebRuntime(t *testing.T) {
|
||||
requireEveryElementNamed(t, "the hierarchy dump", fromDump)
|
||||
requireEveryElementNamed(t, "the web runtime", fromWebRuntime)
|
||||
requireBothPolarities(t, fromWebRuntime)
|
||||
requireTheHandleAgreesWithTheEnumeration(t, fromWebRuntime)
|
||||
compareEnumeratedElements(t, fromDump, fromWebRuntime)
|
||||
compareDerivedFacts(t, fromDump, fromWebRuntime)
|
||||
})
|
||||
@@ -116,6 +122,7 @@ func factsFromHierarchyDump(t *testing.T, dump string) []factRow {
|
||||
enabled: element.Enabled,
|
||||
editable: element.Editable,
|
||||
scrollable: element.Attributes["scrollable"] == "true",
|
||||
hintText: element.Attributes["hintText"],
|
||||
positiveBounds: hasPositiveBounds(
|
||||
element.Bounds.Width(),
|
||||
element.Bounds.Height(),
|
||||
@@ -155,14 +162,16 @@ func factsFromWebRuntime(
|
||||
t.Fatalf("read web runtime facts: %v", err)
|
||||
}
|
||||
var wire []struct {
|
||||
ID string `json:"id"`
|
||||
Tag string `json:"tag"`
|
||||
Clickable bool `json:"clickable"`
|
||||
Enabled bool `json:"enabled"`
|
||||
Editable bool `json:"editable"`
|
||||
Scrollable bool `json:"scrollable"`
|
||||
Width int `json:"width"`
|
||||
Height int `json:"height"`
|
||||
ID string `json:"id"`
|
||||
Tag string `json:"tag"`
|
||||
Clickable bool `json:"clickable"`
|
||||
Enabled bool `json:"enabled"`
|
||||
Editable bool `json:"editable"`
|
||||
Scrollable bool `json:"scrollable"`
|
||||
HintText string `json:"hintText"`
|
||||
HandleClickable bool `json:"handleClickable"`
|
||||
Width int `json:"width"`
|
||||
Height int `json:"height"`
|
||||
}
|
||||
if err := json.Unmarshal([]byte(encoded), &wire); err != nil {
|
||||
t.Fatalf("decode web runtime facts: %v", err)
|
||||
@@ -172,12 +181,14 @@ func factsFromWebRuntime(
|
||||
rows = append(rows, factRow{
|
||||
id: item.ID,
|
||||
facts: elementFacts{
|
||||
tag: item.Tag,
|
||||
clickable: item.Clickable,
|
||||
enabled: item.Enabled,
|
||||
editable: item.Editable,
|
||||
scrollable: item.Scrollable,
|
||||
positiveBounds: hasPositiveBounds(item.Width, item.Height),
|
||||
tag: item.Tag,
|
||||
clickable: item.Clickable,
|
||||
enabled: item.Enabled,
|
||||
editable: item.Editable,
|
||||
scrollable: item.Scrollable,
|
||||
hintText: item.HintText,
|
||||
handleClickable: item.HandleClickable,
|
||||
positiveBounds: hasPositiveBounds(item.Width, item.Height),
|
||||
},
|
||||
})
|
||||
}
|
||||
@@ -222,6 +233,7 @@ func requireBothPolarities(t *testing.T, rows []factRow) {
|
||||
{"enabled", func(f elementFacts) bool { return f.enabled }},
|
||||
{"editable", func(f elementFacts) bool { return f.editable }},
|
||||
{"scrollable", func(f elementFacts) bool { return f.scrollable }},
|
||||
{"hintText", func(f elementFacts) bool { return f.hintText != "" }},
|
||||
{"positiveBounds", func(f elementFacts) bool { return f.positiveBounds }},
|
||||
} {
|
||||
var sawTrue, sawFalse bool
|
||||
@@ -244,6 +256,30 @@ func requireBothPolarities(t *testing.T, rows []factRow) {
|
||||
}
|
||||
}
|
||||
|
||||
// requireTheHandleAgreesWithTheEnumeration compares the V8 host against itself.
|
||||
// An element a spec reaches through state.ax and the same element in the
|
||||
// enumeration must be clickable to the same degree, or a spec taps a container
|
||||
// the picker calls inert. The handle resolved the fact by element.matches over
|
||||
// the tappable selector while the enumeration resolved it by membership of the
|
||||
// set that selector queried, and this is where those two answers are held
|
||||
// together over a real page: an [onclick] attribute, an onclick property that is
|
||||
// not one, elements inside a shadow root.
|
||||
func requireTheHandleAgreesWithTheEnumeration(t *testing.T, rows []factRow) {
|
||||
t.Helper()
|
||||
for _, row := range rows {
|
||||
if row.facts.handleClickable != row.facts.clickable {
|
||||
t.Errorf(
|
||||
"%q (<%s>): the ax handle reports clickable=%v, the enumeration "+
|
||||
"reports clickable=%v",
|
||||
row.id,
|
||||
row.facts.tag,
|
||||
row.facts.handleClickable,
|
||||
row.facts.clickable,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// compareEnumeratedElements is the check that the two producers walk the same
|
||||
// document. It is what notices a producer that roots at body and never sees
|
||||
// `html`, or one that enumerates the head subtree the other drops.
|
||||
@@ -307,6 +343,16 @@ func compareDerivedFacts(t *testing.T, fromDump, fromWebRuntime []factRow) {
|
||||
web.tag,
|
||||
)
|
||||
}
|
||||
if dump.hintText != web.hintText {
|
||||
t.Errorf(
|
||||
"%q (<%s>): the hierarchy dump names the field %q, the web runtime "+
|
||||
"names it %q; the model is shown a different control on each host",
|
||||
row.id,
|
||||
dump.tag,
|
||||
dump.hintText,
|
||||
web.hintText,
|
||||
)
|
||||
}
|
||||
for _, fact := range []struct {
|
||||
name string
|
||||
dump bool
|
||||
|
||||
@@ -488,7 +488,10 @@ function elementHandle(element: Element, selector: unknown): Record<string, unkn
|
||||
text,
|
||||
desc: ariaLabel,
|
||||
class: (element as HTMLElement).className ?? "",
|
||||
clickable: true,
|
||||
// The selector collectTargets and the hierarchy dump (driver.go) both
|
||||
// resolve clickable through. Hardcoded true here, every text node and
|
||||
// container a spec reached through state.ax claimed to be a tap target.
|
||||
clickable: element.matches(TAPPABLE_SELECTOR),
|
||||
enabled: isEnabled(element),
|
||||
editable: isEditableElement(element as HTMLElement),
|
||||
focused: document.activeElement === element,
|
||||
|
||||
@@ -11,11 +11,23 @@
|
||||
|
||||
import { __testing__ } from "../src/web-runtime.ts";
|
||||
|
||||
const { collectTargets, targetElements } = __testing__;
|
||||
const { collectTargets, targetElements, buildAx } = __testing__;
|
||||
|
||||
// The handle facts are read back off the shipped ax element, the way a spec
|
||||
// reaches them and the way internal/verifier/llm.go handleLabel reads them, so
|
||||
// the comparison runs over what the model is actually shown rather than over a
|
||||
// recomputation of it.
|
||||
type AxHandle = { attrs?: Record<string, string>; clickable?: boolean };
|
||||
type Ax = { find(selector: unknown): AxHandle | undefined };
|
||||
|
||||
function handleOf(ax: Ax, id: string): AxHandle | undefined {
|
||||
return id ? ax.find({ id }) : undefined;
|
||||
}
|
||||
|
||||
function domFacts(): unknown[] {
|
||||
const elements = targetElements();
|
||||
const facts = collectTargets();
|
||||
const ax = buildAx() as Ax;
|
||||
if (elements.length !== facts.length) {
|
||||
throw new Error(
|
||||
`collectTargets reported ${facts.length} targets over ${elements.length} elements`,
|
||||
@@ -23,6 +35,7 @@ function domFacts(): unknown[] {
|
||||
}
|
||||
return elements.map((element, index) => {
|
||||
const target = facts[index]!;
|
||||
const handle = handleOf(ax, element.id);
|
||||
return {
|
||||
id: element.id,
|
||||
tag: element.tagName.toLowerCase(),
|
||||
@@ -30,6 +43,8 @@ function domFacts(): unknown[] {
|
||||
enabled: target.enabled,
|
||||
editable: target.editable,
|
||||
scrollable: target.scrollable,
|
||||
hintText: handle?.attrs?.hintText ?? "",
|
||||
handleClickable: handle?.clickable ?? false,
|
||||
width: target.width ?? 0,
|
||||
height: target.height ?? 0,
|
||||
};
|
||||
|
||||
@@ -555,10 +555,28 @@ function domElement(spec: {
|
||||
attributes: Object.entries(attributes).map(([name, value]) => ({ name, value })),
|
||||
labels: (spec.labels ?? []).map((textContent) => ({ textContent })),
|
||||
getAttribute: (name: string) => attributes[name] ?? null,
|
||||
matches: (selector: string) => matchesAnyPart(selector, spec.tag, attributes),
|
||||
getBoundingClientRect: () => ({ left: 0, top: 0, right: 40, bottom: 20, width: 40, height: 20 }),
|
||||
};
|
||||
}
|
||||
|
||||
// matchesAnyPart answers a comma-joined list of tag and attribute selectors over
|
||||
// the fake's own tag and attributes, so the production selector string is what
|
||||
// gets evaluated here and a role added to it is covered without teaching this
|
||||
// harness about it.
|
||||
function matchesAnyPart(
|
||||
selector: string,
|
||||
tag: string,
|
||||
attributes: Record<string, string>,
|
||||
): boolean {
|
||||
return selector.split(",").some((part) => {
|
||||
const attribute = /^\[([^\]=]+)(?:="([^"]*)")?\]$/.exec(part.trim());
|
||||
if (!attribute) return part.trim() === tag;
|
||||
const value = attributes[attribute[1]!];
|
||||
return value !== undefined && (attribute[2] === undefined || value === attribute[2]);
|
||||
});
|
||||
}
|
||||
|
||||
function handleOf(element: unknown): Record<string, unknown> {
|
||||
const global = globalThis as Record<string, unknown>;
|
||||
const original = global.document;
|
||||
@@ -627,6 +645,28 @@ test("an unlabelled field's hintText falls back to aria-label, placeholder, then
|
||||
assert.equal(attrsOf(domElement({ tag: "input", attributes: { name: "note" } })).hintText, "note");
|
||||
});
|
||||
|
||||
// clickable was hardcoded true here while the enumeration and the hierarchy dump
|
||||
// both resolved it through TAPPABLE_SELECTOR, so every text node and container a
|
||||
// spec reached through state.ax claimed to be a tap target on one host only.
|
||||
test("an element reached through ax reports the tappable selector's clickability", () => {
|
||||
const clickabilityOf = (element: unknown) => handleOf(element).clickable;
|
||||
assert.equal(clickabilityOf(domElement({ tag: "button", attributes: { id: "submit" } })), true);
|
||||
assert.equal(clickabilityOf(domElement({ tag: "input", attributes: { id: "amount" } })), true);
|
||||
assert.equal(
|
||||
clickabilityOf(domElement({ tag: "div", attributes: { id: "row", role: "option" } })),
|
||||
true,
|
||||
);
|
||||
assert.equal(
|
||||
clickabilityOf(domElement({ tag: "div", attributes: { id: "click", onclick: "void 0" } })),
|
||||
true,
|
||||
);
|
||||
assert.equal(clickabilityOf(domElement({ tag: "div", attributes: { id: "balance" } })), false);
|
||||
assert.equal(
|
||||
clickabilityOf(domElement({ tag: "span", attributes: { id: "note", role: "presentation" } })),
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
test("a non-editable element carries no hintText", () => {
|
||||
const element = domElement({ tag: "button", attributes: { id: "txn-submit" }, text: "Add credit" });
|
||||
assert.equal(attrsOf(element).hintText, undefined);
|
||||
@@ -664,6 +704,7 @@ test("ax.findAll resolves a selector path segment by segment", () => {
|
||||
textContent: id,
|
||||
dataset: {},
|
||||
getAttribute: () => null,
|
||||
matches: (selector: string) => matchesAnyPart(selector, "div", {}),
|
||||
getBoundingClientRect: () => rect,
|
||||
querySelectorAll: (selector: string) => answers[selector] ?? [],
|
||||
});
|
||||
@@ -704,6 +745,7 @@ test("ax.find and ax.findAll label the element with its selector", () => {
|
||||
textContent: "Submit",
|
||||
dataset: {},
|
||||
getAttribute: () => null,
|
||||
matches: (selector: string) => matchesAnyPart(selector, "div", {}),
|
||||
getBoundingClientRect: () => rect,
|
||||
};
|
||||
const matches = `:is([data-testid="TxnSubmit"], [id="TxnSubmit"])`;
|
||||
|
||||
Reference in new issue
Block a user