Skip to content

[js][py][rb][java][dotnet] add the accessibility snapshot atom and the binding methods - #18232

Open
AutomatedTester wants to merge 85 commits into
trunkfrom
claude/ecstatic-bardeen-6l95xe
Open

AutomatedTester wants to merge 85 commits into
trunkfrom
claude/ecstatic-bardeen-6l95xe

Conversation

@AutomatedTester

@AutomatedTester AutomatedTester commented Oct 23, 2026 •

Copy link
Copy Markdown
Member

🔗 Related Issues

  • Implements the in-page atom of the ADR proposed in SeleniumHQ/selenium#18205 (docs/decisions/18205-accessibility-tree-snapshot.md), the shared corpus the record makes the cross-binding contract, and the binding adoptions (Python, JavaScript, Ruby, Java, .NET). The ADR is still Proposed; this PR implements it as written there, and the open decisions listed under Additional Considerations are the ones to settle on that record or here.
  • User requests the ADR cites: SeleniumHQ/selenium#15969, SeleniumHQ/selenium#16135.

💥 What does this PR do?

Adds the in-page JavaScript that computes an accessibility snapshot, as a TypeScript atom in @selenium/atoms, plus the shared test corpus, and the Python, JavaScript, Ruby, Java and .NET accessibility_snapshot() / getAccessibilitySnapshot() / accessibility_snapshot / GetAccessibilitySnapshotAsync() methods on top of it. One commit per slice, each with its tests written before the implementation:

Scaffold (4a4bf54)

  • Registry entry accessibility-snapshot in typescript/defs.bzl, exported from the barrel as accessibilitySnapshot and published under @selenium/atoms/accessibility-snapshot. package.json was regenerated with typescript/generate-exports.cjs, so //javascript/atoms:exports-up-to-date keeps it honest.
  • typescript/accessibility-snapshot.ts (re-export only) plus core/accessibility/types.ts (the ADR §2 AccessibilityNode shape, also exported as a type) and core/accessibility/snapshot.ts.
  • Fragment //javascript/atoms/fragments:accessibility-snapshot-typescript with the same visibility as the other TypeScript atoms the bindings embed.

Roles (bac5a29)

  • core/accessibility/roles.ts: the explicit role attribute (first token naming a WAI-ARIA 1.2 role authors may use, matched ASCII case-insensitively, abstract and unknown tokens fall through; none/presentation ignored on elements that can take focus and ones with a global ARIA attribute, per WAI-ARIA 1.2 §9.1), then the html-aam implicit mapping including its contextual rows (header/footer outside sectioning content, aside, section and form only when named, li only inside a list, select by multiple/size, input by type and list, td as gridcell inside a grid, th by scope, img alt="" as presentation, required owned elements of a presentational table or list as generic).
  • accessibility_roles_typescript_test.html: 35 tests, 138 assertions.

Hidden-ness (0b99154)

  • core/accessibility/hidden.ts: isHidden in the accname sense. aria-hidden=true on the element or an ancestor (false cannot undo it), inert, display: none, visibility: hidden/collapse on the element's own computed style, content-visibility: hidden for everything below the element, the hidden attribute via the UA stylesheet (so author CSS can show it again), closed details content other than the first summary, noscript, template content and detached elements. An area is as hidden as the image whose usemap points at its map. Ancestry follows the flat tree.
  • hiddenState reports what an element does on its own (rendered, self, contents, subtree) so the walk can prune once and still descend into a visibility-hidden element whose children may be visible.
  • accessibility_hidden_typescript_test.html: 24 tests, 65 assertions, including the cases that pin this as not bot.dom.isShown.

Tree walk (3b8f4dd)

  • core/accessibility/tree.ts: each element is visited once, top-down, with ancestor state threaded down. An element produces a node when its role is not generic, none or presentation or when it has a name; pruned elements hoist their children in place. Subtree-hidden elements are dropped; a visibility-hidden element drops its own node and text but descends; a contents-hidden element keeps its node and drops its children; closed details content is skipped. Text runs become text leaves with collapsed whitespace and no element key. Frames, textareas, video and audio are leaves by element; roles with presentational children are leaves by role.
  • The root always produces a node; a root whose subtree is hidden, by itself or by an ancestor, has no children.
  • core/accessibility/context.ts: a per-call SnapshotContext, alive exactly as long as the call because the snapshot is a point-in-time copy.
  • accessibility_tree_typescript_test.html: 30 tests, 60 assertions.

Names and descriptions (cc70c59)

  • core/accessibility/accname.ts: the accname 1.2 text alternative computation with the html-aam element rules (aria-labelledby, aria-label, embedded control values, native sources, name from content with generated content and block spacing, title, placeholder); descriptions from aria-describedby, aria-description, then title when the name did not use it. core/accessibility/value.ts holds the embedded-control values.
  • A generic element with a name produces a node, and text an ancestor's name was computed from is not repeated as leaves below it while the element structure there still is.
  • accessibility_name_typescript_test.html (now 38 tests) and accessibility_description_typescript_test.html (7 tests, 12 assertions).

Review fixes (6c5e36b): the Qodo findings on the slices above, each with a regression case added first; the two not changed are answered on their threads.

States and values (1f42b8c)

  • core/accessibility/states.ts: checked, pressed, expanded, selected, level, disabled, focused per the ADR §2 table (see Implementation Notes for the two readings of "whenever the role supports it"); value for textbox, searchbox, combobox and range roles. README.npm.md documents the atom.
  • accessibility_states_typescript_test.html (18 tests, 61 assertions) and accessibility_value_typescript_test.html (6 tests, 24 assertions).

Composed DOM and aria-owns (3014e9a)

  • The walk and name-from-content follow the flat tree (shadow roots, slots, fallback, unslotted children never rendered). aria-owns re-parents: owned elements leave their DOM position and follow the owner's own children, so accessibility parentage wins (ADR §3) while the element reference still identifies the element.
  • accessibility_shadow_typescript_test.html (10 tests) and accessibility_owns_typescript_test.html (9 tests).

Shared corpus (80d61c4)

  • common/src/web/accessibility/: 16 plain pages covering the areas above, each with the whole-document snapshot recorded as .expected.json (element references written as ids) and as .expected.txt in the ADR §1 text form; index.json lists them and README.md gives the conventions, how to record, and where engine divergences go. This corpus, not any binding's tests, is the cross-binding contract (ADR §4).
  • typescript/testing/accessibility-text.ts: the reference toText (- role "name" [state] [state=value]: value, two-space indent, - text: content leaves), in the test-only bundle so it is never published; the bindings port it and the corpus .txt files hold the ports to one source.
  • typescript/accessibility-node.schema.json: the ADR §2 shape as JSON Schema with the states table spelled out.
  • accessibility_corpus_typescript_test.html loads each page in an iframe and compares tree and text with the expectation, preferring an engine-specific file when one is recorded; ?record=1 shows the expectations to record instead.
  • Two Node tests with no browser: //javascript/atoms:accessibility-unit-tests (the renderer, and a 64 KiB ceiling on the atom bundle) and //javascript/atoms:accessibility-corpus-consistent (every page has both expectation files, every expectation validates against the schema, every .txt is the renderer's output over its .json, ids are unique, the root is the html element). The second is what turns the ADR's compatibility rule into a failing test.

Hardening (961f924)

  • accessibility_robustness_typescript_test.html: a 5000-node subtree, a 1000-level deep tree, 500 cells labelled by one header, an owner of 500 elements, references to nothing, odd role tokens, duplicate ids, non-ASCII text, out-of-range aria-level, and two snapshots of one tree being identical. Interference from page scripts is deliberately not tested: in the same realm the page's scripts run first and can replace any prototype the atom relies on; the bindings install the atom in a BiDi sandbox realm, which is the design's answer (ADR §4).

Performance (6764d82 to d387d80, six commits)

  • test/accessibility_bench.html (not a test: the suite runs only *_test.html) times the atom on fixed tree shapes and counts the DOM calls it makes, as the before/after for this and for the bindings; README.md records how to run it and the baseline.

  • Five per-call caches and one threaded flag, each committed with its own before/after numbers and each gated on every page, the corpus and the Node tests passing unchanged: hidden state cached in the context and read through the shared style cache; generated content (::before/::after) read once per element and skipped entirely when no stylesheet in the element's scope (adopted sheets, :host::before and ::slotted()::before included) can define any, with an unreadable sheet counting as defining it; the text alternative of an element as a descendant of something being named, and its name-from-content text, cached for the standard traversal; the text alternative of an aria-labelledby/aria-describedby target cached per target; and aria-disabled threaded down the walk instead of climbing every ancestor of every node.

  • Measured in headless Chromium 141 (bench page, times vary about 20% between runs, the call counts are the reliable columns), baseline → after:

    shape µs / element getComputedStyle / element getAttribute / element
    list 8000 8 → 8 1.8 → 1.0 10.8 → 9.3
    table 1000×10 27 → 13 6.7 → 1.0 22.5 → 14.8
    headings 4000 17 → 12 5.7 → 1.0 17.7 → 13.0
    document-like 1000 15 → 12 2.5 → 1.0 14.7 → 11.2
    fan-in 2000 28 → 8 8.5 → 1.0 24.5 → 13.0
  • The robustness page's fan-in test used to claim the shared header was "resolved once per snapshot" when it was not; it now holds a probe whose attribute reads are counted, and the claim is true.

  • Two findings of the review pinned as limitations: UA quotes on q are the keyword open-quote in computed content, not a string, so the atom never read them (names page); and a descendant under a single content-named ancestor is computed once as a descendant and once at top level, two modes no cache can merge.

Code review fixes (711e817 to 0bfdc6d, nine commits), each with its regression test added first and all recorded expectations unchanged:

  • aria-owns: two siblings owning each other were both claimed and both vanished; an owner with presentational children (a button owning a menu) claimed a target it could never emit; an owned element inside a hidden container was placed as rendered. An element is now owned by the first owner in document order that can take it, a relation that would make the tree circular or whose owner has presentational children is dropped, and owned elements are checked against their DOM ancestors' hidden state.
  • Generated content: Chromium serialises content: url(...) with the URL quoted, so icon pseudo-elements named buttons by their image URL; functions are now read as one token. A display: none or visibility: hidden pseudo-element contributes nothing. ::part()::before from the host's scope is now consulted by the gate that skips pseudo-element reads.
  • aria-labelledby: the visited set was shared by a whole name computation, so two cells of a row pointing at one header contributed it once; it is now per attribute (an id listed twice still counts once, as Chromium reads it).
  • focused inside shadow roots: the document's activeElement is the host; the element's own root is asked now, and a host whose open shadow root holds the focus is not itself focused.
  • Submit, reset and button inputs take their label elements ahead of value (html-aam); type=number takes a placeholder.
  • header/footer are generic inside an element with the article, complementary, main, navigation or region role, not only inside those elements.
  • Text form: a value containing a line break broke the one-node-per-line grammar; the renderer escapes \n, \r and \ in values, and aria-valuetext is trimmed like aria-valuenow.
  • .gitattributes forces LF on the corpus .txt/.json files so the byte-exact consistency test holds on a CRLF checkout.
  • Owner roles (a cell's table, an li's list) now go through the context's role cache: the table of a 1000×10 grid had its role attribute read 2202 times per snapshot, now once; attribute reads per element on the table shapes 14.8 → 12.9, fan-in 13.0 → 11.5.

CI fixes and the Qodo findings on the later slices (5e57918, 4b2709e to 291467d):

  • Build: the test bundles listed only the atom sources and their own entry as srcs, and esbuild under Bazel resolves imports only among the sources it is given, so the accessibility test bundle's import of its renderer failed and took the two Node tests and the three closure-test targets with it from the corpus commit on (the local esbuild stand-in resolved from disk and hid it). Every test-only module is now a source of every test bundle. The corpus consistency test took //common/src/web as data, source files from another package that rules_js refuses to copy into a js_test; the corpus now comes through a js_library in its own package. //common/src/web is written in the short form buildifier wants.
  • aria-owns owners are found along the flat tree, so an owner inside an open shadow root re-parents within its tree and a light-DOM owner no slot renders claims nothing (it used to take a rendered target out of the snapshot).
  • disabled: an ancestor's aria-disabled reaches the descendants that can take focus (WAI-ARIA 1.2), not the headings and paragraphs between them, and the ancestor check used for a root or owned element climbs through shadow hosts. aria-pressed, aria-expanded and the grid parts' aria-selected set to undefined or empty count as absent. An option element given another role does not report the native selection.
  • A password field's value is masked, one bullet per character, as the browser's own tree exposes it, both as the node's value and for what the field contributes to an enclosing name; corpus page 004 is re-recorded.
  • Only an HTML slot element is treated as a slot; an SVG element named slot no longer aborts the snapshot.
  • Pinned rather than fixed: a :host::before rule in a closed root's sheet names the host only while some sheet in scope defines generated content, because the gate that skips pseudo-element reads cannot see a closed root's sheets. One finding did not reproduce (b maps to generic, as the robustness test asserts; it passes in all three engines).

Python binding (b974453), the first binding slice:

  • driver.accessibility_snapshot() and element.accessibility_snapshot(), both returning AccessibilityNode (role, name, description, value, states, element, children) with to_text(), in selenium/webdriver/remote/accessibility_snapshot.py.
  • The atom is copied into the package like the other atoms (git-ignored like them) and run over BiDi: script.addPreloadScript installs it into the selenium sandbox realm of every new document, and a document that predates the preload installs it on first use in one callFunction. Results come back with maxDomDepth: 0, so every element is a node remote value whose sharedId becomes a WebElement.
  • Frames: each iframe node is filled in with the tree of its browsing context, found through the frame's contentWindow (a window remote value carries the context id), so cross-origin frames stitch too; a frame that cannot be reached stays a leaf. The whole-page snapshot is rooted at the top-level context of the current window whatever frame the driver has switched to.
  • A subtree root may live in any frame of the window, so its shared id is tried against each browsing context of the window until one owns it; when none does, or the atom reports the root detached, StaleElementReferenceException is raised. A session without BiDi raises WebDriverException naming BiDi, from the accessor both entry points go through.
  • Tests, written first: //py:unit holds to_text() to every corpus page's recorded text (the port is held to the reference renderer by the same files the atom is), the line grammar's escapes, the wire decoder and the preload retry; the BiDi suite snapshots every corpus page whole and compares tree and text (element references mapped to ids, an engine-specific expectation preferred where one exists), then covers the document root, references usable for further commands, subtree roots, frames (including a snapshot taken while switched into a frame, a subtree inside one and an iframe as the root), stale and detached roots, and a snapshot being a copy; the classic suite checks both methods raise. Locally, with the generated bidi package, pytest and ChromeDriver 141 against headless Chromium 141: 20 unit, 26 BiDi and 2 classic tests pass; ruff and mypy are clean.
  • One limitation worth knowing: an element reference inside a child frame is usable through classic commands only after the driver switches to that frame, since classic element commands resolve references in the current browsing context. BiDi-side use works everywhere, as the ADR says.

JavaScript binding (5d8aaff), the second binding slice, the same design in lib/accessibility_snapshot.js:

  • await driver.getAccessibilitySnapshot() and await element.getAccessibilitySnapshot(), both resolving to an AccessibilityNode with toText(); the class is exported from the package. The atom module built by lib/atoms/make-atoms-module.js (new accessibility_snapshot target, shipped in the npm package) is a function whose source is the function declaration script.callFunction takes, so nothing is stringified by hand. Preload into the selenium sandbox realm, on-demand install, maxDomDepth: 0, frame stitching through contentWindow, the per-context search for a subtree root, StaleElementReferenceError for stale and detached roots, and UnsupportedOperationError naming BiDi for a session without it, as in Python.
  • The three BiDi commands go over the driver's connection directly rather than through the generated domain modules: those validate every node remote value's attribute map against the schema and warned once per element, which a snapshot of a few hundred elements turned into a wall of console noise.
  • Tests, written first: small-tests holds toText() to every corpus page's recorded text, the line grammar's escapes, the wire decoder, the BiDi guard and the preload retry; a large test on the BiDi browsers (Chrome, Edge, Firefox) mirrors the Python suite over the corpus and the frame, stale, detached and copy cases. Locally, with the generated BiDi modules compiled from the pinned CDDL, mocha and ChromeDriver 141 against headless Chromium 141: 22 small and 26 large tests pass; eslint and prettier are clean.

Firefox and the binding review round (5786c38, c208a48, 60c5424, 3118f8f, 23731ef, f628e56)

  • Firefox resolves a shared id in any document of the browsing context group, not only the one that owns the node (its NodeCache.getNode compares group ids), so on Firefox a subtree snapshot rooted inside a child frame ran against the top document and CI failed on test_subtree_snapshot_inside_a_frame. The called script now checks root.ownerDocument === document and answers null for a root it does not own, which the bindings treat like an unknown node and move on to the next context; the inside-a-frame tests assert the root is the frame's body.
  • Qodo on the Python slice: an iframe taken as the root now carries its document, as one found below the root does; frame stitching and context discovery swallow only errors that say a frame or node is gone, and a failing browsingContext.getTree propagates; local_dev copies the Python atom. The unit collection finding did not reproduce (//py:unit carries //common/src/web, whose glob covers the corpus, and passed on RBE).
  • Qodo on the JavaScript slice: a frame with an explicit role (<iframe role="application">) was reported under that role and so never stitched; the atom now reports every frame with the ADR's iframe token whatever role attribute it carries (ARIA in HTML allows application, document, img and the presentational pair, and the embedded document is exposed in every case). The JavaScript browser suite names Chrome, Edge and Firefox, the browsers the test environment enables BiDi for, so a macOS run no longer tries Safari.
  • Lint: //javascript/selenium-webdriver:eslint-test checks that every static require resolves in its runfiles, and lib/accessibility_snapshot.js requires the built atom module directly where lib/http.js loads its atoms through a run-time path the rule cannot follow. The module is now in the lint target's data, and ignored by eslint like the generated BiDi modules, since prettier rejects a generated bundle line by line.

Ruby binding (bf7e235, 07d6cbf, 102af37), the third binding slice, the same design in lib/selenium/webdriver/common/accessibility_snapshot.rb:

  • driver.accessibility_snapshot and element.accessibility_snapshot, both returning an AccessibilityNode (role, name, description, value, states, element, children) with #to_text; the bridge holds the AccessibilitySnapshotter (@api private) that does the work. The atom is copied into the gem like the other atoms (//rb/lib/selenium/webdriver/atoms:accessibility-snapshot, read through Atoms#read_atom). Preload into the selenium sandbox realm, on-demand install, maxDomDepth: 0, frame stitching through contentWindow, the per-context search for a subtree root with the ownership check, StaleElementReferenceError for stale and detached roots, and UnsupportedOperationError naming web_socket_url for a session without BiDi, as in Python and JavaScript.
  • The three BiDi commands go over BiDi#send_cmd directly, as the hand-written BiDi::BrowsingContext does, so node remote values come back as plain hashes and the typed protocol layer's record validation never runs per element. The BiDi no such node error and the script's null answer for a root another document owns are raised and rescued as the snapshotter's own NotOwnedError (a WebDriverError); 07d6cbf had made the module require the BiDi error registry for Error::NoSuchNodeError instead, which broke every spec target that depends on the common library alone (no BiDi file in its runfiles) and the unit target's load order, so 102af37 keeps the class in common.
  • states keys are strings, as the wire and the corpus JSON spell them. Signatures in rb/sig cover the new class and the three new methods; steep reports no errors on the new file (two warnings about untyped empty literals, which the codebase does not annotate anywhere).
  • Tests, written first: the unit spec holds #to_text to every corpus page's recorded text, the line grammar's escapes and the wire decoder, checks the classic bridge raises, and runs the context search and the preload retry over a stand-in connection (the top-level document refuses the root, the child owns it; no owner is the stale error); //rb/spec/unit:unit now carries //common/src/web. The BiDi spec (//rb/spec/integration/selenium/webdriver/bidi:accessibility_snapshot-*-bidi, generated for Chrome, Edge and Firefox like the other BiDi-only specs) mirrors the Python suite over the corpus and the document root, element references, subtree roots, frames, stale and detached roots, a snapshot being a copy, and the classic guard through reset_driver!(web_socket_url: false). Corpus files are read as UTF-8 explicitly, since the suite's locale cannot be assumed. Locally, with the bundle and ChromeDriver 141 against headless Chromium 141: 24 unit and 27 integration examples pass; rubocop is clean.

Qodo on the atom after the Ruby slice (d5c5ed1, 3ef8abe)

  • html-aam gives a form element the form landmark only when it has an accessible name; the atom gave it to every form. An unnamed form is now generic (through the same hasAuthorName check section uses), so it is pruned and its controls take its place; corpus pages 001 and 004 are re-recorded, and all three bindings' corpus suites pass on the new expectations. The roles page pins a named, a titled and an unnamed form.
  • That check read only a referenced label's own text and aria-label, so a label holding only an image with alt named nothing; it now also counts aria-label, alt, title or value on the label or any element below it, the attributes accname reads for a referenced label's descendants. Pinned for a form and a section labelled by an element holding only an image.
  • A disabled control with a tabindex cannot take focus, but the focus check read the tabindex first and so refused the presentational role on such a button; native controls are now checked for being disabled before the tabindex is read.
  • Not changed, left open for the author: a hidden aria-owns owner takes its owned elements with it (pinned since the aria-owns slice), because the owner is the accessibility parent and a hidden parent hides its children; the other reading, dropping the relation for a pruned owner, would leave the element where the author moved it away from. The Python line-width finding did not reproduce (ruff format --check passes, as does the Format job).

Author review (e253d8a, dddfee4, f69b13b, 7944e7e)

  • Python: the BiDi guard moves out of the snapshotter into WebDriver._accessibility(), the accessor both entry points go through, as Ruby already guards in the bridge.
  • All three bindings set the preload flag only after script.addPreloadScript succeeds; it used to be set before the call, so a failed preload looked installed and was never tried again. Each binding has a unit test running the preload against a stand-in that fails until told otherwise. In JavaScript, where snapshots can overlap, the registration is one shared promise so two early snapshots do not each register a script.

Java binding (d7f96a1, 9390590, adb839d), the fourth binding slice, the same design in org.openqa.selenium.remote.AccessibilitySnapshotter:

  • WebDriver and WebElement gain default AccessibilityNode getAccessibilitySnapshot() throwing UnsupportedOperationException, the pattern getAriaRole() and getAccessibleName() set, so third-party implementations keep compiling. RemoteWebDriver and RemoteWebElement override it; the driver's package-private accessibility() accessor is the one place both entry points go through, and it throws UnsupportedCommandException naming options.enableBiDi() for a classic-only session before any command is sent.
  • org.openqa.selenium.AccessibilityNode is an immutable value in the core package (getRole(), getName(), getDescription(), getValue(), getStates(), getElement(), getChildren()) with toText(); states are Boolean, Long or String under their wire names, as in the other bindings.
  • The atom ships in the remote jar like getAttribute.js and isDisplayed.js (copy_file of the fragment, listed as a resource) and runs over the existing bidi.module.Script: preload into the selenium sandbox realm, on-demand install, maxDomDepth: 0, ResultOwnership.NONE, frame stitching through contentWindow (WindowProxyProperties.getBrowsingContext()), the per-context search for a subtree root with the ownership check (browsingContext.getTree through BiDi.send with a plain map result, since the remote package cannot depend on bidi/browsingcontext without a cycle), StaleElementReferenceException for stale and detached roots. Node remote values become RemoteWebElements through the driver's element converter, so they are usable for further commands like any found element. The BiDi no such node error, which arrives as a WebDriverException carrying the raw error JSON, is matched on that text; there is no typed exception for it in the Java BiDi layer yet.
  • Tests, written first: //java/test/org/openqa/selenium:SmallTests gains AccessibilityNodeTest, holding toText() to every corpus page's recorded text plus the line grammar's escapes, the indentation and the read-only views (the suite now takes //common/src/web as data, whose filegroup is now visible to the package, and //java/test/org/openqa/selenium/build for InProject); //java/test/org/openqa/selenium/remote:small-tests gains AccessibilitySnapshotUnitTest, a classic-only RemoteWebDriver over a recording executor that asserts both methods raise UnsupportedCommandException and send nothing; //java/test/org/openqa/selenium/bidi:AccessibilitySnapshotTest (and the -firefox, -edge, -chrome-beta, -firefox-beta variants) mirrors the Python suite over the corpus and the document root, element references, subtree roots, frames, stale and detached roots and a snapshot being a copy. Both small suites and the bidi suite now depend on junit-jupiter-params. Locally, the changed and new sources compiled with javac --release 11 against the released 4.51.0 jars, the two small test classes ran under the JUnit console launcher (21 tests), and a harness carrying the bidi test's assertions ran against ChromeDriver 141 and headless Chromium 141 (46 checks); google-java-format 1.37.0 was applied. 9390590 reorders one dependency the Format job's buildifier wanted sorted; adb839d makes //common/src/web visible to the core test package, which RBE's analysis required.

Qodo after the Java slice (189d055, e9e1a3d)

  • Chromium resolves an attr() in a pseudo-element's computed content to its string before serialising it, but Firefox keeps the function, so a button named by content: attr(data-label) lost that label there. The parser now reads the named attribute from the originating element for an attr() token (name only) and still treats url() and the other functions as not text; the names page pins a present and a missing attribute.
  • An area is as hidden as the image that uses its map, found with a document-wide query that every area of the map repeated; the context now caches the answer per map name, and the robustness page snapshots a map of 200 areas and counts one query.
  • Ruby: snapshot element references go through Remote::Bridge.element_class, as every other lookup does, so a configured custom element class applies to them; the unit spec snapshots with a custom class.

.NET binding (84859e7, 3cbbed5, 9868337), the fifth binding slice, the same design in OpenQA.Selenium.AccessibilitySnapshotter:

  • WebDriver.GetAccessibilitySnapshotAsync() and WebElement.GetAccessibilitySnapshotAsync(), both Task<AccessibilityNode> with a CancellationToken, as virtual methods on the classes rather than members of IWebDriver/IWebElement, the way ComputedAccessibleLabel and ComputedAccessibleRole are exposed, so third-party interface implementations keep compiling; the API is async because the .NET BiDi layer is. The driver's internal accessor is the one place both entry points go through, and it throws UnsupportedOperationException naming options.UseWebSocketUrl when the session was started without BiDi, before any command is sent.
  • OpenQA.Selenium.AccessibilityNode is an immutable sealed class (Role, Name, Description, Value, States as IReadOnlyDictionary<string, object> of bool, long or string, Element as IWebElement?, Children) with ToText().
  • The atom is embedded through generated_resource_utilities like the other three atoms (ResourceUtilities.AccessibilitySnapshotAtom). As first written the snapshotter opened its own BiDi connection with AsBiDiAsync() on the first snapshot and closed it with the driver; the review round below moves that connection into WebDriver. Over it: preload into the selenium sandbox realm (AddPreloadScriptOptions.Sandbox), on-demand install, MaxDomDepth = 0, ResultOwnership.None, frame stitching through contentWindow (WindowProxyRemoteValue.Value.Context), the per-context search for a subtree root with the ownership check (BrowsingContext.GetTreeAsync rooted at the window), StaleElementReferenceException for stale and detached roots. Node remote values become WebElements through the driver's WebElementFactory, so a configured custom element factory applies. The BiDi no such node error arrives as a BiDiException whose message starts with the error code and is matched on that text, as in Java.
  • Qodo on the slice (3cbbed5): disposal takes the connection lock, so a first snapshot still connecting when the driver is disposed cannot leave its connection open and a later snapshot fails with ObjectDisposedException rather than reconnecting; a failure while closing the connection is logged rather than thrown, so the driver's own session quit and executor disposal always run. 9868337 drops an unused using the Format job's dotnet format style removed.
  • Tests, written first: //dotnet/test/unit/webdriver:unit gains AccessibilityNodeUnitTests (the line grammar's escapes, indentation, read-only copies, argument checks) and AccessibilitySnapshotUnitTests (both methods raise UnsupportedOperationException on a session without BiDi; see the review round below for their current shape); per dotnet/TESTING.md the unit project stays off the filesystem, so the corpus text comparison lives in the browser suite. //dotnet/test/webdriver:AccessibilitySnapshotTests (and its browser variants) mirrors the Python suite over the corpus (tree compared with JsonNode.DeepEquals, text byte for byte, an engine-specific expectation preferred where one exists) and the document root, element references, subtree roots, frames, stale and detached roots and a snapshot being a copy. Locally, with a .NET 10 SDK: the WebDriver sources (minus the generated DevTools protocol, which needs Bazel) compiled with the new files, the unit fixtures ran under NUnit, the browser fixture compiled against stand-ins for the test environment, a harness carrying its assertions ran against ChromeDriver 141 and headless Chromium 141 (46 checks), and dotnet format whitespace and style report no changes.

Wire shape of the atom (ab4dbc8; then 7c8e434 Python, ffb506a JavaScript, 7664cc3 Ruby, fa48ac2 Java, dbb6be9 .NET; e306305 on Firefox), the simplification round that moves binding logic into the atom:

  • A second atom entry, accessibility-snapshot-wire (core/accessibility/wire.ts, exported as accessibilitySnapshotWire, fragment accessibility-snapshot-wire-typescript, published under @selenium/atoms/accessibility-snapshot-wire), answers in the one shape a binding needs: { json, elements, windows }, the tree as one JSON string whose nodes carry element as an index into elements and, on an iframe node whose frame has a content window, frame as an index into windows; null for a root this document does not own; { error: 'detached' } for a root no longer in its document. So a binding parses one string with its own JSON library and reads the two arrays, which BiDi serialises as node references and browsing context ids in one pass, instead of decoding a nested remote value and asking each frame for its contentWindow. The pure encodeWire is unit tested without a DOM; accessibility_wire_typescript_test.html (8 tests) pins the shape against the plain atom's tree, first-mention index order, the frame pointer and the data answers; the corpus page decodes the wire form of every page too; the Node tests hold the wire bundle to the same 64 KiB ceiling (51,647 bytes). The plain accessibility-snapshot entry is unchanged and stays published for in-page use.
  • Each binding now sends the wire entry inline, (root) => (ATOM)(root), once per document. Gone from every binding: the recursive remote-value decoder, the second callFunction per frame, the ownership check written into the declaration, the detached match on an exception's text, and the preload with its flag, its retry and the on-demand install (addPreloadScript is no longer used and no state is kept per realm; the only cost is the atom's source on each call, which the browser parses once per document). Kept: the BiDi no such node match (still an error, not data, until the typed exceptions exist), the per-context search, frame stitching and the BiDi guards. The Python, JavaScript and Ruby unit tests now cover the wire decoder (indices resolved, absent fields absent, frame children through the callback) and their stand-in connections answer the wire shape; Java gains two fromWire tests; .NET's decoder stays private (the unit project has no internals access) and is covered by the browser suite. The five binding corpus suites pass unchanged on the new entry (Python 20 unit and 26 BiDi, JavaScript 24 small and 26 browser, Ruby 25 unit and 27 integration, Java 4 unit and 46 harness checks, .NET 6 unit and 46 harness checks, on ChromeDriver 141 and headless Chromium 141), as do ruff, mypy, eslint, prettier, rubocop, steep, google-java-format and dotnet format whitespace.
  • Firefox (e306305): Firefox hands a shared id of another document's node to the called function as null, and the wire entry read null as "no root" and snapshotted the whole document, so RBE failed every binding's Firefox suite on the subtree-inside-a-frame case. A null root now answers null, the same as a root another document owns, so the bindings move on to the next browsing context; only an absent root means the whole document. The wire page pins both answers.
  • Not moved: rendering the text form in the atom, which would make to_text() either a second call or a second payload on every snapshot; held for the ADR thread.

.NET review by @nvborisenko (44b04be, 4ce3934, 19da7a3, 2c47192, 81c9b20, 1fc7f71, 55c5148, 67ccb84, 5caaf88, 1b28d31, 3554a3c, 6ff78ca, baad8a3, 482a22a, e8706de, 0c0dcdf, 7e06a36, fb18d41, ca13798, 2feeea9, 8fa4696)

  • WebDriver owns the BiDi connection: a session whose capabilities carry a WebSocket URL connects in the constructor, right after the session is created, so every internal component that works over BiDi shares one IBiDi (the internal BiDi property) for the driver's life, nothing is opened lazily and no connection lock or close flag is needed; the driver closes it from Dispose/DisposeAsyncCore. A connection that cannot be made fails the constructor, which quits the session it just started; BiDi/WebDriverConnectionTests pins that with a WebSocket URL nothing listens on, asserting the WebSocketException the refused connection raises (moved out of the snapshot fixture in 7e06a36, since it is about the connection, not snapshots; the exact exception asserted in fb18d41). The driver's field for the lazily created snapshotter is named after its class (2feeea9). The public AsBiDiAsync() extension is unchanged and keeps opening a connection its caller owns, by the maintainer's decision; internal components use the shared one, which connects with default options (propagating top-level options downstream is future work, also by the maintainer's decision). The constructor documents this (482a22a), and the two snapshot methods' exception docs now say the UnsupportedOperationException is for a session started without UseWebSocketUrl, since a session that asked for BiDi and could not connect fails in the constructor instead.
  • AccessibilitySnapshotter has no reference to WebDriver: it takes the IBiDi, an action giving the current window handle and an element factory, is stateless, and the driver holds one instance in a Lazy<T>, created on first use and dropped with the connection on disposal. The BiDi no such node error is read as a null answer (only for a call that sent a shared id), the same the atom gives for a root another document owns, so the context search reads one value and the internal exception type that carried it is gone. Every private helper is named with a verb (ReadContext, GetRequired, FormatLine, ListRoles, ...), every member of the .NET files in this PR, private helpers included, is described with a <summary> rather than a line comment (0c0dcdf), and the classes new in this PR follow the newer member style the maintainer asked for: no this. qualifiers, private instance fields _camelCase, private static fields s_camelCase (ca13798; WebDriver and WebElement keep the style around the lines added to them).
  • The .NET build is free of warnings from this PR's files (8fa4696): the XML comment on WebDriver.CreateElementFromReference, copied from GetElementFromResponse, named a parameter the method does not have (CS1572), and the one nullable annotation in the unit tests (CS8632) went with the test that moved in 7e06a36. The .NET / Unit Tests and .NET / Browser Tests logs on 8fa4696 report no compiler or analyzer warnings.
  • The browser fixture derives from DriverTestFixture, setting UseWebSocketUrl in ConfigureDriverOptions, so one browser serves every test; the corpus expectations are fetched from the test web server over HttpClient, as the pages are (index.json for the case list, then <page>.expected.json and .txt, the engine-specific file first with a 404 falling back to the shared one), so nothing is read from the source tree. A client is opened per read, since the NUnit analyzer refuses a disposable field without a one-time tear-down and the case list is read at discovery time. The readers are async and awaited from the test (e8706de); the case list, which NUnit discovers synchronously, uses the client's synchronous Send rather than blocking on a task. Open for the author: whether the corpus pages should instead be written out explicitly in each binding's test (with a check against index.json so a new page cannot be skipped silently), a cross-binding choice the maintainer raised; the index.json the fixture reads over HTTP is the copy the web server project carries into the build output (its csproj links common/src/web/** with copy-to-output, and its Bazel target lists //common/src/web as data), the same way the shared web extensions reach the test build.
  • Unit tests (6ff78ca, baad8a3): the hand-written recording executor and driver subclass are replaced by Moq (Mock<ICommandExecutor> answering every command with success, Mock<WebDriver> calling the base), the tests assert only what is thrown and nothing about the commands sent, since ICommandExecutor is slated for removal; neither fixture sets a parallel scope, which the project will turn on in one place; and the tests are named Member_Scenario_ExpectedResult as the other unit tests are, one case per test (12 tests across the three fixtures, all passing locally under NUnit).
  • Test infrastructure: the driver factory's remote path applies the fixture's options callback, so a Browser.Remote run gets its WebSocket URL too. A change that made AppServer.StartAsync() idempotent was reverted as out of scope (3554a3c); the double start it guarded against predates this PR and does not affect the snapshot tests.
  • Answered without change: ToText() keeps the ADR's name all five bindings share; the text form joins lines with \n on every platform because it is compared byte for byte with the corpus and the other bindings (now said in a comment); the JSON handling is JsonDocument/JsonElement only, so it is AOT-friendly; the state keys stay strings, since they are the wire names the corpus and the public States dictionary use. 4ce3934 sorts the System.Text.Json using ahead of the project's, as the Format job's dotnet format style wants.

🔧 Implementation Notes

  • One registry line drives the npm subpath, the browser-global test shim and the binding artifact, so the two shapes the ADR asks for come from one esbuild output and cannot drift.
  • The implementation gets its own folder, core/accessibility/, which needed ATOM_SRCS and the tsconfig include to become typescript/core/**/*.ts.
  • Where html-aam says an element has "no corresponding role" the atom reports generic, so the ADR's pruning rule treats it like a div.
  • Deliberate choices the ADR leaves open, each pinned by a test and called out in a code comment: input type=password is a textbox and summary is a button; frames report the ADR's iframe token, and an explicit role on a frame never replaces it; title beats placeholder on text inputs; inline children concatenate without an added space (Save<b>now</b> is Savenow, as Chromium computes it); an embedded control's value is read ahead of its own label when it sits inside something being named; pressed and, for grids, selected appear only when the element sets them (the ADR table says "whenever the role supports it" for both; this reading follows ARIA's stated default for the absent attribute); an ancestor's aria-disabled reaches focusable descendants only; a hidden aria-owns owner hides its owned elements. In the corpus, a label's text appears both as its control's name and as a text leaf, as Playwright's snapshot also renders it, and a treeitem's name from content includes its nested group, as accname's step 2F reads.
  • Hidden-ness is computed from getComputedStyle, not from bot.dom.isShown, and "focusable" for the presentational conflict rule is its own small check, not core/focusable.ts. Two engine facts are encoded explicitly: Chromium decides noscript rendering in layout rather than in its UA stylesheet, and closed details hides its content through a UA shadow tree script cannot see.
  • Limitations of script, each pinned by a test and documented in code: a closed shadow root cannot be seen (its host's light children are walked as if rendered, such a host reports focused when focus is inside it, and its sheet's :host::before is read only while some sheet in scope defines generated content).
  • Every cache lives in the per-call SnapshotContext and dies with the call. A module-level cache would be faster on a second snapshot of an unchanged page and wrong on the first snapshot after any attribute change, because element identity does not change with the attribute.
  • Three contract points pinned here are the main things to review: the whole-page root is the html element as role document; an explicit root always produces a node; invalid roots throw.
  • Bindings: the frames corpus page (014-iframe) records the atom's view, with iframe leaves; each binding's test asserts the stitched tree for it separately. The corpus README's conventions may want a binding-level expectation for frame pages now that five bindings stitch them. Since the wire round, what a binding holds is only what is the binding's: the public node type and its text form, the mapping of references to its element class, and frame stitching across browsing contexts.

Verification. The sandbox this was built in cannot fetch Bazel's external repositories, so the Bazel targets did not run there. On the current head 8fa4696 every check is green (55 checks): Lint (buildifier, google-java-format, ruff, rubocop, dotnet format, prettier), CI (the per-language jobs and the Java and .NET browser jobs on Linux, macOS and Windows, the .NET remote job), RBE ("Test / All RBE tests", which runs the Python, JavaScript, Ruby, Java and .NET snapshot suites, including every new target below, on Chrome, Edge and Firefox) and CodeQL. The one failure the wire round met on CI was the Firefox null root above, red on all five bindings' Firefox targets on 81c9b20 and green from e306305 on. closure-test-safari is compatible with macOS only and no CI job runs the atoms suites on macOS, so Safari has not run. Locally the equivalent checks ran with the pinned tools: esbuild 0.27.0 with the repo's atom config, tsc 5.8.3 with the package tsconfig (clean), the export generator's --check mode, a Node vm load of the bundle without a DOM, the two Node tests over the esbuild output, and every QUnit page in headless Chromium 141 against the shipped bundles, red before each implementation and green after, and unchanged through each performance commit. For the bindings, the generated BiDi layers were produced locally from the pinned CDDL with the repo's own generators, and the suites ran under pytest, mocha, rspec, the JUnit console launcher and NUnit as described above; CI runs //py:unit, //py:test-{chrome,firefox,edge}-bidi, //javascript/selenium-webdriver:small-tests, the per-browser test-accessibility-snapshot-test-js-* targets, //rb/spec/unit:unit, the accessibility_snapshot-*-bidi Ruby targets, //java/test/org/openqa/selenium:SmallTests, //java/test/org/openqa/selenium/remote:small-tests, the per-browser AccessibilitySnapshotTest targets in //java/test/org/openqa/selenium/bidi, //dotnet/test/unit/webdriver:unit and the per-browser AccessibilitySnapshotTests targets in //dotnet/test/webdriver.

bazel test //javascript/atoms:exports-up-to-date //javascript/atoms:atoms-load-in-node \
           //javascript/atoms:accessibility-unit-tests //javascript/atoms:accessibility-corpus-consistent
bazel test //javascript/atoms:closure-test{,-chrome,-edge,-safari} --test_filter='accessibility_.*_typescript_test'
bazel test //py:unit //py:test-chrome-bidi --test_arg=-k --test_arg=accessibility_snapshot
bazel test //javascript/selenium-webdriver:small-tests //javascript/selenium-webdriver:test-accessibility-snapshot-test-js-chrome
bazel test //rb/spec/unit:unit //rb/spec/integration/selenium/webdriver/bidi:accessibility_snapshot-chrome-bidi
bazel test //java/test/org/openqa/selenium:SmallTests //java/test/org/openqa/selenium/remote:small-tests \
           //java/test/org/openqa/selenium/bidi:AccessibilitySnapshotTest //java/test/org/openqa/selenium/bidi:AccessibilitySnapshotTest-firefox
bazel test //dotnet/test/unit/webdriver:unit //dotnet/test/webdriver:AccessibilitySnapshotTests //dotnet/test/webdriver:AccessibilitySnapshotTests-chrome

🤖 AI assistance

  • No substantial AI assistance used
  • AI assisted (complete below)
    • Tool(s): Claude Code
    • What was generated: the implementation plan, the QUnit pages, the modules under core/accessibility/, the test bundle and renderer, the corpus pages and recorded expectations, the schema and Node tests, the benchmark page, the build wiring, the README sections, the review-fix and performance rounds, the Python, JavaScript, Ruby, Java and .NET bindings and their tests, the wire-shape round, the commit messages and this description, under the author's direction and following the ADR
    • I reviewed all AI output and can explain the change

💡 Additional Considerations

  • Decisions to confirm on the ADR thread or here: root node for the whole page, iframe as the frame token and that a frame's explicit role never replaces it, the generic mapping for no-role elements, the password/summary divergences, title before placeholder, treating a closed shadow host's light-DOM children as rendered, a hidden explicit root returning its node with no children, keeping the element structure (but not the text) below a node named from its content, pressed/grid selected appearing only when set, first-owner-wins for conflicting aria-owns, a hidden aria-owns owner hiding its owned elements, inherited aria-disabled reaching focusable descendants only, masking rather than omitting password values, the text form's rules (false states omitted, value after a colon with line breaks and backslashes escaped, description not rendered), whether the text form should also be computed in the atom, whether each binding's corpus test should list the pages explicitly rather than read index.json (raised by the .NET maintainer; the same answer should apply to all five), in Python WebDriverException as the "unsupported operation" error for sessions without BiDi, since the binding has no dedicated class for it, in Ruby string keys for states, in Java UnsupportedCommandException for a classic-only session, a default interface method throwing UnsupportedOperationException for other implementations, and AccessibilityNode as a final class in the core package rather than an interface, and in .NET an async-only API (GetAccessibilitySnapshotAsync) on the WebDriver/WebElement classes rather than the interfaces, with the driver opening one BiDi connection in its constructor for every UseWebSocketUrl session, shared by its internal BiDi components, so a session that asks for BiDi fails fast when the connection cannot be made (Java logs and continues; the .NET maintainer has asked that the cross-binding policy be settled by the TLC).
  • Follow-on: a typed NoSuchNodeException in the Java BiDi layer and a typed error in the .NET one so the snapshotters can stop matching the error text; propagating top-level options to the driver-owned .NET connection's transport; measuring the BiDi wire cost of the bindings on the bench shapes; the out-of-CI comparison with @wdio/snapshot and native trees that the ADR §5 asks for, whose findings go into the corpus README's divergence list; a macOS job for the atoms suites if Safari coverage is wanted.
  • Cross-binding: all five bindings now consume the wire fragment; the corpus and its schema are what each binding is held to.
  • High-risk area touched per AGENTS.md: javascript/atoms/ and common/src/ (test fixtures). The change is additive (new registry entries, new folder, two glob widenings, two fragments, one test bundle, two Node tests, a new corpus directory, one filegroup visibility line and one js_library, one bench page, two .gitattributes lines; in Python one new module, two methods, one copy_file and one data entry; in JavaScript one new module, two methods, one export, one atom module target, one package entry, one lint data entry and one eslint ignore; in Ruby one new module, three methods, one copy_file, two data entries and the signatures; in Java two new classes, two default interface methods, two overrides and one accessor, one copy_file and one resource, one filegroup visibility line and the test wiring; in .NET two new classes, two virtual methods, two internal accessors (the snapshotter and the driver-owned BiDi connection), one internal element helper, the constructor's BiDi connection, two disposal hooks and one resource entry, plus one small fix in the test driver factory); no existing atom's source or output and no existing fixture changes.

🔄 Types of changes

  • New feature (non-breaking change which adds functionality and tests!)

🤖 Generated with Claude Code

https://claude.ai/code/session_016ubn295Cpb8fnrBgbXBMXw

First slice of the in-page accessibility snapshot proposed in ADR 18205
(docs/decisions/18205-accessibility-tree-snapshot.md): the atom exists,
ships through @selenium/atoms, and pins the root contract, so the
remaining slices (roles, hidden, tree walk, names, states, composed DOM,
corpus) and the binding PRs can build against a stable subpath.

- registry entry `accessibility-snapshot` in typescript/defs.bzl,
  exported as `accessibilitySnapshot` and published under
  `@selenium/atoms/accessibility-snapshot`; package.json regenerated
  with typescript/generate-exports.cjs
- entry point plus core/accessibility/{types,snapshot}.ts; the
  ATOM_SRCS and tsconfig globs now reach subdirectories under core/
- fragment `accessibility-snapshot-typescript` with the same visibility
  as the other TypeScript atoms the bindings embed
- QUnit page accessibility_tree_typescript_test.html, written first:
  the whole page is rooted at the html element as role `document`
  named from the title, Document and html roots are equivalent, every
  node carries the full schema with empty description/states, and a
  non-element or detached root throws rather than returning a partial
  tree

Below the document root every element is still reported as an unnamed
`generic` node; roles, names, states and children arrive in the next
slices, so README.npm.md is not updated yet.
@selenium-ci

Copy link
Copy Markdown
Member

/agentic_describe

@selenium-ci

Copy link
Copy Markdown
Member

/agentic_review

@selenium-ci selenium-ci added B-atoms JavaScript chunks generated by Google closure B-build Includes scripting, bazel and CI integrations labels Oct 23, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Add accessibility snapshot atom scaffold and role computation

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Publishes a draft accessibility-snapshot atom and node schema for future bindings.
• Computes explicit and HTML-AAM implicit roles for validated roots; tree traversal remains pending.
• Adds QUnit coverage for root contracts and contextual role mappings.
Diagram

graph TD
  A["Atom entry"] --> B["Root validation"] --> C{"Explicit role?"} -->|Yes| D{"Presentation conflict?"} -->|No| F["Node schema"] --> G["Snapshot result"]
  C -->|No| E["Implicit mapping"] --> F
  D -->|Yes| E
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Read native browser accessibility trees
  • ➕ Uses the browser's accessibility computations instead of maintaining role mappings.
  • ➖ Requires browser-protocol and binding integration rather than a portable in-page atom.
  • ➖ May produce different results across engines.

Recommendation: The in-page atom is the better fit for a shared, cross-browser package and a staged implementation. Keep its partial output clearly identified until traversal and names arrive, and wait for acceptance of the proposed ADR before merging.

Files changed (12) +944 / -3

Enhancement (5) +441 / -0
index.tsExport the snapshot function and node types +2/-0

Export the snapshot function and node types

• Adds accessibilitySnapshot and its TypeScript types to the atoms barrel.

javascript/atoms/index.ts

accessibility-snapshot.tsAdd the public snapshot atom entry point +21/-0

Add the public snapshot atom entry point

• Re-exports the core snapshot function and public node types without placing implementation logic in the entry point.

javascript/atoms/typescript/accessibility-snapshot.ts

roles.tsCompute ARIA and HTML-AAM roles +302/-0

Compute ARIA and HTML-AAM roles

• Resolves usable explicit role tokens, applies presentational-role conflict rules, and falls back to implicit roles. Covers contextual landmarks, lists, tables, form controls, and documented mapping exceptions.

javascript/atoms/typescript/core/accessibility/roles.ts

snapshot.tsValidate roots and produce initial snapshot nodes +77/-0

Validate roots and produce initial snapshot nodes

• Normalizes document roots, rejects invalid or detached elements, and returns a schema-complete node with its computed role. The document receives a title-derived name; other names and all children remain empty pending later slices.

javascript/atoms/typescript/core/accessibility/snapshot.ts

types.tsDefine the public accessibility node schema +39/-0

Define the public accessibility node schema

• Defines AccessibilityNode and StateValue, including optional values and backing elements for future snapshot features.

javascript/atoms/typescript/core/accessibility/types.ts

Tests (2) +473 / -0
accessibility_roles_typescript_test.htmlCover explicit and implicit role resolution +357/-0

Cover explicit and implicit role resolution

• Adds QUnit fixtures for role-token fallback, presentation conflicts, HTML-AAM mappings, and context-dependent roles. Assertions snapshot each fixture as a root without relying on unfinished tree traversal.

javascript/atoms/test/accessibility_roles_typescript_test.html

accessibility_tree_typescript_test.htmlPin the initial snapshot root contract +116/-0

Pin the initial snapshot root contract

• Tests document-root equivalence, title-derived naming, node shape, and rejection of invalid or detached roots.

javascript/atoms/test/accessibility_tree_typescript_test.html

Other (5) +30 / -3
BUILD.bazelInclude nested accessibility modules in atom builds +1/-1

Include nested accessibility modules in atom builds

• Expands the shared TypeScript source glob to include subdirectories under core, making the new implementation available to bundling and compilation.

javascript/atoms/BUILD.bazel

BUILD.bazelExpose a snapshot fragment to binding packages +13/-0

Expose a snapshot fragment to binding packages

• Adds a copy target for the generated atom with visibility matching other TypeScript fragments. No binding consumes it in this PR.

javascript/atoms/fragments/BUILD.bazel

package.jsonPublish the accessibility-snapshot subpath +6/-1

Publish the accessibility-snapshot subpath

• Adds the runtime export and declaration mapping for @selenium/atoms/accessibility-snapshot.

javascript/atoms/package.json

tsconfig.jsonType-check nested core modules +1/-1

Type-check nested core modules

• Includes TypeScript files in core subdirectories so the accessibility implementation is compiled.

javascript/atoms/tsconfig.json

defs.bzlRegister the snapshot atom and browser global +9/-0

Register the snapshot atom and browser global

• Adds the atom to the TypeScript registry under its own accessibility namespace, enabling generation and publication through existing atom targets.

javascript/atoms/typescript/defs.bzl

@qodo-code-review

qodo-code-review Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

Grey Divider


Action required

1. Custom BiDi connections cannot start ✗ Dismissed
Description
ConnectBiDi() calls AsBiDiAsync() without the configuration callback that the public connection
API accepts. When an endpoint requires custom WebSocket headers, proxy settings, certificates, or
transport, driver construction fails before the caller can connect with those settings.
Code

dotnet/src/webdriver/WebDriver.cs[251]


2. Driver disposal can hang indefinitely ✓ Resolved
Description
CloseBiDiAsync checks and sets bidiClosed without synchronizing concurrent callers before they
wait on bidiConnecting. When synchronous or asynchronous driver disposals overlap, both can pass
the guard, and one can dispose the semaphore without releasing it while the other remains queued
indefinitely.
Code

dotnet/src/webdriver/WebDriver.cs[R874-877]


3. Remote runs cannot take snapshots ✓ Resolved
Description
ConfigureDriverOptions enables UseWebSocketUrl, but DriverFactory.CreateDriver returns a
remote driver before applying that callback or any supplied options. When the suite runs with
Browser.Remote, the session has no WebSocket capability and every
GetAccessibilitySnapshotAsync() call throws UnsupportedOperationException.
Code

dotnet/test/webdriver/AccessibilitySnapshotTests.cs[R54-56]


View high (9)
4. Image-labelled forms lose their landmark ✗ Dismissed
Description
implicitRole() uses hasAuthorName() for forms, but that helper checks an aria-labelledby
target’s raw text and aria-label, not names supplied by its descendants. For `<form
aria-labelledby="label"> referencing <div id="label"><img alt="Billing"></div>`, name computation
yields “Billing” while the form receives the generic role, so snapshots omit its form landmark.
Code

javascript/atoms/typescript/core/accessibility/roles.ts[R275-276]


5. Subtree snapshots fail inside frames ✓ Resolved
Description
AccessibilitySnapshotter#snapshot rescues Error::NoSuchNodeError, but the Ruby error namespace
defines no such exception class. When an element belongs to a child browsing context and the
top-level context cannot resolve its shared ID, Ruby raises NameError instead of trying the child
context and returning the snapshot consistently with the other bindings.
Code

rb/lib/selenium/webdriver/common/accessibility_snapshot.rb[R228-231]


6. Iframe snapshots omit their documents ✓ Resolved
Description
_stitch_frames() only examines node.children, so it never stitches the root when that root is
itself an iframe. Calling WebElement.accessibility_snapshot() on an iframe therefore returns a
frame node without its reachable document, while the page-level path attaches that document.
Code

py/selenium/webdriver/remote/accessibility_snapshot.py[R305-307]


7. The new unit tests fail during collection ✗ Dismissed
Description
_corpus_pages() reads index.json while pytest parametrizes the new unit test, but the Bazel unit
target has no runfiles dependency on the shared web corpus. Running //py:unit therefore encounters
a missing file before it can collect the tests.
Code

py/test/unit/selenium/webdriver/common/accessibility_snapshot_tests.py[R46-50]


8. Cyclic labels produce wrong names ✓ Resolved
Description
referenceTexts, contentTexts, and descendantTexts cache per-element results without accounting
for the traversal’s visited set, even though fromReferences omits targets already in that set.
When an ancestor traversal encounters overlapping or cyclic aria-labelledby paths, it can cache
text with a reference omitted and reuse it when a descendant is later named with a fresh visited
set.
Code

javascript/atoms/typescript/core/accessibility/accname.ts[R103-114]


9. The scale test fails on bold text ✗ Dismissed
Description
The large-subtree test expects the bold element's text to be hoisted as a generic child, but the
snapshot assigns that element the strong role and emits a node for it. Every generated list item
contains <b>bold</b>, so the assertion fails deterministically with `['link', 'text', 'text',
'strong']` instead of the expected roles.
Code

javascript/atoms/test/accessibility_robustness_typescript_test.html[R101-102]


10. Shadow-owned items keep the wrong parent ✓ Resolved
Description
recordOwnership() uses root.querySelectorAll('[aria-owns]') instead of the flat-tree traversal,
so it misses owners inside open shadow roots and includes unslotted light-DOM owners that the walker
never visits. An owner inside a shadow root leaves its claimed target at its normal position, while
an unreachable unslotted owner can cause a visible target to be skipped; neither combined path has
focused test coverage.
Code

javascript/atoms/typescript/core/accessibility/tree.ts[R180-184]


11. Labelled inputs inside names show label, not value ✓ Resolved
Description
computeTextAlternative runs nativeName/fromLabels before embeddedValue, so a recursively
reached or referenced textbox, selection control, or range control contributes its own label rather
than its current value. This affects aria-labelledby self-references and content-named cells,
links, buttons, or outer labels containing a separately labelled control; when two labels each wrap
the other’s control, fromLabels can also recurse until the snapshot aborts with a stack overflow.
Code

javascript/atoms/typescript/core/accessibility/accname.ts[R300-311]


12. Decorative images acquire a name ✓ Resolved
Description
nativeName() treats alt="" as absent, allowing an image’s title to become its name. For an
<img alt="" title="Tooltip">, the role remains presentational but the nonempty name makes the tree
walk emit an image node that should have stayed pruned.
Code

javascript/atoms/typescript/core/accessibility/accname.ts[R244-246]



Remediation recommended

13. Failed browser connections break startup 📘 Rule violation ≡ Correctness
Description
ConnectBiDi() synchronously propagates a failed WebSocket connection from the WebDriver
constructor, which then quits the new session and rethrows the error. When the same webSocketUrl
connection fails, the Java binding logs the failure and continues as a classic-only driver, so users
receive different startup behavior across bindings.
Code

dotnet/src/webdriver/WebDriver.cs[R244-251]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The .NET binding now fails WebDriver construction when the initial BiDi WebSocket connection cannot be established, while the Java binding logs the failure and continues without BiDi. This creates inconsistent user-visible startup behavior for the same capability.

## Fix Focus Areas
- dotnet/src/webdriver/WebDriver.cs[241-251]
- java/src/org/openqa/selenium/remote/RemoteWebDriver.java[492-545]

## Recommended Fix
Choose and document one cross-binding policy for initial BiDi connection failures, then apply it consistently. Either preserve the fail-fast behavior across bindings or catch and log the connection failure in .NET while leaving the session usable as a classic-only driver.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗


14. A hidden owner makes visible elements vanish 🐞 Bug ≡ Correctness
Description
collectOwners() and recordOwnership() add aria-owns targets to context.claimed even when the
owner is hidden by aria-hidden, display:none, an ancestor, or collapsed details content. When
visitElement prunes that owner, its collectChildren never places the targets, while traversal
skips them at their DOM positions—even if a target is visible.
Code

javascript/atoms/typescript/core/accessibility/tree.ts[R234-245]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Hidden `aria-owns` owners can claim visible targets even though traversal will never visit the owners to place those targets, causing the targets to be skipped at their DOM positions.

## Fix Focus Areas
- javascript/atoms/typescript/core/accessibility/tree.ts[208-250]

## Recommended Fix
In `collectOwners` or `recordOwnership`, do not claim targets for owners the walk will prune, including owners with `hiddenState(owner) === 'subtree'`, owners hidden by an ancestor, and owners in collapsed details content. Do not stop descending past a pruned owner unless its whole subtree is pruned. Add a regression test with a hidden owner and a visible target outside it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗


15. Closed shadow hosts lose generated names 🐞 Bug ≡ Correctness
Description
mayHaveGeneratedContent checks a host’s own stylesheet only when element.shadowRoot exposes that
root. A closed root can apply :host::before { content: "Pre "; } while element.shadowRoot is
null, so a host without a matching document rule has its pseudo text cached as empty rather than
included in its name.
Code

javascript/atoms/typescript/core/accessibility/accname.ts[R183-185]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A closed shadow stylesheet can generate content on its host, but the host cannot expose that stylesheet through `element.shadowRoot`; treating the missing root as proof of no generated content loses name text.
## Fix Focus Areas
- javascript/atoms/typescript/core/accessibility/accname.ts[179-200]
## Recommended Fix
Do not use absence of an accessible shadow root as proof that the host has no generated content. Retain computed pseudo-style reads wherever the scope check cannot establish that absence, and add a closed-root `:host::before` name regression.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗


View medium (45)
16. Hidden closed-shadow children seem visible 🐞 Bug ≡ Correctness
Description
flatTreeParent() returns the host for its light-DOM child when host.shadowRoot and
child.assignedSlot are null, even though both are null for children of a closed shadow root. An
unslotted child of such a host consequently reaches the document in isHidden() and returns false;
a slotted child can likewise bypass hiding on its inaccessible slot ancestors.
Code

javascript/atoms/typescript/core/accessibility/hidden.ts[R80-82]

Dismiss ↗ | View ↗


17. The connection test accepts any failure ✗ Dismissed
Description
Constructor_UnreachableWebSocketUrl_Throws accepts any exception from driver construction instead
of proving that the configured unreachable WebSocket connection failed. Because construction also
covers session startup and cleanup, an unrelated constructor regression or failure before attempting
the endpoint can satisfy the test without exercising the intended URL path.
Code

dotnet/test/unit/webdriver/BiDi/WebDriverConnectionTests.cs[44]


18. Page snapshot failures lose error details ✓ Resolved
Description
CallAsync() now turns every BiDi exception mentioning no such node into null, including calls
with no element root. During a whole-page snapshot, SnapshotAsync() converts that null into a
generic “script returned nothing” exception, discarding the remote error and its cause instead of
exposing why the call failed.
Code

dotnet/src/webdriver/AccessibilitySnapshotter.cs[R184-187]


19. BiDi sessions open two connections ✗ Dismissed
Description
ConnectBiDi() opens and retains a WebSocket, while the existing AsBiDiAsync() extension always
opens another rather than returning that connection. The BiDi test fixture calls the extension after
constructing a driver with a WebSocket URL, so each fixture uses two connections for one session and
must close them separately.
Code

dotnet/src/webdriver/WebDriver.cs[251]


20. The test suite leaves a server running ✓ Resolved
Description
CorpusPages() now calls Fetch() during NUnit test discovery, accessing
EnvironmentManager.Instance.WebServer before assembly fixture setup and potentially issuing a
request before the server is ready. This initializes one AppServer, then AssemblyFixtureBase
starts the same server again, replacing _app without stopping the first listener and leaving it
undisposed during the test run.
Code

dotnet/test/webdriver/AccessibilitySnapshotTests.cs[R338-339]


21. A close failure can skip session cleanup ✓ Resolved
Description
WebDriver.DisposeAsyncCore() awaits accessibility.DisposeAsync() before its existing
session-quit and command-executor cleanup, without protecting that cleanup from a close failure. If
the BiDi broker or transport throws during disposal, the exception exits the method before the
classic session is quit and its executor is disposed.
Code

dotnet/src/webdriver/WebDriver.cs[R809-812]


22. Disposal can leave a connection open ✓ Resolved
Description
AccessibilitySnapshotter.Dispose() returns while bidi is null without waiting for an in-progress
SessionAsync() connection attempt. If driver disposal overlaps the first snapshot, that attempt
can assign a new BiDi connection after the driver has dropped the snapshotter, leaving the
connection open; async disposal can also dispose the semaphore before the initializer releases it.
Code

dotnet/src/webdriver/AccessibilitySnapshotter.cs[R97-102]


23. Generated labels disappear from names ✗ Dismissed
Description
readPseudoContent() skips every parenthesized token in computed CSS content, including
text-producing attr(...) expressions. In browsers that retain attr(data-label) in the computed
value, a button or link named from its generated content loses that label in the snapshot.
Code

javascript/atoms/typescript/core/accessibility/accname.ts[R275-276]


24. Snapshot elements lose custom behavior ✗ Dismissed
Description
snapshot_context constructs decoded element references with Element.new rather than the bridge's
configured element class. When a Ruby client configures a custom element class, elements obtained
from accessibility snapshots lack its methods, unlike elements returned by normal lookup commands.
Code

rb/lib/selenium/webdriver/common/accessibility_snapshot.rb[270]


25. Parallel snapshots install duplicate scripts ✓ Resolved
Description
ensurePreload_, _ensure_preload, and ensure_preload now leave their shared preload flags unset
while registration is in flight, allowing overlapping snapshots to each send
script.addPreloadScript. Each successful command registers a separate script that runs on future
documents, so concurrent first snapshots in the JavaScript, Python, or Ruby bindings can leave
repeated atom installations in the session.
Code

javascript/selenium-webdriver/lib/accessibility_snapshot.js[328]


26. Python decoder exceeds the line width ✗ Dismissed
Description
node_from_remote_value leaves its function signature and children expression longer than the
repository's 120-character Ruff width. Running the repository formatter on this new module will
reflow these lines, creating formatting churn before the file passes the enforced check.
Code

py/selenium/webdriver/remote/accessibility_snapshot.py[138]


27. Disabled buttons regain a button role ✓ Resolved
Description
canTakeFocus() returns true for a valid tabindex before checking whether a native control is
disabled. A disabled button with tabindex="0" and role="presentation" therefore has its
presentational role rejected even though it cannot take focus.
Code

javascript/atoms/typescript/core/accessibility/roles.ts[R105-107]


28. Every unnamed form shows up as a landmark ✓ Resolved
Description
implicitRole() returns 'form' for every <form> without checking for an accessible name.
html-aam maps an unnamed form to no role. Elsewhere in the same function, section and nested
aside already go through hasAuthorName(). Because of this, every plain <form> produces a
form landmark node rather than being hoisted as generic. Corpus page 004 (- form with no name)
locks that behaviour into the contract all bindings share.
Code

javascript/atoms/typescript/core/accessibility/roles.ts[R273-274]


29. Re-roled frames lose their documents ✓ Resolved
Description
stitchFrames_() recognizes a frame only when its reported role is exactly iframe, although the
atom accepts an explicit role on an iframe while still treating the element as a leaf. An `<iframe
role="group">` therefore has no child document in the binding's snapshot even when its browsing
context is reachable.
Code

javascript/selenium-webdriver/lib/accessibility_snapshot.js[366]


30. Safari runs fail the new browser suite ✓ Resolved
Description
The new integration suite uses the unfiltered test-environment builder and calls the BiDi-only
snapshot method in its cases. On macOS the harness includes Safari but does not request
webSocketUrl for it, so those cases hit the method's unsupported-session guard.
Code

javascript/selenium-webdriver/test/accessibility_snapshot_test.js[89]


31. Driver failures yield incomplete snapshots ✓ Resolved
Description
_stitch_frames() catches every WebDriverException from a child snapshot and treats it as an
unreachable frame. A command or session failure unrelated to frame reachability therefore returns a
tree with that frame unstitched, with the failure only logged at debug level.
Code

py/selenium/webdriver/remote/accessibility_snapshot.py[R310-313]


32. Local development misses the new atom ✓ Resolved
Description
py:local_dev copies an explicit list of generated JavaScript resources into the Python source
tree, but that list omits accessibilitySnapshot.js. After the normal local-development generation
command, _atom() cannot load the gitignored resource from the source package and raises
FileNotFoundError.
Code

py/BUILD.bazel[R165-168]


33. Frame lookup failures report live roots stale ✓ Resolved
Description
_contexts() swallows every WebDriverException from get_tree() and stops after yielding the
top-level context. When context discovery fails for a reason other than a missing root, a live
element in a child frame is never tried there and snapshot() raises
StaleElementReferenceException.
Code

py/selenium/webdriver/remote/accessibility_snapshot.py[R340-344]


34. A navigating frame aborts the snapshot ✓ Resolved
Description
_call() converts a BiDi “no such node” error into _NoSuchNode, but _stitch_frames() catches
only WebDriverException. If a child document disappears between frame discovery and its snapshot
call, the exception escapes instead of leaving that iframe as a leaf.
Code

py/selenium/webdriver/remote/accessibility_snapshot.py[R309-311]


35. Shadow-part generated text drops out of names ✓ Resolved
Description
mayHaveGeneratedContent checks the element’s root, its own shadow root and its assigned slot’s
root, but not enclosing tree scopes whose stylesheets can target the element through ::part() or
forwarded exportparts. When an outer scope supplies ::before or ::after content to a part and
the checked scopes have no generated-content rules, the shortcut treats that content as empty,
omitting it from enclosing button or link names.
Code

javascript/atoms/typescript/core/accessibility/accname.ts[R179-188]


36. Multiline values split text snapshots ✓ Resolved
Description
toText() appends node.value directly to a line without escaping line breaks. When a textarea
contains multiple lines, its snapshot value retains those breaks, so the reference renderer emits
extra lines that look like separate records instead of one line for the textbox.
Code

javascript/atoms/typescript/testing/accessibility-text.ts[R60-62]


37. Snapshots can crash on foreign slots ✓ Resolved
Description
flatTreeChildren() treats every element with local name slot as an HTMLSlotElement and calls
assignedNodes() without checking its namespace. Walking an SVG or MathML element named slot
reaches this branch even though that element has no such method, aborting the snapshot with a type
error.
Code

javascript/atoms/typescript/core/accessibility/hidden.ts[R125-126]


38. Mutual owners vanish from snapshots ✓ Resolved
Description
recordOwnership() rejects self-ownership and DOM ancestors but accepts two siblings that name each
other in aria-owns, adding both to claimed. Each is then skipped at its DOM position, and
neither owner is visited to place the other, so both disappear from a snapshot containing them.
Code

javascript/atoms/typescript/core/accessibility/tree.ts[R191-193]


39. Re-roled choices retain a selection state ✓ Resolved
Description
selected() returns the native <option> selection before checking the computed role passed to it.
If an option is assigned another explicit role, such as link, its snapshot reports selected
despite reporting that different role.
Code

javascript/atoms/typescript/core/accessibility/states.ts[R90-94]


40. Snapshots expose typed passwords in plain text ✓ Resolved
Description
computeValue returns embeddedValue(element, role) for every textbox, and implicitRole maps
input type=password to textbox, so the node's value is the field's real unmasked
input.value. Any snapshot taken after a test types a credential carries the secret in its result,
and the result can be serialized into logs, test reports or failure artifacts. Browsers' own
accessibility trees mask this value.
Code

javascript/atoms/typescript/core/accessibility/states.ts[R180-182]


41. Shadow controls lose inherited disabled state ✓ Resolved
Description
ariaDisabled() walks parentElement only, so it stops at a shadow root without inspecting its
host. When the host has aria-disabled="true", a snapshot rooted at a control inside that shadow
root omits the inherited disabled state.
Code

javascript/atoms/typescript/core/accessibility/states.ts[R126-132]


42. Focused shadow controls lose their focus state ✓ Resolved
Description
focused() compares the element only with ownerDocument.activeElement, which points to the shadow
host when focus is inside a shadow root. A snapshot rooted at a focused shadow control therefore
omits focused: true, even though that connected control is a valid snapshot root.
Code

javascript/atoms/typescript/core/accessibility/states.ts[R141-144]


43. Invisible children still add text to names ✓ Resolved
Description
fromContent suppresses direct text on a visibility:hidden child but still includes its
::before and ::after content, while textAlternative can still use that child’s aria-label,
native alt, or title. When such a child appears inside a button, its invisible generated or
attribute-based text can enter the button’s accessible name, even though independently visible
descendants may still contribute.
Code

javascript/atoms/typescript/core/accessibility/accname.ts[R165-169]


44. Hidden text enters editable values ✓ Resolved
Description
embeddedValue() falls back to textContent for a custom textbox without a value property,
rather than reading its rendered editable value. A contenteditable textbox containing a hidden
descendant therefore passes that descendant’s text into an enclosing element’s name.
Code

javascript/atoms/typescript/core/accessibility/value.ts[R58-61]


45. Unselected choices enter parent names ✓ Resolved
Description
embeddedValue() returns a custom listbox’s entire textContent rather than the text of its
selected options. When that listbox is embedded in a heading or label, unselected choices contribute
to the enclosing element’s accessible name.
Code

javascript/atoms/typescript/core/accessibility/value.ts[R58-61]


46. Presentational tables retain row structure ✓ Resolved
Description
collectChildren() prunes a table with role="presentation" but computes its required descendants'
ordinary roles without inheriting presentation. A table containing tbody, tr, and td therefore
still exposes rowgroup, row, and cell nodes beneath the pruned table instead of flattening that
structural markup.
Code

javascript/atoms/typescript/core/accessibility/tree.ts[R121-123]


47. Unrendered fallback content shows up in the tree ✗ Dismissed
Description
collectChildren() walks the DOM children of <video>, <audio> and <object>, which have role
generic and get hoisted. Their fallback markup is therefore emitted even though the browser does
not render it while the media or object is supported/loaded. Pages that include "Your browser does
not support video" text or download links inside a media element get those nodes in the snapshot,
though neither the page nor the browser's accessibility tree shows them.
Code

javascript/atoms/typescript/core/accessibility/tree.ts[R119-124]


48. Image-map links never appear in snapshots ✓ Resolved
Description
hiddenState() treats a computed display: none as subtree. The UA stylesheet gives every
<area> element display: none, so collectChildren() always skips areas, and the area[href] →
link mapping in implicitRole can never be reached during the walk. Clickable image-map regions
that assistive technology exposes as links are missing from every snapshot, and isHidden reports
them as hidden.
Code

javascript/atoms/typescript/core/accessibility/hidden.ts[R118-123]


49. Broken labels create false landmarks ✓ Resolved
Description
hasAuthorName() treats any nonempty aria-labelledby attribute as a name without resolving its
referenced elements. A section whose attribute points to a missing or empty target is consequently
assigned region, and a similarly labelled aside inside sectioning content is assigned
complementary, despite having no name from that reference.
Code

javascript/atoms/typescript/core/accessibility/roles.ts[R91-92]


50. Textarea default text shows as a child node ✓ Resolved
Description
collectChildren() descends into every non-leaf role, and textbox is not in
CHILDREN_PRESENTATIONAL, so the DOM child text node of a <textarea> (its default value, which is
not its current value) is emitted as a text child of the textbox node. Any textarea with initial
markup content gets a stale text node in the snapshot, and it will still be there next to the
value field once the states/value slice lands.
Code

javascript/atoms/typescript/core/accessibility/tree.ts[R125-128]


51. Titled sections lose their landmark role ✓ Resolved
Description
hasAuthorName() checks aria-label and aria-labelledby but omits the title attribute when
selecting conditional implicit roles. A <section title="Search results"> is therefore reported as
generic instead of region, so the tree walk prunes a landmark that has an author-provided
accessible name.
Code

javascript/atoms/typescript/core/accessibility/roles.ts[R86-92]


52. Hidden-root snapshots expose descendants ✓ Resolved
Description
buildTree() checks only hiddenState(root), which deliberately ignores the root’s ancestors. A
connected root inside an aria-hidden or display:none container, or inside closed-details
content, therefore still receives children that are excluded from the page’s accessibility tree.
Code

javascript/atoms/typescript/core/accessibility/tree.ts[R142-144]


53. Inert controls appear in snapshots ✓ Resolved
Description
collectChildren() continues through elements marked inert because its hiddenness gate does not
account for that attribute. A rendered <div inert><button>Save</button></div> consequently exposes
the button, although inert content is excluded from the browser’s accessibility tree.
Code

javascript/atoms/typescript/core/accessibility/tree.ts[R110-113]


54. Closed details expose hidden text ✓ Resolved
Description
collectChildren() emits text nodes before its closed-details check, which runs only for element
children. Direct text after the first summary in a closed <details> therefore appears in the
snapshot even though an element at the same position is excluded.
Code

javascript/atoms/typescript/core/accessibility/tree.ts[R89-93]


55. Graphic symbols expose child structure ✓ Resolved
Description
CHILDREN_PRESENTATIONAL omits the supported graphics-symbol role, so isLeafRole() allows the
walk to descend into it. A graphic symbol containing nested semantic elements consequently exposes
child nodes instead of remaining an atomic symbol.
Code

javascript/atoms/typescript/core/accessibility/tree.ts[R28-31]


56. Frames can expose fallback content ✓ Resolved
Description
isLeafRole() stops at a frame only when its computed role is exactly iframe. A frame with
another valid explicit role can therefore be traversed by collectChildren(), emitting fallback DOM
that is not rendered while its browsing context is active.
Code

javascript/atoms/typescript/core/accessibility/tree.ts[R125-127]


57. CSS-hidden content appears rendered ✓ Resolved
Description
hiddenState() checks computed display and visibility but not content-visibility, so it
returns rendered for an element with content-visibility: hidden. When that property is set on a
container, isHidden() also treats its descendants as rendered because the ancestor walk never
encounters a subtree-hiding state.
Code

javascript/atoms/typescript/core/accessibility/hidden.ts[R121-123]


58. Document-role subtrees inherit the page title ✓ Resolved
Description
accessibilitySnapshot() assigns documentName(element) whenever the computed role is document,
without checking whether the element is the document root. A subtree rooted at `<div
role="document">` consequently receives the page title as its name even though naming below the
document root is deferred.
Code

javascript/atoms/typescript/core/accessibility/snapshot.ts[R72-74]


59. ARIA attributes leave elements hidden ✓ Resolved
Description
GLOBAL_ARIA_ATTRIBUTES omits global properties including aria-disabled, aria-errormessage,
aria-haspopup and aria-invalid, so computeRole() can retain an explicit none or
presentation role. For example, a non-focusable <ul role="none" aria-disabled="true"> is
reported as none instead of falling back to its implicit list role.
Code

javascript/atoms/typescript/core/accessibility/roles.ts[R46-50]


60. Non-focusable links lose presentation ✓ Resolved
Description
computeRole() uses the shared isFocusable() helper to reject none and presentation, but that
helper considers every anchor focusable even without an href. An <a role="none"> without an
href therefore reports generic rather than its permitted explicit role; the same check
misclassifies other non-focusable elements treated as focusable by the helper.
Code

javascript/atoms/typescript/core/accessibility/roles.ts[R298-300]



Informational

61. Text under content-named nodes can disappear 🐞 Bug ≡ Correctness
Description
collectChildren drops every text leaf below a node whose name source is 'content', assuming the
text is already in that name. A descendant whose own aria-label or aria-labelledby replaces its
text during recursion (e.g. <h1>Intro <span aria-label="X">Y</span></h1>) gets its replacement
into the name, while its real text 'Y' is suppressed. That text then appears nowhere in the
snapshot.
Code

javascript/atoms/typescript/core/accessibility/tree.ts[123]

Dismiss ↗ | View ↗


62. Large image maps make snapshots slow ✗ Dismissed
Description
areaState() runs ownerDocument.querySelectorAll('img[usemap]') and then checks the hidden state
and ancestors of each matching image, every time it is asked about an <area>. Nothing caches the
answer per map, so a page with many areas and many usemap images does areas × images selector and
ancestor work in a single snapshot.
Code

javascript/atoms/typescript/core/accessibility/hidden.ts[R61-66]


63. Non-toggle elements report a false state ✓ Resolved
Description
expanded() returns value === 'true' for any aria-expanded attribute that is present, and
computeStates treats any aria-pressed that is present as a toggle. So
aria-expanded=""/"undefined" yields expanded: false and aria-pressed=""/"undefined" yields
pressed: false. In ARIA those values mean the element is not expandable or not a toggle button,
which is exactly the misleading pressed=false case the module's comment says it avoids.
Code

javascript/atoms/typescript/core/accessibility/states.ts[R76-80]


View low (2)
64. Static content under aria-disabled reads as disabled ✓ Resolved
Description
ariaDisabled walks every ancestor and marks any node below an aria-disabled="true" element as
disabled: true, whatever its role. WAI-ARIA 1.2 limits the inherited state to focusable
descendants. A <div aria-disabled="true"> wrapping headings, paragraphs or lists therefore reports
all of them as disabled, so assertions on disabled give false positives for content that cannot be
operated.
Code

javascript/atoms/typescript/core/accessibility/states.ts[R125-133]


65. CSS alt text with a slash in it is lost ✓ Resolved
Description
pseudoContent searches the raw computed content string for the first ' / ' with indexOf,
without skipping quoted strings. A value such as "a / b" / "alt", or a plain "1 / 2" with no
alternative, is cut inside the quoted string. The string regex then returns the wrong text or none,
so the generated content drops out of the element's name.
Code

javascript/atoms/typescript/core/accessibility/accname.ts[R124-127]


Grey Divider

⚠️ Warning | Comment size exceeded, inner finding content trimmed View findings
⚠️ Warning | Comment size exceeded, non finding content trimmed Learn more

Grey Divider

Qodo Logo

Comment thread javascript/atoms/typescript/core/accessibility/roles.ts Outdated
Comment thread javascript/atoms/typescript/core/accessibility/roles.ts Outdated
Comment thread javascript/atoms/typescript/core/accessibility/snapshot.ts Outdated
@selenium-ci

Copy link
Copy Markdown
Member

/agentic_review

@AutomatedTester AutomatedTester changed the title [js] add the accessibility-snapshot atom to @selenium/atoms: scaffold and roles [js] add the accessibility-snapshot atom to @selenium/atoms: scaffold, roles and hidden-ness Oct 23, 2026
Comment thread javascript/atoms/typescript/core/accessibility/hidden.ts
Comment on lines +80 to +82
const parentElement = parent as Element
if (parentElement.shadowRoot) {
return NOT_RENDERED

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

2. Hidden closed-shadow children seem visible 🐞 Bug ≡ Correctness

flatTreeParent() returns the host for its light-DOM child when host.shadowRoot and
child.assignedSlot are null, even though both are null for children of a closed shadow root. An
unslotted child of such a host consequently reaches the document in isHidden() and returns false;
a slotted child can likewise bypass hiding on its inaccessible slot ancestors.
Agent Prompt
## Issue description
The flat-tree traversal silently treats light-DOM children of closed shadow hosts as rendered, although their actual slot assignments cannot be inspected.

## Fix Focus Areas
- javascript/atoms/typescript/core/accessibility/hidden.ts[61-87]
- javascript/atoms/test/accessibility_hidden_typescript_test.html[148-168]

## Recommended Fix
Establish an explicit way to handle inaccessible closed-root assignments rather than returning the ordinary DOM parent as proof of rendering. Add coverage for an unslotted child and a child assigned beneath hidden shadow content.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changed. A closed shadow root is invisible to script: host.shadowRoot and child.assignedSlot are both null, and there is no API that distinguishes "no shadow root" from "closed shadow root" without side effects. Treating those children as rendered is the only choice that does not drop light-DOM content that may well be slotted. The limitation is documented in flatTreeParent and in the PR description, and ADR 18205 already lists closed roots as a known edge.


Generated by Claude Code

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit cf6f55c

@selenium-ci

Copy link
Copy Markdown
Member

/agentic_review

@AutomatedTester
AutomatedTester force-pushed the claude/ecstatic-bardeen-6l95xe branch from da3cf41 to 204a0a8 Compare October 23, 2026 11:29
@selenium-ci

Copy link
Copy Markdown
Member

/agentic_review

@AutomatedTester AutomatedTester changed the title [js] add the accessibility-snapshot atom to @selenium/atoms: scaffold, roles and hidden-ness [js] add the accessibility-snapshot atom to @selenium/atoms: scaffold, roles, hidden-ness and tree walk Oct 23, 2026
Comment thread javascript/atoms/typescript/core/accessibility/tree.ts Outdated
Comment thread javascript/atoms/typescript/core/accessibility/tree.ts Outdated
Comment thread javascript/atoms/typescript/core/accessibility/tree.ts Outdated
Comment thread javascript/atoms/typescript/core/accessibility/tree.ts
Comment thread javascript/atoms/typescript/core/accessibility/tree.ts Outdated
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 204a0a8

Second slice of ADR 18205: every node now carries the role its explicit
`role` attribute names, or the html-aam implicit role.

- explicit roles: the first token naming a WAI-ARIA 1.2 role authors
  may use (abstract roles and unknown tokens fall through), matched
  ASCII case-insensitively; none/presentation are ignored on focusable
  elements and on elements with a global ARIA attribute (WAI-ARIA 1.2
  section 9.1)
- implicit roles: the html-aam table, including the contextual rows
  (header/footer outside sectioning content, aside and section only
  when labelled, li only inside ul/ol/menu, select by multiple/size,
  input by type and list, td as gridcell inside a grid, th by scope,
  img alt="")
- elements html-aam gives no role report `generic`, so the pruning rule
  in ADR 18205 treats them like a div; frames report the ADR's `iframe`
  token
- two deliberate divergences towards what engines expose, pinned by the
  test page: input type=password is a textbox and summary is a button

accessibility_roles_typescript_test.html was written first and failed
28 of 32 tests against the previous slice; it now passes 125 assertions
in Chromium, with the tree page unchanged.
Third slice of ADR 18205: core/accessibility/hidden.ts decides whether
an element is hidden in the accname sense, which the tree walk in the
next slice uses to leave nodes out.

- isHidden: aria-hidden=true on the element or an ancestor (aria-hidden
  false cannot undo it), display none, visibility hidden or collapse on
  the element's own computed style, the hidden attribute through the UA
  stylesheet (so author CSS can show it again), closed details content
  other than the first summary, noscript, template content, detached
  elements
- ancestry follows the flat tree: a slotted element follows its slot, a
  shadow root's child its host; an unslotted child of a shadow host and
  the fallback content of a filled slot are not rendered
- hiddenState reports what an element does on its own (rendered, self,
  subtree) so the walk can prune a subtree once and still descend into
  a visibility-hidden element whose children may be visible
- this is deliberately not bot.dom.isShown: opacity, off-screen position
  and the clip pattern stay rendered, which the page pins

The module is reached through a test-only bundle, typescript/testing/
accessibility.ts, registered in TEST_BUNDLES and never published, since
the public atom only exposes hidden-ness through the tree walk.

accessibility_hidden_typescript_test.html was written first and failed
15 of 21 tests against a stub; it now passes all 55 assertions in
Chromium, with the roles and tree pages unchanged.
Fourth slice of ADR 18205: a snapshot now has children. The walk visits
each element once, top-down, threading ancestor state instead of asking
isHidden per node.

- core/accessibility/tree.ts: an element produces a node when its role
  is not generic, none or presentation (or, once names land, when it
  has a name); pruned elements hoist their children in place, so
  document order is kept; aria-hidden and display none drop the
  subtree; a visibility hidden element drops its own node and text but
  descends, since a child may set itself visible again; closed details
  content is skipped; text runs become `text` leaves with collapsed
  whitespace and no element; iframe and roles with presentational
  children (button, img, checkbox, option, ...) are leaves
- the root always produces a node, even a generic or hidden one, so a
  subtree snapshot of a plain div is deterministic; a root hidden as a
  subtree has no children
- core/accessibility/context.ts: per-call SnapshotContext with a role
  cache and the set of placed elements. It lives exactly as long as the
  call because the snapshot is a point-in-time copy; a module-level
  cache would return stale results after the page changes an
  attribute. Style and name caches join it when the name slice needs
  them.

The walk uses childNodes; the composed-DOM slice swaps in the flat tree
and aria-owns. Names below the document root are still empty, so text
leaves are not yet suppressed under nodes named from content.

accessibility_tree_typescript_test.html gained the walk fixtures first
and failed 6 of 17 tests against the previous slice; it now passes all
44 assertions in Chromium, with the roles and hidden pages unchanged.
@AutomatedTester
AutomatedTester force-pushed the claude/ecstatic-bardeen-6l95xe branch from 204a0a8 to 3b8f4dd Compare October 23, 2026 11:37
@selenium-ci

Copy link
Copy Markdown
Member

/agentic_review

Comment thread javascript/atoms/typescript/core/accessibility/tree.ts Outdated
Comment thread javascript/atoms/typescript/core/accessibility/roles.ts Outdated
Comment thread javascript/atoms/typescript/core/accessibility/roles.ts Outdated
Comment thread javascript/atoms/typescript/core/accessibility/tree.ts Outdated
Comment thread javascript/atoms/typescript/core/accessibility/tree.ts Outdated
Comment thread javascript/atoms/typescript/core/accessibility/hidden.ts Outdated
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 3b8f4dd

Fifth slice of ADR 18205: every node now carries its accessible name and
description, following the accname 1.2 text alternative computation with
the html-aam rules for native elements.

- core/accessibility/accname.ts: aria-labelledby (space-joined, hidden
  references included, not followed from a referenced element, a self
  reference contributes the control value), aria-label, native sources
  (labels with the control itself left out, button input values with
  the Submit and Reset defaults, img and area alt, legend, figcaption,
  caption, option and optgroup labels, svg title, the document title),
  embedded control values, name from content for the roles that allow
  it and for every descendant during recursion, with ::before and
  ::after content included and block-level children set off by spaces,
  then title, then placeholder for text inputs
- descriptions: aria-describedby, aria-description, then title when the
  name did not already use it
- core/accessibility/value.ts: the values accname step 2E reads from
  textboxes, selects and range controls; the states slice reuses it
- core/accessibility/context.ts gains the computed-style and name
  caches, which is where repeated work first appears: labelledby
  targets are read once per snapshot and descendants keep the style
  the walk already resolved
- the walk applies the full pruning rule, so a generic element with a
  name now produces a node, and text an ancestor's name was computed
  from is not repeated as leaves below it, while the element structure
  there still is

Two choices the record leaves open are pinned by the pages: title beats
placeholder on text inputs, as html-aam orders them, and inline children
concatenate without an added space ("Save<b>now</b>" is "Savenow").

accessibility_name_typescript_test.html (29 tests) and
accessibility_description_typescript_test.html (7 tests) were written
first and failed 27 and 6 of them against the previous slice; the tree
page gained the named-generic and consumed-text cases and failed 6 of
19. All five pages now pass in Chromium: 74, 12, 47, 125 and 55
assertions.
@selenium-ci

Copy link
Copy Markdown
Member

/agentic_review

@AutomatedTester AutomatedTester changed the title [js] add the accessibility-snapshot atom to @selenium/atoms: scaffold, roles, hidden-ness and tree walk [js] add the accessibility-snapshot atom to @selenium/atoms: scaffold, roles, hidden-ness, tree walk and names Oct 23, 2026
Comment thread javascript/atoms/typescript/core/accessibility/accname.ts
Comment thread javascript/atoms/typescript/core/accessibility/value.ts Outdated
Comment thread javascript/atoms/typescript/core/accessibility/value.ts Outdated
Comment thread javascript/atoms/typescript/core/accessibility/accname.ts Outdated
Comment thread javascript/atoms/typescript/core/accessibility/accname.ts Outdated
Comment thread javascript/atoms/typescript/core/accessibility/accname.ts Outdated
}
const node = createNode(role, name.text, computeDescription(element, context), element)
if (!isLeafRole(role)) {
collectChildren(element, context, false, textConsumed || name.source === 'content', node.children)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Informational

23. Text under content-named nodes can disappear 🐞 Bug ≡ Correctness

collectChildren drops every text leaf below a node whose name source is 'content', assuming the
text is already in that name. A descendant whose own aria-label or aria-labelledby replaces its
text during recursion (e.g. <h1>Intro <span aria-label="X">Y</span></h1>) gets its replacement
into the name, while its real text 'Y' is suppressed. That text then appears nowhere in the
snapshot.
Agent Prompt
## Issue description
Text leaves are suppressed under content-named nodes even when a descendant's text was replaced by its own aria-label or labelledby in the name.

## Fix Focus Areas
- javascript/atoms/typescript/core/accessibility/tree.ts[115-124]

## Recommended Fix
When descending into an element whose own `computeName` source is 'label' or 'reference', reset `textConsumed` to false for its subtree, because that subtree's text was not used in the ancestor's name.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Dismiss ↗ | View ↗

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changed, by design. In <h1>Intro <span aria-label="X">Y</span></h1> the heading's name is "Intro X" and the span survives as a named generic node, so "X" is what the tree reports twice and "Y" nowhere. That matches what assistive technology hears: aria-label replaces the span's text in the heading's name and in the span's own name. Repeating "Y" as a text leaf would put text in the snapshot that no user is given. Keeping element structure but not text below a content-named node is one of the choices the PR description lists for confirmation on the ADR thread.


Generated by Claude Code

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit cc70c59

Comment thread dotnet/test/webdriver/AccessibilitySnapshotTests.cs Outdated
The browser fixture fetched the corpus expectations from the test web
server by blocking on the HttpClient tasks with GetAwaiter().GetResult().
The maintainer asked for an async FetchAsync instead. The expectation
readers are now async and awaited from the test, which is async already.
The one read that cannot await is the case list, since NUnit discovers
test cases synchronously; it uses the client's synchronous Send and
parses the index from the response stream, so no task is blocked on.
@selenium-ci

Copy link
Copy Markdown
Member

/agentic_review

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit e8706de

The maintainer asked that members be described with the language's
documentation comments rather than line comments, private ones too.
Every line comment that sat above a member of the snapshotter, the node,
the driver's BiDi members and the browser fixture is now a <summary>,
and the private helpers that had no comment get a one-line summary.
No code changes.
@selenium-ci

Copy link
Copy Markdown
Member

/agentic_review

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 0c0dcdf

Comment thread dotnet/test/unit/webdriver/AccessibilitySnapshotUnitTests.cs Outdated
The test that a driver fails to construct when its WebSocket URL cannot
be reached sat in the accessibility snapshot unit tests, where the
maintainer pointed out it does not belong: it checks the driver's BiDi
connection, not snapshots. It moves to WebDriverConnectionTests in the
BiDi unit test folder, and the snapshot fixture keeps only the two
tests about snapshots without BiDi, with a helper that no longer takes
a WebSocket URL.
@selenium-ci

Copy link
Copy Markdown
Member

/agentic_review

/// NUnit discovers test cases synchronously, so the case list is the one corpus read that cannot
/// await; it uses the client's synchronous Send rather than blocking on a task.
/// </summary>
public static IEnumerable<string> ListCorpusPages()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO it will be better to explicitly enumerate well-known pages instead of parametrized tests relying on WebServer in runtime.
Discussable. Needs to be aligned with all bindings.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed it is a cross-binding call, so I have not changed it here and am leaving the thread open for @AutomatedTester. Where the five bindings stand: Python, JavaScript, Ruby and Java all parametrize their corpus test from index.json, read from the checkout at collection time, so a page added to the corpus is picked up everywhere without touching any binding; .NET is the only one reading the index over HTTP, and only because the browser suite was asked not to read the source tree. If the choice is an explicit list, the shape I would propose for all five is the same: the page names written out in the test, plus one test that asserts the written list equals the corpus index.json, so a new page fails loudly instead of being silently skipped. Happy to apply that to all bindings in a follow-up commit once the author says which way to go.


Generated by Claude Code

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. We should discuss. My concern is that test should never rely on some "unpredictable" file path (index.json) in this case. This is why I proposed to get it from WebServer since it is already served properly.
To highlight the issue: in .NET it is possible to (pseudo):

dotnet build --output "ftp://something"
cd "ftp://something"
dotnet run # execute tests

index.json file is not part of build. Agent, please see how we deal with shared web extensions to test it as aprt of build. Summarize.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary of how the shared web extensions reach the .NET tests as part of the build, and what it means for the corpus:

Two build paths, both ship the shared files with the test output.

  • Bazel: //common/extensions is a filegroup listed in data of the dotnet_nunit_test_suite in dotnet/test/webdriver/BUILD.bazel, so the files are in the test's runfiles. The web server library (//dotnet/test/testing.webserver) lists //common/src/web the same way.
  • csproj: Selenium.WebDriver.Tests.csproj links ..\..\..\common\extensions\** as <None> items with CopyToOutputDirectory=PreserveNewest under common\extensions\..., so dotnet build --output X copies them into X/common/extensions/. Selenium.Testing.WebServer.csproj does the same for common\src\web\**, so the whole common/src/web tree, the accessibility corpus and its index.json included, lands in X/common/src/web/ as part of the build.

How the tests find them at run time.

  • WebExtensionTests.LocateRelativePath and FirefoxDriverTests.GetPath call Bazel.Runfiles.Create().Rlocation("_main/common/extensions/...") (the Runfiles NuGet package, referenced by both the Bazel target and the csproj) and fall back to a path relative to the assembly's base directory when there is no runfiles manifest (plain dotnet test).
  • AppServer.FindWebContentRoot walks up from AppContext.BaseDirectory until it finds common/src/web, so under dotnet build --output X it serves X/common/src/web, and under Bazel the runfiles copy. EnvironmentManager resolves configured paths the same way, through TEST_SRCDIR/_main when set.

Implication for this fixture. Because the web server project already carries common/src/web into the build output, the index.json the fixture reads over HTTP is the one from the build output, not from the source checkout: in your dotnet build --output "ftp://something" scenario it is served from .../common/src/web/accessibility/index.json inside that output. So the HTTP read is already "part of the build" in the same sense the extensions are. If reading it from disk is preferred, the extension pattern applies directly: link common\src\web\accessibility\index.json (or the whole corpus) into the test csproj and resolve it with Runfiles.Rlocation with the base-directory fallback, no new mechanism needed. The explicit list in the test plus a check against the served or linked index.json remains the third option. I have changed nothing pending the discussion and the author's call, since the other four bindings parametrize from index.json on disk and should move together.


Generated by Claude Code

Comment thread dotnet/test/unit/webdriver/BiDi/WebDriverConnectionTests.cs Outdated
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 7e06a36

The test that a driver fails to construct when its WebSocket URL cannot
be reached accepted any exception, so an unrelated constructor failure
would have passed it. It now asserts the WebSocketException the refused
connection raises, as the inner exception of the invocation exception
the mocked driver's proxy wraps it in, and its name says so.
@selenium-ci

Copy link
Copy Markdown
Member

/agentic_review

Comment thread dotnet/test/webdriver/AccessibilitySnapshotTests.cs Outdated
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit fb18d41

Comment thread dotnet/src/webdriver/BUILD.bazel
The maintainer asked that classes new in this PR follow the newer .NET
conventions rather than the older ones the existing classes still mix:
no `this.` when accessing members, private instance fields named
`_camelCase` and private static fields `s_camelCase`. The snapshotter,
the node and the browser fixture drop every `this.` qualifier, the
snapshotter's three fields and logger and the node's state order are
renamed, and the classes that are not new keep their existing style.
No behaviour changes.
Comment thread dotnet/src/webdriver/WebDriver.cs Outdated
@selenium-ci

Copy link
Copy Markdown
Member

/agentic_review

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit ca13798

The maintainer asked that the driver's field holding the lazily created
AccessibilitySnapshotter be named after that class rather than
"accessibility". No behaviour change.
@selenium-ci

Copy link
Copy Markdown
Member

/agentic_review

The helper's XML comment was copied from GetElementFromResponse and
still described a "response" parameter the method does not have, which
the compiler reported as CS1572 on every .NET build. The comment now
describes the method and its elementId parameter. The other warning the
build reported, a nullable annotation in the snapshot unit tests, went
away when that test moved in 7e06a36. No code changes.
@selenium-ci

Copy link
Copy Markdown
Member

/agentic_review

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 8fa4696

@nvborisenko

Copy link
Copy Markdown
Member

I missed ADR or discussion of public API. This PR proposed flatten API: driver.GetAccessibilitySnapshot(). But what if driver.Accessibility.GetSnashot()? Are we expecting more methods in accessibility area? In any case I vote to flatten structure: driver.CaptureScreenshot() and driver.GetAccessibilitySnapshot().

Hm. may be GetAccessibilityTree?

Copy link
Copy Markdown
Member Author

@nvborisenko the public API is decided in ADR 18205 (SeleniumHQ/selenium#18205, docs/decisions/18205-accessibility-tree-snapshot.md, section "1. Public API"), which this PR implements. It is the flat shape you vote for: two methods per binding, one on the driver for the whole page and one on the element for a subtree, with the AccessibilitySnapshot stem and the AccessibilityNode type name fixed across the five bindings and only casing and sync/async following each binding's idiom (hence GetAccessibilitySnapshotAsync() in .NET, next to GetScreenshot()). No driver.Accessibility namespace is planned: the whole surface is these two methods plus AccessibilityNode.ToText(), so there is nothing to group.

On "Snapshot" versus "Tree": the ADR's title is "Accessibility tree snapshots", and the word is deliberate. The result is a computed, point-in-time copy of the tree that does not update with the page and is explicitly not the browser's native accessibility tree, and the name matches what users know from Playwright's ariaSnapshot() and Puppeteer's accessibility.snapshot(). Changing it would be a cross-binding change to the ADR, so it belongs on SeleniumHQ/selenium#18205 for @AutomatedTester to decide; I have not changed anything here.

One .NET detail worth your eye: the ADR text says .NET would use extension methods to avoid new interface members. This PR instead adds virtual methods on the WebDriver and WebElement classes, the way ComputedAccessibleLabel and ComputedAccessibleRole are exposed, which also keeps third-party IWebDriver/IWebElement implementations compiling and lets drivers override. If you would rather have extension methods as the ADR words it, say so and I will switch.


Generated by Claude Code

@nvborisenko

nvborisenko commented Oct 26, 2026 •

Copy link
Copy Markdown
Member

If GetAccessibilitySnapshotAsync() is just helper, then it can be extension method. If it is part of contract, then it should be in IWebDriver interface. If it is additional to implementation, then in WebDriver class (inheritable) - which is IWebDriver?.

And this is a concern: why we need IWebDriver contract and WebDriver abstract class together..

Agent, don't react on this comment.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-atoms JavaScript chunks generated by Google closure B-build Includes scripting, bazel and CI integrations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants