Add screen-reader support: Orca and VoiceOver#9156
Conversation
471e94c to
407c93e
Compare
20107ea to
91420e4
Compare
91420e4 to
5a2a36d
Compare
5a2a36d to
655cf40
Compare
8a6760a to
a13a3a2
Compare
9c6b0ae to
5a8ce20
Compare
a200ed7 to
23afee8
Compare
4a97543 to
1a379d6
Compare
1a379d6 to
0b80d00
Compare
ab3990b to
4342f61
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughLadybird adds an end-to-end accessibility system: WebContent serializes DOM accessibility data over IPC, UI processes expose it through Qt AT-SPI2 and AppKit, Orca integration handles Linux navigation and speech, and extensive fixtures, bridge tests, CI jobs, and documentation validate the behavior. ChangesScreen-reader accessibility
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Comment |
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/lagom-template.yml (1)
327-352: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winAvoid running the Linux Release accessibility suite twice.
The generic Release
cteststep at Lines 337–340 already includes the CMake-registeredLibWebAccessibilityBridgeLayer1andLayer2tests. The dedicated step reruns both suites, increasing CI time and flakiness exposure. Remove the dedicated step, or exclude this regex from the generic run.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/lagom-template.yml around lines 327 - 352, Remove the dedicated “Test - LibWeb AT-SPI2 bridge (Release)” step from the workflow, since the generic Release “Test” step already runs the registered accessibility bridge tests. Preserve the existing generic test conditions and command unchanged.
🟠 Major comments (27)
.github/workflows/lagom-template.yml-219-249 (1)
219-249: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGrant Accessibility to the Python executable that actually runs the tests.
PyObjC is installed into the active
actions/setup-pythoninterpreter, and preflight checkspython3first. The grant loop only updates TCC for system Python paths, so it can grant one executable and immediately test an ungranted tool-cache executable.Resolve and reuse one Python path—such as
realpath "$(command -v python3)"—for both the TCC insertion and AX preflight.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/lagom-template.yml around lines 219 - 249, The workflow currently grants Accessibility to fixed system Python paths instead of the interpreter used by the tests. Resolve one active Python executable path, such as the canonical path from command -v python3, and reuse it in both the TCC grant loop and the AX preflight’s Python selection, removing the competing path list so both operations target the same interpreter.Tests/LibWeb/AccessibilityBridge/run_tests.sh-138-157 (1)
138-157: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not trust inherited resource-ownership markers.
If the caller exports
LADYBIRD_RUNTIME_DIR_OWNED=1, cleanup recursively deletes its existingXDG_RUNTIME_DIR. An inheritedXVFB_PIDsimilarly causes an unrelated process to be killed. Initialize both markers locally before acquiring resources.Proposed fix
+XVFB_PID="" +LADYBIRD_RUNTIME_DIR_OWNED=0 + if [[ -z "${XDG_RUNTIME_DIR:-}" ]] || [[ ! -d "${XDG_RUNTIME_DIR}" ]]; then ... - if [[ -n "${LADYBIRD_RUNTIME_DIR_OWNED:-}" && -n "${XDG_RUNTIME_DIR:-}" ]]; then + if [[ "${LADYBIRD_RUNTIME_DIR_OWNED}" == 1 && -n "${XDG_RUNTIME_DIR:-}" ]]; then🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LibWeb/AccessibilityBridge/run_tests.sh` around lines 138 - 157, Initialize LADYBIRD_RUNTIME_DIR_OWNED and XVFB_PID locally to empty values before the runtime-directory setup and any Xvfb acquisition in run_tests.sh. Ensure cleanup_outer only acts on ownership and process markers established by this script, not inherited environment values.Tests/LibWeb/AccessibilityBridge/tests_orca/test_focus_changed.py-53-62 (1)
53-62: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not suppress crashes from the focus handler.
Catching every non-
AttributeErrorexception makes both “does not crash” tests pass when the production handler raisesTypeError,RuntimeError, or another real failure. Let the exception fail the test; if the synthetic event is incomplete, explicitly model the missing fields instead.Proposed fix
- try: - self.orca.script._on_focused_changed(event) - except AttributeError as exc: - self.fail( - f"_on_focused_changed raised AttributeError — likely a pre-Orca-50 API call slipping back in: {exc}" - ) - except Exception: - # Other exceptions (TypeError from a partially-constructed event, errors deep in super's call chain - # against the synthetic environment, etc.) aren't what this regression test is guarding against. - pass + self.orca.script._on_focused_changed(event)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LibWeb/AccessibilityBridge/tests_orca/test_focus_changed.py` around lines 53 - 62, Update the focus-handler test around _on_focused_changed to stop catching and suppressing generic exceptions; allow TypeError, RuntimeError, and other failures to propagate and fail the test. Retain only the targeted AttributeError assertion, and explicitly add any missing synthetic-event fields needed for the handler to execute successfully.Source: Linters/SAST tools
Libraries/LibWebView/AccessibilityNodeData.h-96-96 (1)
96-96: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRepresent pressed state as a tri-state value.
aria-pressedsupportsfalse,true, andmixed, but this boolean permanently loses the mixed state across IPC. Use an enum analogous toCheckedState.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWebView/AccessibilityNodeData.h` at line 96, Replace the boolean is_pressed field in AccessibilityNodeData with a tri-state enum analogous to CheckedState, representing false, true, and mixed aria-pressed values. Update its IPC serialization and any consumers to preserve and handle all three states.Libraries/LibWebView/AccessibilityNodeData.cpp-103-104 (1)
103-104: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReject invalid enum values during IPC decoding.
Arbitrary bytes are cast directly to
CheckedStateandExpandedState. A malformed WebContent message can place impossible enum values in the cache and break downstream switches. Decode the byte, validate its range, then cast.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWebView/AccessibilityNodeData.cpp` around lines 103 - 104, Update the IPC decoding around checked_state and expanded_state to decode each byte into a temporary value, validate it against the valid CheckedState and ExpandedState ranges, and reject malformed values before casting. Preserve the existing TRY-based failure propagation and only construct the enums after validation.Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp-130-140 (1)
130-140: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winLet ARIA naming take precedence over the
<option>fallback.This branch bypasses
accessible_name()for every option, soaria-labelandaria-labelledbyare ignored. Compute the standard accessible name first and fall back tolabel/text only when it is empty.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp` around lines 130 - 140, Update the option handling in the accessibility name assignment to compute the standard accessible name via element.accessible_name(document) first, preserving aria-label and aria-labelledby. Only when that name is empty should the HTMLOptionElement label(), then text(), fallback be used; keep the existing non-option behavior unchanged.Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp-409-411 (1)
409-411: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftUse text-fragment bounds for text leaves.
Every text leaf receives its parent’s full rectangle. Because
AccessibilityTreeManager::hit_test_recursive()returns the deepest matching child, multiple leaves under one parent cause the last leaf to be returned anywhere inside that parent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp` around lines 409 - 411, Update the text-node bounds assignment in the accessibility tree construction to use the text fragment’s own bounds rather than the parent element’s full rectangle. Ensure each text leaf’s bounds correspond only to its rendered text fragment so AccessibilityTreeManager::hit_test_recursive() selects the correct leaf.Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp-150-153 (1)
150-153: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPopulate native heading levels.
Only
aria-levelis serialized, leaving ordinary<h1>–<h6>nodes at level0. Fall back toHTMLHeadingElement::heading_level()when no valid ARIA level exists.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp` around lines 150 - 153, Update the heading-level assignment in the accessibility tree construction to use HTMLHeadingElement::heading_level() when aria_level() is absent or cannot be parsed. Preserve a valid parsed aria-level as the preferred value, and ensure ordinary h1–h6 elements no longer remain at level 0.Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp-164-165 (1)
164-165: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSerialize
aria-disabledstate.
is_actually_disabled()covers host-language disabling, but ARIA widgets witharia-disabled="true"remain exposed as enabled.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp` around lines 164 - 165, Update the disabled-state serialization near node_data.is_disabled so it also detects an element’s aria-disabled="true" state, while retaining the existing is_actually_disabled() check. Ensure node_data.is_disabled is set whenever either host-language disabling or ARIA disabling applies.Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp-117-126 (1)
117-126: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRestore the implicit role for every presentation-role conflict.
When global ARIA attributes invalidate
none/presentation, non-image elements still serialize the presentation role. For example, a labelled button loses its button semantics. Use the element’s implicit role for every conflict, not only images.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp` around lines 117 - 126, Update the conflict-handling branch in the accessibility tree role assignment so every element with a none/presentation role and global ARIA attributes uses its implicit semantic role. Replace the non-image fallback in this branch with the existing implicit-role resolution mechanism, preserving the image behavior and the normal role serialization path outside the conflict.Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp-326-364 (1)
326-364: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not expose disabled or ARIA-readonly controls as editable.
Disabled text inputs are marked editable, while
aria-readonly="true"sets read-only after editability without clearing it. This produces contradictory platform states.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp` around lines 326 - 364, Update the editability logic in the input, textarea, and contenteditable handling to exclude disabled controls, and ensure the aria_read_only override clears node_data.is_editable when set to true. Preserve read-only and other state assignments while preventing contradictory editable/read-only states.Libraries/LibWeb/DOM/Node.cpp-3102-3109 (1)
3102-3109: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not prune a subtree solely because its element lacks a layout node.
display: contentselements have no layout node while their descendants remain rendered. This condition returns early and removes those visible descendants from the accessibility tree. Restrict subtree pruning to actual hidden semantics such asdisplay:none, visibility exclusion, andaria-hidden.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/DOM/Node.cpp` around lines 3102 - 3109, Update the accessibility-tree pruning condition in the surrounding Node traversal to stop treating a missing element->layout_node() as sufficient for subtree exclusion. Retain pruning for aria-hidden and actual CSS visibility/display-hidden semantics, while allowing display: contents elements without layout nodes to expose their rendered descendants.Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp-262-287 (1)
262-287: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPopulate the string
valuepayload.
node_data.valueis never assigned, so text controls, selects, and ARIA value text arrive at platform bridges with an empty current value. Serialize applicable host-control values andaria-valuetext.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp` around lines 262 - 287, Populate node_data.value alongside the numeric accessibility fields in the AccessibilityTreeNode construction flow. Serialize the current value for applicable HTML host controls, including text controls and select elements, and use the element’s aria-valuetext when present, ensuring the resulting string reaches platform accessibility bridges without overwriting valid applicable values.Libraries/LibWebView/AccessibilityTreeManager.cpp-131-139 (1)
131-139: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDo not treat reverse child order as paint order.
Accessibility/DOM child order does not account for z-index, positioned descendants, or stacking contexts. Overlapping elements can therefore return the wrong accessibility target. Preserve paint ordering or delegate hit testing to layout/painting data.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWebView/AccessibilityTreeManager.cpp` around lines 131 - 139, Update hit_test_recursive so it does not assume current->child_ids reverse order represents visual paint order. Use the layout/painting hit-testing data or an existing paint-order traversal that accounts for z-index, positioned descendants, and stacking contexts, while preserving the deepest-hit behavior and bounds fallback.Libraries/LibWeb/DOM/Node.cpp-3087-3095 (1)
3087-3095: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPreserve option-group and exclusion semantics for
<select>children.Appending every entry from
list_of_options()directly under the select drops<optgroup>structure and includes options regardless ofhiddenoraria-hidden. Build the select subtree while bypassing only the option layout-node requirement.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/DOM/Node.cpp` around lines 3087 - 3095, Update the HTMLSelectElement handling in the accessibility-tree construction to preserve optgroup hierarchy and existing hidden/aria-hidden exclusion semantics. Traverse the select’s child structure rather than appending every entry from list_of_options() directly, and bypass only the layout-node requirement for option elements. Keep options nested under their optgroup accessibility nodes while excluding hidden or aria-hidden entries.Libraries/LibWeb/Page/Page.h-535-535 (1)
535-535: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAllow the callback to report that focus was cleared.
This signature cannot represent a null active element, and the caller skips notification when focus becomes null. Accessibility consumers can therefore retain a stale focused node. Accept
Optional<Web::UniqueNodeID>or add an explicit clearing callback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/Page/Page.h` at line 535, Update the Page::page_did_change_active_element callback to accept Optional<Web::UniqueNodeID>, and adjust its callers to notify it when the active element is cleared by passing an empty optional. Preserve passing the node ID for non-null active elements so accessibility consumers receive both focus and focus-cleared notifications.Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp-537-558 (1)
537-558: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftMap selections contained by the node, not only exact focus-node matches.
The implementation requires
focus_node == value(). Element text interfaces therefore receive no caret for descendant text, and selections spanning multiple text nodes are omitted entirely.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp` around lines 537 - 558, Update the selection mapping in the accessibility-tree population logic to handle selections whose focus or anchor nodes are descendants of value(), not only exact matches. Traverse or otherwise determine containment for descendant text nodes, convert their DOM offsets appropriately, and populate caret and selection ranges for selections contained by the node, including ranges spanning multiple text nodes.Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp-171-193 (1)
171-193: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftBuild a logical table grid before assigning coordinates and headers.
Sibling
colspansums do not account for cells occupying columns throughrowspan. This gives later cells incorrect column indices and assigns headers to the wrong columns. Build an occupancy grid and apply each header across its full row/column span.Also applies to: 200-258
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp` around lines 171 - 193, Replace the direct sibling-colspan calculation in the table-cell accessibility logic with a logical occupancy grid built from all table rows and cells. Track positions consumed by row_span, assign each cell the first available column, and apply each header cell across its complete row_span and col_span before assigning coordinates and headers. Update the related logic around node_data.cell_row_index and node_data.cell_column_index consistently.Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp-289-311 (1)
289-311: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHonor ARIA selection and checked state independently of element class.
aria-selectedis never serialized, and theelse ifprevents everyHTMLInputElementfrom usingaria-checked, including inputs whose role is overridden to checkbox or switch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp` around lines 289 - 311, Update the accessibility-state serialization around node_data.is_selected and node_data.checked_state so aria-selected is read and serialized for every element, not only option elements. Separate aria-checked handling from the HTMLInputElement type check, allowing valid ARIA values to apply to inputs with overridden checkbox or switch roles while retaining native input state when aria-checked is absent.Libraries/LibWebView/AccessibilityTreeManager.cpp-41-47 (1)
41-47: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftAnnounce newly inserted live-region content.
A new descendant has no entry in
m_nodes, so this branch skips it. Consequently, appending text to a live region—the primary live-region use case—produces no notification. Compare each live region’s accessible text/subtree between generations rather than only same-ID node names.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWebView/AccessibilityTreeManager.cpp` around lines 41 - 47, Update the live-region change detection around m_nodes and on_live_region_changed to handle newly inserted descendants instead of skipping them when old_it is m_nodes.end(). Compare each live region’s accessible text or subtree across generations, and announce newly added content while preserving existing notifications for changed same-ID nodes.Libraries/LibWeb/DOM/Element.cpp-3513-3523 (1)
3513-3523: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winaria-hidden walk should ignore
falsevaluesis_aria_hidden()and the duplicated loop inexclude_from_accessibility_tree()stop at the firstaria-hiddenattribute of any value, so a descendant’saria-hidden="false"can mask an ancestor’saria-hidden="true". Return only on"true"(or reuseis_aria_hidden()) in both places.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/DOM/Element.cpp` around lines 3513 - 3523, Update Element::is_aria_hidden() and the duplicated aria-hidden traversal in exclude_from_accessibility_tree() so only an attribute value of "true" returns or blocks the walk; continue traversing ancestors when the nearest value is "false" or any other value, preserving detection of an ancestor’s aria-hidden="true".Libraries/LibWeb/DOM/Document.cpp-3683-3685 (1)
3683-3685: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winForward the cleared active-element state
page_did_change_active_element()only carries aWeb::UniqueNodeID, and the UI handlers use that to set the focused accessibility node. Whenm_active_elementbecomes null, nothing is sent, so the screen reader can keep a stale focused-node state instead of falling back to the document root/body. Consider forwarding an explicit clear/sentinel value here, or add a separate “focus cleared” notification.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/DOM/Document.cpp` around lines 3683 - 3685, Update the active-element notification in Document’s relevant focus-clearing path so page_did_change_active_element() is also invoked when m_active_element is null, using the established clear/sentinel UniqueNodeID or a dedicated focus-cleared notification. Preserve the existing unique_id() behavior for non-null active elements and ensure the UI can fall back to the document root/body.UI/Qt/OrcaScripts/orca_customizations_bootstrap.py-34-35 (1)
34-35: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not silently swallow custom-script failures.
Catching
Exceptionand doing nothing makes import or constructor bugs silently fall back to Orca’s default script, disabling Ladybird’s accessibility behavior without diagnostics. Catch expected import failures separately and log unexpected exceptions with their traceback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@UI/Qt/OrcaScripts/orca_customizations_bootstrap.py` around lines 34 - 35, Update the exception handling around custom-script loading in the bootstrap flow: handle expected import failures separately, while logging unexpected exceptions with traceback details before falling back to Orca’s default script. Remove the silent `except Exception: pass` behavior so constructor or import bugs remain diagnosable.Source: Linters/SAST tools
UI/AppKit/Interface/LadybirdAccessibilityElement.mm-958-966 (1)
958-966: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard against a reversed same-node text-marker range.
start_offandend_offare clamped to the text length but not ordered. IfendData.offset < startData.offset,end_off - start_offunderflows to a huge length andsubstring_viewreads out of bounds / trips aVERIFY.🛡️ Proposed clamp
i32 start_off = MIN(startData.offset, static_cast<i32>(text.length())); i32 end_off = MIN(endData.offset, static_cast<i32>(text.length())); + if (end_off < start_off) + end_off = start_off; auto slice = text.substring_view(start_off, end_off - start_off);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@UI/AppKit/Interface/LadybirdAccessibilityElement.mm` around lines 958 - 966, Update the same-node branch in the text-marker range handling to validate or clamp the offsets so a reversed range where endData.offset is less than startData.offset cannot produce a negative or underflowed substring length. Preserve normal extraction for ordered offsets and ensure reversed ranges yield a safe empty or otherwise established clamped result before calling substring_view.Services/WebContent/ConnectionFromClient.cpp-1636-1667 (1)
1636-1667: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClamp AT-supplied character offsets before converting to code units.
offset_start/offset_endarrive over IPC and are fed straight intocode_unit_offset_of()andsubstring_view(). If an offset exceeds the node's character count,view.length_in_code_units() - end_code_unitunderflows (hugesize_t) andsubstring_view/code_unit_offset_ofcan trip aVERIFY, crashing the WebContent (renderer) process. A screen reader supplying an end-of-text or off-by-one offset is a realistic trigger. Clamp offsets to[0, character_count](and order start ≤ end) before use.Please confirm
Utf16View::code_unit_offset_ofbehavior for an offset greater than the character count (crash vs. clamp):#!/bin/bash rg -nP --type=cpp -C4 '\bcode_unit_offset_of\b' AK/ Libraries/ | head -60 ast-grep run --pattern 'code_unit_offset_of($_)' --lang cpp AK/ Libraries/🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Services/WebContent/ConnectionFromClient.cpp` around lines 1636 - 1667, Clamp AT-supplied offsets to the target text’s character count and ensure start is no greater than end before calling Utf16View::code_unit_offset_of or substring_view. Update the convert_to_code_units lambda and the edit_value flow in the surrounding connection handler, preserving caret behavior while preventing out-of-range conversions and subtraction underflow.UI/AppKit/Interface/LadybirdAccessibilityElement.mm-362-370 (1)
362-370: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSensitive Data Exposure (CWE-200): Exposure of Sensitive Information to an Unauthorized Actor
Reachability: External
Mask password values in accessibility UI/AppKit/Interface/LadybirdAccessibilityElement.mm:362-370; Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp:403-409. Password inputs are serialized as ordinary
text leafnodes, andAXValuereturnsdata->namedirectly, so accessibility clients can read the raw password. Carry a secure-text flag throughAccessibilityNodeDataand expose these fields as secure text / masked value instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@UI/AppKit/Interface/LadybirdAccessibilityElement.mm` around lines 362 - 370, Propagate a secure-text flag through AccessibilityNodeData for password inputs, including the text-leaf serialization in AccessibilityTreeNode, and update the NSAccessibilityValueAttribute handling in LadybirdAccessibilityElement to expose secure fields as secure text with a masked value instead of returning data->name. Preserve existing behavior for non-password text leaves and other accessibility values.UI/Qt/Application.cpp-110-112 (1)
110-112: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winWrite
orca-customizations.pyatomically.QFile::WriteOnlytruncates the existing file before the new content is safely persisted, so a crash or partial write can wipe user customizations outside the managed block. UseQSaveFile, checkwrite()andcommit(), and keep the original file on failure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@UI/Qt/Application.cpp` around lines 110 - 112, Update the customizations write flow using QSaveFile instead of QFile so the existing orca-customizations.py remains intact until replacement succeeds. Check the full write result and require commit() to succeed, preserving the original file and handling failures without replacing it.
🧹 Nitpick comments (1)
Libraries/LibWeb/DOM/Document.cpp (1)
6023-6041: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicates the tree-build/empty-check pattern from
dump_accessibility_tree_as_json().Both functions build an
AccessibilityTreeNode, checkaccessibility_tree->value(), and synthesize a fallback "document" node on empty. Consider extracting a shared helper that returns the built tree (or handles the empty case), to avoid the two implementations drifting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Libraries/LibWeb/DOM/Document.cpp` around lines 6023 - 6041, Extract the shared AccessibilityTreeNode construction and empty-document handling used by build_accessibility_node_data() and dump_accessibility_tree_as_json() into a common helper. Update both callers to reuse that helper while preserving the existing synthesized document-node behavior and each function’s output-specific serialization.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/lagom-template.yml:
- Around line 327-352: Remove the dedicated “Test - LibWeb AT-SPI2 bridge
(Release)” step from the workflow, since the generic Release “Test” step already
runs the registered accessibility bridge tests. Preserve the existing generic
test conditions and command unchanged.
---
Major comments:
In @.github/workflows/lagom-template.yml:
- Around line 219-249: The workflow currently grants Accessibility to fixed
system Python paths instead of the interpreter used by the tests. Resolve one
active Python executable path, such as the canonical path from command -v
python3, and reuse it in both the TCC grant loop and the AX preflight’s Python
selection, removing the competing path list so both operations target the same
interpreter.
In `@Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp`:
- Around line 130-140: Update the option handling in the accessibility name
assignment to compute the standard accessible name via
element.accessible_name(document) first, preserving aria-label and
aria-labelledby. Only when that name is empty should the HTMLOptionElement
label(), then text(), fallback be used; keep the existing non-option behavior
unchanged.
- Around line 409-411: Update the text-node bounds assignment in the
accessibility tree construction to use the text fragment’s own bounds rather
than the parent element’s full rectangle. Ensure each text leaf’s bounds
correspond only to its rendered text fragment so
AccessibilityTreeManager::hit_test_recursive() selects the correct leaf.
- Around line 150-153: Update the heading-level assignment in the accessibility
tree construction to use HTMLHeadingElement::heading_level() when aria_level()
is absent or cannot be parsed. Preserve a valid parsed aria-level as the
preferred value, and ensure ordinary h1–h6 elements no longer remain at level 0.
- Around line 164-165: Update the disabled-state serialization near
node_data.is_disabled so it also detects an element’s aria-disabled="true"
state, while retaining the existing is_actually_disabled() check. Ensure
node_data.is_disabled is set whenever either host-language disabling or ARIA
disabling applies.
- Around line 117-126: Update the conflict-handling branch in the accessibility
tree role assignment so every element with a none/presentation role and global
ARIA attributes uses its implicit semantic role. Replace the non-image fallback
in this branch with the existing implicit-role resolution mechanism, preserving
the image behavior and the normal role serialization path outside the conflict.
- Around line 326-364: Update the editability logic in the input, textarea, and
contenteditable handling to exclude disabled controls, and ensure the
aria_read_only override clears node_data.is_editable when set to true. Preserve
read-only and other state assignments while preventing contradictory
editable/read-only states.
- Around line 262-287: Populate node_data.value alongside the numeric
accessibility fields in the AccessibilityTreeNode construction flow. Serialize
the current value for applicable HTML host controls, including text controls and
select elements, and use the element’s aria-valuetext when present, ensuring the
resulting string reaches platform accessibility bridges without overwriting
valid applicable values.
- Around line 537-558: Update the selection mapping in the accessibility-tree
population logic to handle selections whose focus or anchor nodes are
descendants of value(), not only exact matches. Traverse or otherwise determine
containment for descendant text nodes, convert their DOM offsets appropriately,
and populate caret and selection ranges for selections contained by the node,
including ranges spanning multiple text nodes.
- Around line 171-193: Replace the direct sibling-colspan calculation in the
table-cell accessibility logic with a logical occupancy grid built from all
table rows and cells. Track positions consumed by row_span, assign each cell the
first available column, and apply each header cell across its complete row_span
and col_span before assigning coordinates and headers. Update the related logic
around node_data.cell_row_index and node_data.cell_column_index consistently.
- Around line 289-311: Update the accessibility-state serialization around
node_data.is_selected and node_data.checked_state so aria-selected is read and
serialized for every element, not only option elements. Separate aria-checked
handling from the HTMLInputElement type check, allowing valid ARIA values to
apply to inputs with overridden checkbox or switch roles while retaining native
input state when aria-checked is absent.
In `@Libraries/LibWeb/DOM/Document.cpp`:
- Around line 3683-3685: Update the active-element notification in Document’s
relevant focus-clearing path so page_did_change_active_element() is also invoked
when m_active_element is null, using the established clear/sentinel UniqueNodeID
or a dedicated focus-cleared notification. Preserve the existing unique_id()
behavior for non-null active elements and ensure the UI can fall back to the
document root/body.
In `@Libraries/LibWeb/DOM/Element.cpp`:
- Around line 3513-3523: Update Element::is_aria_hidden() and the duplicated
aria-hidden traversal in exclude_from_accessibility_tree() so only an attribute
value of "true" returns or blocks the walk; continue traversing ancestors when
the nearest value is "false" or any other value, preserving detection of an
ancestor’s aria-hidden="true".
In `@Libraries/LibWeb/DOM/Node.cpp`:
- Around line 3102-3109: Update the accessibility-tree pruning condition in the
surrounding Node traversal to stop treating a missing element->layout_node() as
sufficient for subtree exclusion. Retain pruning for aria-hidden and actual CSS
visibility/display-hidden semantics, while allowing display: contents elements
without layout nodes to expose their rendered descendants.
- Around line 3087-3095: Update the HTMLSelectElement handling in the
accessibility-tree construction to preserve optgroup hierarchy and existing
hidden/aria-hidden exclusion semantics. Traverse the select’s child structure
rather than appending every entry from list_of_options() directly, and bypass
only the layout-node requirement for option elements. Keep options nested under
their optgroup accessibility nodes while excluding hidden or aria-hidden
entries.
In `@Libraries/LibWeb/Page/Page.h`:
- Line 535: Update the Page::page_did_change_active_element callback to accept
Optional<Web::UniqueNodeID>, and adjust its callers to notify it when the active
element is cleared by passing an empty optional. Preserve passing the node ID
for non-null active elements so accessibility consumers receive both focus and
focus-cleared notifications.
In `@Libraries/LibWebView/AccessibilityNodeData.cpp`:
- Around line 103-104: Update the IPC decoding around checked_state and
expanded_state to decode each byte into a temporary value, validate it against
the valid CheckedState and ExpandedState ranges, and reject malformed values
before casting. Preserve the existing TRY-based failure propagation and only
construct the enums after validation.
In `@Libraries/LibWebView/AccessibilityNodeData.h`:
- Line 96: Replace the boolean is_pressed field in AccessibilityNodeData with a
tri-state enum analogous to CheckedState, representing false, true, and mixed
aria-pressed values. Update its IPC serialization and any consumers to preserve
and handle all three states.
In `@Libraries/LibWebView/AccessibilityTreeManager.cpp`:
- Around line 131-139: Update hit_test_recursive so it does not assume
current->child_ids reverse order represents visual paint order. Use the
layout/painting hit-testing data or an existing paint-order traversal that
accounts for z-index, positioned descendants, and stacking contexts, while
preserving the deepest-hit behavior and bounds fallback.
- Around line 41-47: Update the live-region change detection around m_nodes and
on_live_region_changed to handle newly inserted descendants instead of skipping
them when old_it is m_nodes.end(). Compare each live region’s accessible text or
subtree across generations, and announce newly added content while preserving
existing notifications for changed same-ID nodes.
In `@Services/WebContent/ConnectionFromClient.cpp`:
- Around line 1636-1667: Clamp AT-supplied offsets to the target text’s
character count and ensure start is no greater than end before calling
Utf16View::code_unit_offset_of or substring_view. Update the
convert_to_code_units lambda and the edit_value flow in the surrounding
connection handler, preserving caret behavior while preventing out-of-range
conversions and subtraction underflow.
In `@Tests/LibWeb/AccessibilityBridge/run_tests.sh`:
- Around line 138-157: Initialize LADYBIRD_RUNTIME_DIR_OWNED and XVFB_PID
locally to empty values before the runtime-directory setup and any Xvfb
acquisition in run_tests.sh. Ensure cleanup_outer only acts on ownership and
process markers established by this script, not inherited environment values.
In `@Tests/LibWeb/AccessibilityBridge/tests_orca/test_focus_changed.py`:
- Around line 53-62: Update the focus-handler test around _on_focused_changed to
stop catching and suppressing generic exceptions; allow TypeError, RuntimeError,
and other failures to propagate and fail the test. Retain only the targeted
AttributeError assertion, and explicitly add any missing synthetic-event fields
needed for the handler to execute successfully.
In `@UI/AppKit/Interface/LadybirdAccessibilityElement.mm`:
- Around line 958-966: Update the same-node branch in the text-marker range
handling to validate or clamp the offsets so a reversed range where
endData.offset is less than startData.offset cannot produce a negative or
underflowed substring length. Preserve normal extraction for ordered offsets and
ensure reversed ranges yield a safe empty or otherwise established clamped
result before calling substring_view.
- Around line 362-370: Propagate a secure-text flag through
AccessibilityNodeData for password inputs, including the text-leaf serialization
in AccessibilityTreeNode, and update the NSAccessibilityValueAttribute handling
in LadybirdAccessibilityElement to expose secure fields as secure text with a
masked value instead of returning data->name. Preserve existing behavior for
non-password text leaves and other accessibility values.
In `@UI/Qt/Application.cpp`:
- Around line 110-112: Update the customizations write flow using QSaveFile
instead of QFile so the existing orca-customizations.py remains intact until
replacement succeeds. Check the full write result and require commit() to
succeed, preserving the original file and handling failures without replacing
it.
In `@UI/Qt/OrcaScripts/orca_customizations_bootstrap.py`:
- Around line 34-35: Update the exception handling around custom-script loading
in the bootstrap flow: handle expected import failures separately, while logging
unexpected exceptions with traceback details before falling back to Orca’s
default script. Remove the silent `except Exception: pass` behavior so
constructor or import bugs remain diagnosable.
---
Minor comments:
In `@Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp`:
- Around line 383-386: Update the visited-links handling around the
HTMLAnchorElement check so anchor elements populate the node’s advertised
is_visited state using the anchor’s actual visited status. Remove the no-op cast
and preserve the default false value for non-anchor elements.
In `@Tests/LibWeb/Accessibility/expected/aria-multiselectable.txt`:
- Around line 3-9: Update the accessibility snapshot for the listbox options in
aria-multiselectable.txt to include each option’s serialized aria-selected
state: true for First and Second, and false for Third. Preserve the existing
role and name assertions while adding the selection markers under the
corresponding option entries.
In `@Tests/LibWeb/Accessibility/expected/aria-states.txt`:
- Around line 9-10: Update the accessibility snapshot entry for the custom
button named “ARIA disabled” to include the expected disabled/aria-disabled
assertion alongside its existing text leaf, preserving the fixture’s hierarchy
and formatting.
In `@Tests/LibWeb/Accessibility/expected/form-validation.txt`:
- Line 7: Update the password control entry in the accessibility snapshot to
include its textbox role alongside name="Password" editable required, matching
the username control’s representation and preserving the existing attributes.
In `@Tests/LibWeb/AccessibilityBridge/README.md`:
- Around line 47-62: Correct the README documentation to state that the runner
executes unittest rather than pytest, and replace the inaccurate universal
15-second timeout claim with the actual defaults: 60 seconds for Ladybird
startup and 5 seconds for descendant polling. Update the Flakiness budget
section while preserving its guidance to investigate slow behavior instead of
increasing timeouts.
In `@Tests/LibWeb/AccessibilityBridge/run_tests.sh`:
- Around line 116-127: Replace the hard-coded /tmp/xvfb.log redirection in the
Xvfb startup block with a private mktemp-created log file, retain its path for
diagnostics, and remove it in the EXIT cleanup trap. Update the failure message
to report the generated log path while preserving the existing startup timeout
behavior.
In `@Tests/LibWeb/AccessibilityBridge/tests_orca/test_active_document.py`:
- Around line 47-50: Update the assertion setup in the active-document test so
Atspi.Text.get_character_count is called only when first lacks a name and the
node supports the Text interface; named nodes must satisfy the assertion without
requiring Text. Preserve the existing failure message and validation of unnamed
nodes with text.
In `@Tests/LibWeb/AccessibilityBridge/tests_orca/test_say_all.py`:
- Around line 54-79: Strengthen test_paragraph_with_two_links_reads_in_dom_order
by asserting the exact contents tuple sequence, including pre-link, first link,
between-link, second link, and post-link entries in DOM order. Validate each
link tuple has role "link" and the expected non-empty link text, rather than
relying only on joined-text membership and link count.
In `@Tests/LibWeb/AccessibilityBridge/tests/test_attributes.py`:
- Around line 31-40: Update the exception handling in visit so bridge exceptions
are no longer silently ignored: re-raise the exception to fail the test, or
record the affected node in missing. Preserve the existing role filtering and
attribute validation behavior for successfully visited nodes.
In `@Tests/LibWeb/AccessibilityBridge/tests/test_interfaces.py`:
- Around line 71-91: The test test_named_section_advertises_text currently skips
its assertion because the fixture’s named section has a paragraph child. Add a
distinct empty aria-labeled section to the fixture and update the test’s target
name to select it, ensuring supports_interface(named, "text") is always
exercised.
In `@Tests/LibWeb/AccessibilityBridge/tests/test_states.py`:
- Around line 125-129: Update the is_disabled assertion in the accessibility
state test to require both Atspi.StateType.SENSITIVE and Atspi.StateType.ENABLED
to be absent, replacing the permissive OR condition with the appropriate
all-absent check while preserving the existing assertion message.
In `@Tests/LibWeb/AccessibilityBridgeMac/README.md`:
- Around line 31-39: Add language identifiers to all listed fenced code blocks:
label both command fences in Tests/LibWeb/AccessibilityBridgeMac/README.md
(lines 31-39) as shell, and label the architecture diagram, action-flow, and
Orca-to-IPC flow fences in Documentation/ScreenReaderSupport.md (lines 54-92,
402-406, and 412-419) with appropriate identifiers such as text.
- Around line 50-52: Align the “Flakiness budget” statement with the startup
timeout configured by the Ladybird harness: update the documentation to state
the actual 60-second default and describe how it can be overridden, or change
the harness default to 15 seconds and keep the documented budget consistent. Use
the harness startup-timeout configuration as the source of truth.
In `@Tests/LibWeb/AccessibilityBridgeMac/tests/test_roles.py`:
- Around line 190-206: Update test_landmark_role_descriptions to assert that
_find_by_subrole returns at least one hit for every listed landmark subrole
instead of calling skipTest when none are found. Keep the existing
role-description assertion using the first matching hit.
In `@Tests/LibWeb/AccessibilityBridgeMac/tests/test_search_predicate.py`:
- Around line 58-77: Strengthen test_forward_with_limit_caps_count and
test_forward_with_limit_1_returns_one by asserting that results are non-empty
for the first leaf fixture, while retaining the existing upper-bound checks. Use
the fixture’s known elements after leaves[0] so positive AXResultsLimit values
must return results rather than merely allowing an empty list.
In `@Tests/LibWeb/AccessibilityBridgeMac/tests/test_states.py`:
- Around line 29-37: Update _action_names to propagate exceptions from
AXUIElementCopyActionNames instead of catching and converting them to an empty
list, and raise or otherwise fail when the returned AX error code is nonzero.
Preserve the empty-list result only for valid successful queries with no action
names and for the existing elem is None case.
In `@Tests/LibWeb/AccessibilityBridgeMac/tests/test_text_markers.py`:
- Around line 210-245: Remove the skipTest branch in
test_marker_for_static_text_position_resolves_to_text when
AXTextMarkerForPosition returns nil. Assert that marker is not None so a failed
hit test fails the test, then retain the existing AXUIElementForTextMarker and
AXStaticText assertions.
- Around line 52-58: Update test_start_and_end_markers_are_distinct to resolve
start and end through AXUIElementForTextMarker, then assert the returned text
elements differ; remove the repr-based comparison while preserving the
fixture-specific distinctness assertion.
In `@Tests/LibWeb/AccessibilityBridgeMac/tests/test_tree_shape.py`:
- Around line 164-188: Update
test_no_ignored_role_node_is_an_accessibility_element to read each AXGroup’s
AXSubrole and exclude legitimate landmark groups before recording offenders.
Replace the arbitrary len(offenders) <= 5 assertion with a strict zero-offender
assertion, preserving the existing diagnostic message and child-role detection.
In `@UI/AppKit/Interface/LadybirdAccessibilityElement.mm`:
- Around line 23-26: Update createTextMarker so the bytes passed to
AXTextMarkerCreate do not include uninitialized TextMarkerData padding: use a
fixed zero-initialized byte buffer or zero-initialize the serialized storage
before assigning node_id and offset. Preserve the marker’s existing field values
and length while ensuring all copied bytes are initialized.
In `@UI/AppKit/Interface/LadybirdWebView.mm`:
- Around line 443-448: Remove the unconditional first-responder transfer from
the accessibility-tree update callback around the window/firstResponder logic in
LadybirdWebView. Keep responder changes confined to the explicit URL-bar Enter
handling path, while preserving the routine accessibility updates and allowing
onURLChange/onTitleChange callbacks to leave the current responder unchanged.
In `@UI/Qt/OrcaScripts/orca_customizations_bootstrap.py`:
- Line 6: Update the comment containing “user’s” to use an ASCII apostrophe
(`user's`) instead of the typographic apostrophe, preserving the comment’s
meaning and clearing Ruff RUF003.
In `@UI/Qt/Tab.cpp`:
- Around line 728-734: Update the initialization logic around the
on_loading_state_change callback and set_loading(view().is_loading()) so the
initial loading state also schedules an accessibility-tree request when the view
is already loading. Preserve the existing callback behavior for subsequent
loading-state changes without duplicating requests unnecessarily.
---
Nitpick comments:
In `@Libraries/LibWeb/DOM/Document.cpp`:
- Around line 6023-6041: Extract the shared AccessibilityTreeNode construction
and empty-document handling used by build_accessibility_node_data() and
dump_accessibility_tree_as_json() into a common helper. Update both callers to
reuse that helper while preserving the existing synthesized document-node
behavior and each function’s output-specific serialization.
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (10)
Tests/LibWeb/AccessibilityBridgeMac/tests/test_tree_shape.py-164-188 (1)
164-188: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not allow ignored-node leaks under an arbitrary threshold.
This test passes with up to five exact leak patterns, so a small regression remains undetected. Distinguish legitimate landmark groups using their subrole, then require the genuine offender list to be empty.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LibWeb/AccessibilityBridgeMac/tests/test_tree_shape.py` around lines 164 - 188, The test test_no_ignored_role_node_is_an_accessibility_element should exclude legitimate AXGroup landmark nodes by checking their AXSubrole before recording offenders, then require the remaining offender list to be empty instead of allowing an arbitrary threshold. Preserve the existing child-role pattern used to identify unnamed paragraph or generic leaks.Tests/LibWeb/AccessibilityBridge/harness/orca.py-168-176 (1)
168-176: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDo not silently retain the previous active Orca script.
If
set_active_script(None, ...)fails, this catch hides the failure and leaves process-global state that contaminates subsequent tests. Clearself._scriptinfinally, but allow the deregistration failure to fail the test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LibWeb/AccessibilityBridge/harness/orca.py` around lines 168 - 176, Update the teardown logic around set_active_script in OrcaSession so deregistration exceptions are not swallowed; remove the broad exception suppression and ensure self._script is cleared in a finally block regardless of whether deregistration succeeds.Tests/LibWeb/AccessibilityBridge/harness/ladybird.py-133-134 (1)
133-134: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign the startup timeout with the 15-second flakiness budget.
The default is 60 seconds, while the test README says anything beyond 15 seconds must fail and be investigated. Use 15 seconds by default, retaining the environment override for exceptional debugging.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LibWeb/AccessibilityBridge/harness/ladybird.py` around lines 133 - 134, Update the startup_timeout default in the Ladybird AT-SPI2 startup flow to 15 seconds instead of 60, while preserving the LADYBIRD_AT_SPI2_STARTUP_TIMEOUT environment-variable override for debugging.Tests/LibWeb/AccessibilityBridge/README.md-47-51 (1)
47-51: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winClarify that isolation is opt-in outside CI.
Line 47 says the runner always creates private X and D-Bus sessions, but Line 51 says this happens only with
CIor--isolated. Update Line 47 so local users know the default uses their live AT-SPI2 session.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LibWeb/AccessibilityBridge/README.md` around lines 47 - 51, Update the introductory runner description in the README to state that private X, D-Bus, and AT-SPI2 sessions are created only in CI or when `--isolated` is used; clarify that local runs otherwise use the existing live AT-SPI2 session, while preserving the existing isolation details.Tests/LibWeb/AccessibilityBridge/tests/test_states.py-121-129 (1)
121-129: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRequire both enabled-state flags to be absent.
The current
orpasses if only one ofSENSITIVEorENABLEDis missing, despite the documented contract requiring neither state.Proposed fix
- is_disabled = not _has_state(disabled, Atspi.StateType.SENSITIVE) or not _has_state( - disabled, Atspi.StateType.ENABLED - ) - self.assertTrue(is_disabled, "disabled button must *not* have SENSITIVE+ENABLED states") + self.assertFalse(_has_state(disabled, Atspi.StateType.SENSITIVE)) + self.assertFalse(_has_state(disabled, Atspi.StateType.ENABLED))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LibWeb/AccessibilityBridge/tests/test_states.py` around lines 121 - 129, Update test_disabled_button_has_not_sensitive_state so the disabled-state assertion requires both Atspi.StateType.SENSITIVE and Atspi.StateType.ENABLED to be absent; replace the current any-missing logic with an all-absent check while preserving the existing error message and state lookup behavior.Tests/LibWeb/AccessibilityBridge/tests/test_attributes.py-27-43 (1)
27-43: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not silently omit nodes from the tag invariant.
Catching and discarding every exception lets this test pass when querying a node fails—particularly when
get_name()throws after detecting a missing tag. Let the test fail instead.Proposed fix
def visit(obj, _depth): - try: - role = obj.get_role_name() - attrs = get_attributes_dict(obj) - if role in ("document web", "filler"): - return - if "tag" not in attrs: - missing.append((role, obj.get_name())) - except Exception: - pass + role = obj.get_role_name() + attrs = get_attributes_dict(obj) + if role in ("document web", "filler"): + return + if "tag" not in attrs: + missing.append((role, obj.get_name()))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LibWeb/AccessibilityBridge/tests/test_attributes.py` around lines 27 - 43, The test_every_role_exposes_tag traversal must not suppress node-query failures. Remove the broad exception handling in visit so errors from get_role_name, get_attributes_dict, or get_name propagate and fail the test, while retaining the existing document web/filler exclusions and missing-tag assertion.Source: Linters/SAST tools
Tests/LibWeb/AccessibilityBridge/tests/test_interfaces.py-71-91 (1)
71-91: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEnsure this test exercises the named-section rule.
The current fixture has a child, so the conditional skips the only assertion. Add a dedicated childless named section, assert that it was found, and check its Text interface unconditionally.
walk(self.doc, visit) - if named is not None and named.get_child_count() == 0: - self.assertTrue(supports_interface(named, "text")) + self.assertIsNotNone(named, "fixture must contain a childless named section") + self.assertEqual(named.get_child_count(), 0) + self.assertTrue(supports_interface(named, "text"))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LibWeb/AccessibilityBridge/tests/test_interfaces.py` around lines 71 - 91, The test fixture and test_named_section_advertises_text should explicitly include a childless named section, separate from the existing named section with a paragraph child. Locate that childless section by its distinct accessible name, assert it was found, and unconditionally verify it supports the Text interface; remove the child-count guard and skip behavior.Tests/LibWeb/AccessibilityBridge/run_tests.sh-114-127 (1)
114-127: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winOther (CWE-377): Insecure Temporary File
Reachability: External
Use a unique file for the Xvfb log.
A local user can pre-create
/tmp/xvfb.logas a symlink, causing this script to truncate another file writable by the test-running user. Usemktempand remove the generated log during cleanup.Proposed fix
+XVFB_LOG="" XVFB_SOCKET="/tmp/.X11-unix/X${XVFB_DISPLAY#:}" if [[ ! -S "${XVFB_SOCKET}" ]]; then - Xvfb "${XVFB_DISPLAY}" -screen 0 "${XVFB_SCREEN}" -nolisten tcp >/tmp/xvfb.log 2>&1 & + XVFB_LOG="$(mktemp -t ladybird-xvfb-XXXXXX.log)" + Xvfb "${XVFB_DISPLAY}" -screen 0 "${XVFB_SCREEN}" -nolisten tcp >"${XVFB_LOG}" 2>&1 & XVFB_PID=$! - trap 'kill ${XVFB_PID} 2>/dev/null || true' EXIT + trap 'kill "${XVFB_PID}" 2>/dev/null || true; [[ -z "${XVFB_LOG:-}" ]] || rm -f "${XVFB_LOG}"' EXIT ... - echo " log: /tmp/xvfb.log" >&2 + echo " log: ${XVFB_LOG}" >&2🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LibWeb/AccessibilityBridge/run_tests.sh` around lines 114 - 127, Use mktemp to create a unique Xvfb log file before launching Xvfb, pass that path to its output redirection, and remove the generated file in the existing EXIT trap alongside terminating XVFB_PID. Update the startup failure message to reference the unique log path instead of /tmp/xvfb.log.Source: Linters/SAST tools
Tests/LibWeb/AccessibilityBridge/tests_orca/test_say_all.py-40-79 (1)
40-79: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert link text and actual DOM ordering.
These tests currently pass when link text is absent or sentence fragments are shuffled. Verify the one-link text and the complete
pre → first link → between → second link → postsequence.Example assertions
strings = [text for (_o, _s, _e, text) in contents] joined = " ".join(strings) self.assertIn("A paragraph with", joined, ...) + self.assertIn("one", joined, f"link text missing: {strings!r}") self.assertIn("link embedded", joined, ...) ... joined = " ".join(strings) + fragments = ["Paragraph with", "first", "and", "second", "links."] + positions = [joined.find(fragment) for fragment in fragments] + self.assertNotIn(-1, positions, f"sentence fragment missing: {strings!r}") + self.assertEqual(positions, sorted(positions), f"sentence contents are out of DOM order: {strings!r}") ... link_objs = [...] + self.assertEqual([obj.get_name() for obj in link_objs[:2]], ["first", "second"])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LibWeb/AccessibilityBridge/tests_orca/test_say_all.py` around lines 40 - 79, Strengthen test_paragraph_with_one_link_contents_include_pre_and_post_prose to assert the link’s actual text appears in sentence contents, not just surrounding prose. Update test_paragraph_with_two_links_reads_in_dom_order to verify the complete pre-prose → first-link text → between-prose → second-link text → post-prose ordering, while retaining the link-object checks.Tests/LibWeb/AccessibilityBridgeMac/harness/ladybird.py-141-142 (1)
141-142: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUse one macOS accessibility startup budget. The runner permits 60 seconds while the documentation defines anything over 15 seconds as flakiness.
Tests/LibWeb/AccessibilityBridgeMac/harness/ladybird.py#L141-L142: change the default to 15 seconds, or intentionally establish a different budget.Tests/LibWeb/AccessibilityBridgeMac/README.md#L50-L52: document the same timeout used by the runner.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LibWeb/AccessibilityBridgeMac/harness/ladybird.py` around lines 141 - 142, The macOS accessibility startup timeout is inconsistent with the documented 15-second flakiness threshold. Update the startup_timeout default used by wait_for in ladybird.py to 15 seconds, and update the corresponding timeout documentation in Tests/LibWeb/AccessibilityBridgeMac/README.md lines 50-52 to match.
🧹 Nitpick comments (2)
.github/workflows/lagom-template.yml (1)
344-352: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid running
LibWebAccessibilityBridgetwice in Release CI.The generic non-Sanitizer
Teststep already runs the complete Release CTest suite. Either remove this dedicated invocation or exclude this test from the generic step if a separately named job is required.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/lagom-template.yml around lines 344 - 352, Remove the dedicated “Test - LibWeb AT-SPI2 bridge (Release)” step invoking LibWebAccessibilityBridge, since the generic non-Sanitizer Test step already runs the complete Release CTest suite. Preserve the existing AT-SPI2 execution for configurations where it is not covered by that generic Release suite.Tests/LibWeb/AccessibilityBridge/tests/test_hypertext_fallback.py (1)
24-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNarrow the fallback to
GLib.Error.
Atspi.Text.get_character_count()can raise a PyGObjectGLib.Errorfor unsupported text objects; catchingExceptionhere also hides unrelated test bugs. ImportGLiband handle that specific error instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Tests/LibWeb/AccessibilityBridge/tests/test_hypertext_fallback.py` around lines 24 - 29, Update _full_text to import GLib and catch only GLib.Error around Atspi.Text.get_character_count; preserve the empty-string fallback for unsupported text objects while allowing unrelated exceptions to propagate.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/lagom-template.yml:
- Around line 219-249: Update the Accessibility grant loop to include the
configured interpreter at ${{ env.pythonLocation }}/bin/python alongside the
existing candidates. In the AX preflight step, use that same configured
executable for the ApplicationServices trust probe instead of selecting an
unrelated PATH interpreter, while preserving the existing fallback handling and
validation behavior.
- Around line 274-275: Update the macOS Sanitizer conditions for the “Build with
Qt” and general “Test” steps in the workflow so non-self-hosted macOS runners do
not skip the general ctest suite. Keep any self-hosted-specific behavior intact,
but ensure the normal Sanitizer Test step remains enabled for non-self-hosted
macOS instead of running only AccessibilityBridgeMac.
In `@Documentation/ScreenReaderSupport.md`:
- Around line 1142-1163: The WebDocument search in the child traversal
incorrectly limits each node to five children instead of five nesting levels.
Update the stack traversal around the local stack in the relations-building loop
to store each node with its depth, traverse all children, and stop expanding
nodes once depth five is reached while preserving WebDocument detection and
relation creation.
In `@Libraries/LibWeb/DOM/Document.cpp`:
- Around line 3683-3685: Update the active-element change handling around
page_did_change_active_element so the callback is invoked for both non-null and
null m_active_element transitions. Pass the existing clear/sentinel value
expected by page_did_change_active_element when m_active_element is null, while
preserving unique_id() for non-null elements.
In `@Tests/LibWeb/AccessibilityBridge/tests_orca/test_focus_changed.py`:
- Around line 53-62: Update the _on_focused_changed invocation in the
focus-change tests to remove the broad Exception handler. Allow unexpected
exceptions to fail the test, retaining only the explicit AttributeError
assertion unless a specifically verified synthetic-event exception must be
suppressed.
In `@UI/Qt/AccessibilityInterface.cpp`:
- Around line 107-120: Update AccessibilityInterface::~AccessibilityInterface()
to delete its m_object backing QObject, centralizing QObject cleanup when the
interface is destroyed. In UI/Qt/AccessibilityInterface.cpp lines 107-120,
ensure the destructor performs this cleanup; in UI/Qt/WebContentView.cpp lines
1076-1081, retain the existing deregistration and ensure it deletes interfaces
through the corrected destructor.
- Around line 850-878: Update the ignored-child handling in the offset resolver
around offset_to_y_in_container to recursively promote nested unnamed generic
nodes, matching build_hypertext(). Reuse the same traversal semantics for
descendants so deeply nested ignored spans contribute their text and boundaries
in document order, preserving correct offset-to-coordinate mapping.
- Around line 896-915: Update compute_visual_line_starts to use the documented
half-line-height tolerance when comparing each character’s y-coordinate with
prev_y, rather than starting a new line for every difference. Reuse the
available line-height metric, treat differences within half that height as the
same visual line, and preserve the existing handling for unavailable coordinates
and genuine line transitions.
- Around line 235-263: Update the QAccessible::TextInterface handling to
preserve a text representation for unnamed containers such as article and
blockquote. In the role checks surrounding node_data(), grant an unnamed
container TextInterface when its hypertext contains textual content, or
otherwise keep its text-leaf children exposed; do not alter the existing
paragraph, leaf-like role, or named leafless-node behavior.
- Around line 1122-1127: Update columnIndex() to advance the logical column by
each preceding sibling cell’s column_span rather than by one. Keep returning the
accumulated column when sibling_id matches m_node_id, and use the existing cell
span data with an appropriate default for non-spanning cells.
- Around line 379-476: Update AccessibilityInterface::map_role() to explicitly
map the remaining interactive roles spinbutton, menuitemcheckbox, menuitemradio,
treeitem, option, and switch to their corresponding QAccessible control roles
before the fallback return of QAccessible::Section. Preserve the existing
mappings and ensure each role retains its role-specific accessibility semantics.
- Around line 347-354: Propagate a protected-text/password flag from DOM
accessibility nodes through the text-leaf and hypertext data paths, and have
QAccessible expose masked characters instead of verbatim text for password
nodes. Update AccessibilityInterface handling around QAccessible::Value and
state reporting to set QAccessible::State::passwordEdit, while preserving normal
text/value behavior for non-password nodes.
In `@UI/Qt/OrcaScripts/Ladybird/script_utilities.py`:
- Around line 551-601: Update active_document so the super().active_document()
fast-path result is accepted and cached only when it is alive and its parent is
showing; otherwise continue into the existing cached-document and tree-search
fallback. Preserve the current behavior for valid active documents and ensure
stale or inactive-tab results are rejected.
In `@UI/Qt/WebContentView.cpp`:
- Around line 193-197: Update the delayed callback created in the initial
accessibility-focus path of WebContentView so it checks isVisible() at execution
time before calling notify_accessibility_focus_on_document_root(). Apply the
same guard to the corresponding callback in the other affected path, while
preserving the existing posted-focus state handling.
---
Minor comments:
In `@Tests/LibWeb/AccessibilityBridge/harness/ladybird.py`:
- Around line 133-134: Update the startup_timeout default in the Ladybird
AT-SPI2 startup flow to 15 seconds instead of 60, while preserving the
LADYBIRD_AT_SPI2_STARTUP_TIMEOUT environment-variable override for debugging.
In `@Tests/LibWeb/AccessibilityBridge/harness/orca.py`:
- Around line 168-176: Update the teardown logic around set_active_script in
OrcaSession so deregistration exceptions are not swallowed; remove the broad
exception suppression and ensure self._script is cleared in a finally block
regardless of whether deregistration succeeds.
In `@Tests/LibWeb/AccessibilityBridge/README.md`:
- Around line 47-51: Update the introductory runner description in the README to
state that private X, D-Bus, and AT-SPI2 sessions are created only in CI or when
`--isolated` is used; clarify that local runs otherwise use the existing live
AT-SPI2 session, while preserving the existing isolation details.
In `@Tests/LibWeb/AccessibilityBridge/run_tests.sh`:
- Around line 114-127: Use mktemp to create a unique Xvfb log file before
launching Xvfb, pass that path to its output redirection, and remove the
generated file in the existing EXIT trap alongside terminating XVFB_PID. Update
the startup failure message to reference the unique log path instead of
/tmp/xvfb.log.
In `@Tests/LibWeb/AccessibilityBridge/tests_orca/test_say_all.py`:
- Around line 40-79: Strengthen
test_paragraph_with_one_link_contents_include_pre_and_post_prose to assert the
link’s actual text appears in sentence contents, not just surrounding prose.
Update test_paragraph_with_two_links_reads_in_dom_order to verify the complete
pre-prose → first-link text → between-prose → second-link text → post-prose
ordering, while retaining the link-object checks.
In `@Tests/LibWeb/AccessibilityBridge/tests/test_attributes.py`:
- Around line 27-43: The test_every_role_exposes_tag traversal must not suppress
node-query failures. Remove the broad exception handling in visit so errors from
get_role_name, get_attributes_dict, or get_name propagate and fail the test,
while retaining the existing document web/filler exclusions and missing-tag
assertion.
In `@Tests/LibWeb/AccessibilityBridge/tests/test_interfaces.py`:
- Around line 71-91: The test fixture and test_named_section_advertises_text
should explicitly include a childless named section, separate from the existing
named section with a paragraph child. Locate that childless section by its
distinct accessible name, assert it was found, and unconditionally verify it
supports the Text interface; remove the child-count guard and skip behavior.
In `@Tests/LibWeb/AccessibilityBridge/tests/test_states.py`:
- Around line 121-129: Update test_disabled_button_has_not_sensitive_state so
the disabled-state assertion requires both Atspi.StateType.SENSITIVE and
Atspi.StateType.ENABLED to be absent; replace the current any-missing logic with
an all-absent check while preserving the existing error message and state lookup
behavior.
In `@Tests/LibWeb/AccessibilityBridgeMac/harness/ladybird.py`:
- Around line 141-142: The macOS accessibility startup timeout is inconsistent
with the documented 15-second flakiness threshold. Update the startup_timeout
default used by wait_for in ladybird.py to 15 seconds, and update the
corresponding timeout documentation in
Tests/LibWeb/AccessibilityBridgeMac/README.md lines 50-52 to match.
In `@Tests/LibWeb/AccessibilityBridgeMac/tests/test_tree_shape.py`:
- Around line 164-188: The test
test_no_ignored_role_node_is_an_accessibility_element should exclude legitimate
AXGroup landmark nodes by checking their AXSubrole before recording offenders,
then require the remaining offender list to be empty instead of allowing an
arbitrary threshold. Preserve the existing child-role pattern used to identify
unnamed paragraph or generic leaks.
---
Nitpick comments:
In @.github/workflows/lagom-template.yml:
- Around line 344-352: Remove the dedicated “Test - LibWeb AT-SPI2 bridge
(Release)” step invoking LibWebAccessibilityBridge, since the generic
non-Sanitizer Test step already runs the complete Release CTest suite. Preserve
the existing AT-SPI2 execution for configurations where it is not covered by
that generic Release suite.
In `@Tests/LibWeb/AccessibilityBridge/tests/test_hypertext_fallback.py`:
- Around line 24-29: Update _full_text to import GLib and catch only GLib.Error
around Atspi.Text.get_character_count; preserve the empty-string fallback for
unsupported text objects while allowing unrelated exceptions to propagate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ebf8edb2-77e0-4c07-90f4-f14329d8c020
📒 Files selected for processing (229)
.github/actions/setup/action.yml.github/workflows/ci.yml.github/workflows/lagom-template.ymlCMakeLists.txtDocumentation/ScreenReaderSupport.mdLibraries/LibWeb/CSS/SelectorMatching.cppLibraries/LibWeb/DOM/AccessibilityTreeNode.cppLibraries/LibWeb/DOM/AccessibilityTreeNode.hLibraries/LibWeb/DOM/Document.cppLibraries/LibWeb/DOM/Document.hLibraries/LibWeb/DOM/Element.cppLibraries/LibWeb/DOM/Element.hLibraries/LibWeb/DOM/Node.cppLibraries/LibWeb/HTML/HTMLTableElement.hLibraries/LibWeb/Page/Page.hLibraries/LibWebView/AccessibilityNodeData.cppLibraries/LibWebView/AccessibilityNodeData.hLibraries/LibWebView/AccessibilityTreeManager.cppLibraries/LibWebView/AccessibilityTreeManager.hLibraries/LibWebView/CMakeLists.txtLibraries/LibWebView/Forward.hLibraries/LibWebView/PageInfo.hLibraries/LibWebView/ViewImplementation.cppLibraries/LibWebView/ViewImplementation.hLibraries/LibWebView/WebContentClient.cppLibraries/LibWebView/WebContentClient.hServices/WebContent/ConnectionFromClient.cppServices/WebContent/ConnectionFromClient.hServices/WebContent/PageClient.cppServices/WebContent/PageClient.hServices/WebContent/WebContentClient.ipcServices/WebContent/WebContentServer.ipcTests/LibWeb/Accessibility/expected/accesskey.txtTests/LibWeb/Accessibility/expected/aria-checked-override.txtTests/LibWeb/Accessibility/expected/aria-current.txtTests/LibWeb/Accessibility/expected/aria-described.txtTests/LibWeb/Accessibility/expected/aria-description.txtTests/LibWeb/Accessibility/expected/aria-hidden-inheritance.txtTests/LibWeb/Accessibility/expected/aria-labels.txtTests/LibWeb/Accessibility/expected/aria-live-assertive.txtTests/LibWeb/Accessibility/expected/aria-live-regions.txtTests/LibWeb/Accessibility/expected/aria-multiselectable.txtTests/LibWeb/Accessibility/expected/aria-pressed.txtTests/LibWeb/Accessibility/expected/aria-roles.txtTests/LibWeb/Accessibility/expected/aria-states.txtTests/LibWeb/Accessibility/expected/basic-structure.txtTests/LibWeb/Accessibility/expected/checkbox-states.txtTests/LibWeb/Accessibility/expected/complex-form.txtTests/LibWeb/Accessibility/expected/deeply-nested-landmarks.txtTests/LibWeb/Accessibility/expected/details-expanded.txtTests/LibWeb/Accessibility/expected/dialog.txtTests/LibWeb/Accessibility/expected/display-contents.txtTests/LibWeb/Accessibility/expected/document-language.txtTests/LibWeb/Accessibility/expected/empty-and-hidden.txtTests/LibWeb/Accessibility/expected/form-controls.txtTests/LibWeb/Accessibility/expected/form-validation.txtTests/LibWeb/Accessibility/expected/generic-transparency.txtTests/LibWeb/Accessibility/expected/heading-and-emoji.txtTests/LibWeb/Accessibility/expected/heading-levels.txtTests/LibWeb/Accessibility/expected/headings.txtTests/LibWeb/Accessibility/expected/hidden-content.txtTests/LibWeb/Accessibility/expected/image-alt.txtTests/LibWeb/Accessibility/expected/images.txtTests/LibWeb/Accessibility/expected/inline-text.txtTests/LibWeb/Accessibility/expected/input-states.txtTests/LibWeb/Accessibility/expected/interactive-elements.txtTests/LibWeb/Accessibility/expected/landmarks.txtTests/LibWeb/Accessibility/expected/link-url.txtTests/LibWeb/Accessibility/expected/links-and-buttons.txtTests/LibWeb/Accessibility/expected/lists.txtTests/LibWeb/Accessibility/expected/menu-and-menubar.txtTests/LibWeb/Accessibility/expected/mixed-content.txtTests/LibWeb/Accessibility/expected/multiple-tables.txtTests/LibWeb/Accessibility/expected/named-generic.txtTests/LibWeb/Accessibility/expected/nested-lists.txtTests/LibWeb/Accessibility/expected/nested-structure.txtTests/LibWeb/Accessibility/expected/radio-and-select.txtTests/LibWeb/Accessibility/expected/role-presentation-conflict.txtTests/LibWeb/Accessibility/expected/role-presentation.txtTests/LibWeb/Accessibility/expected/select-hidden-options.txtTests/LibWeb/Accessibility/expected/select-optgroup.txtTests/LibWeb/Accessibility/expected/semantic-sections.txtTests/LibWeb/Accessibility/expected/separator-and-roles.txtTests/LibWeb/Accessibility/expected/skip-nav.txtTests/LibWeb/Accessibility/expected/table-complex.txtTests/LibWeb/Accessibility/expected/table-headers.txtTests/LibWeb/Accessibility/expected/table-no-header.txtTests/LibWeb/Accessibility/expected/table-single-row.txtTests/LibWeb/Accessibility/expected/table-spans.txtTests/LibWeb/Accessibility/expected/tables.txtTests/LibWeb/Accessibility/expected/text-formatting.txtTests/LibWeb/Accessibility/expected/text-values.txtTests/LibWeb/Accessibility/expected/values-range.txtTests/LibWeb/Accessibility/input/accesskey.htmlTests/LibWeb/Accessibility/input/aria-checked-override.htmlTests/LibWeb/Accessibility/input/aria-current.htmlTests/LibWeb/Accessibility/input/aria-described.htmlTests/LibWeb/Accessibility/input/aria-description.htmlTests/LibWeb/Accessibility/input/aria-hidden-inheritance.htmlTests/LibWeb/Accessibility/input/aria-labels.htmlTests/LibWeb/Accessibility/input/aria-live-assertive.htmlTests/LibWeb/Accessibility/input/aria-live-regions.htmlTests/LibWeb/Accessibility/input/aria-multiselectable.htmlTests/LibWeb/Accessibility/input/aria-pressed.htmlTests/LibWeb/Accessibility/input/aria-roles.htmlTests/LibWeb/Accessibility/input/aria-states.htmlTests/LibWeb/Accessibility/input/basic-structure.htmlTests/LibWeb/Accessibility/input/checkbox-states.htmlTests/LibWeb/Accessibility/input/complex-form.htmlTests/LibWeb/Accessibility/input/deeply-nested-landmarks.htmlTests/LibWeb/Accessibility/input/details-expanded.htmlTests/LibWeb/Accessibility/input/dialog.htmlTests/LibWeb/Accessibility/input/display-contents.htmlTests/LibWeb/Accessibility/input/document-language.htmlTests/LibWeb/Accessibility/input/empty-and-hidden.htmlTests/LibWeb/Accessibility/input/form-controls.htmlTests/LibWeb/Accessibility/input/form-validation.htmlTests/LibWeb/Accessibility/input/generic-transparency.htmlTests/LibWeb/Accessibility/input/heading-and-emoji.htmlTests/LibWeb/Accessibility/input/heading-levels.htmlTests/LibWeb/Accessibility/input/headings.htmlTests/LibWeb/Accessibility/input/hidden-content.htmlTests/LibWeb/Accessibility/input/image-alt.htmlTests/LibWeb/Accessibility/input/images.htmlTests/LibWeb/Accessibility/input/inline-text.htmlTests/LibWeb/Accessibility/input/input-states.htmlTests/LibWeb/Accessibility/input/interactive-elements.htmlTests/LibWeb/Accessibility/input/landmarks.htmlTests/LibWeb/Accessibility/input/link-url.htmlTests/LibWeb/Accessibility/input/links-and-buttons.htmlTests/LibWeb/Accessibility/input/lists.htmlTests/LibWeb/Accessibility/input/menu-and-menubar.htmlTests/LibWeb/Accessibility/input/mixed-content.htmlTests/LibWeb/Accessibility/input/multiple-tables.htmlTests/LibWeb/Accessibility/input/named-generic.htmlTests/LibWeb/Accessibility/input/nested-lists.htmlTests/LibWeb/Accessibility/input/nested-structure.htmlTests/LibWeb/Accessibility/input/radio-and-select.htmlTests/LibWeb/Accessibility/input/role-presentation-conflict.htmlTests/LibWeb/Accessibility/input/role-presentation.htmlTests/LibWeb/Accessibility/input/select-hidden-options.htmlTests/LibWeb/Accessibility/input/select-optgroup.htmlTests/LibWeb/Accessibility/input/semantic-sections.htmlTests/LibWeb/Accessibility/input/separator-and-roles.htmlTests/LibWeb/Accessibility/input/skip-nav.htmlTests/LibWeb/Accessibility/input/table-complex.htmlTests/LibWeb/Accessibility/input/table-headers.htmlTests/LibWeb/Accessibility/input/table-no-header.htmlTests/LibWeb/Accessibility/input/table-single-row.htmlTests/LibWeb/Accessibility/input/table-spans.htmlTests/LibWeb/Accessibility/input/tables.htmlTests/LibWeb/Accessibility/input/text-formatting.htmlTests/LibWeb/Accessibility/input/text-values.htmlTests/LibWeb/Accessibility/input/values-range.htmlTests/LibWeb/AccessibilityBridge/.gitignoreTests/LibWeb/AccessibilityBridge/CMakeLists.txtTests/LibWeb/AccessibilityBridge/README.mdTests/LibWeb/AccessibilityBridge/harness/__init__.pyTests/LibWeb/AccessibilityBridge/harness/ladybird.pyTests/LibWeb/AccessibilityBridge/harness/orca.pyTests/LibWeb/AccessibilityBridge/harness/testcase.pyTests/LibWeb/AccessibilityBridge/harness/tree.pyTests/LibWeb/AccessibilityBridge/input/abbreviations.htmlTests/LibWeb/AccessibilityBridge/input/listitems.htmlTests/LibWeb/AccessibilityBridge/input/paragraphs.htmlTests/LibWeb/AccessibilityBridge/input/roles.htmlTests/LibWeb/AccessibilityBridge/input/states.htmlTests/LibWeb/AccessibilityBridge/input/tables.htmlTests/LibWeb/AccessibilityBridge/input/tree_shape.htmlTests/LibWeb/AccessibilityBridge/run_tests.shTests/LibWeb/AccessibilityBridge/tests/__init__.pyTests/LibWeb/AccessibilityBridge/tests/test_attributes.pyTests/LibWeb/AccessibilityBridge/tests/test_character_rect.pyTests/LibWeb/AccessibilityBridge/tests/test_hypertext_fallback.pyTests/LibWeb/AccessibilityBridge/tests/test_interfaces.pyTests/LibWeb/AccessibilityBridge/tests/test_lifecycle.pyTests/LibWeb/AccessibilityBridge/tests/test_line_boundary.pyTests/LibWeb/AccessibilityBridge/tests/test_regressions.pyTests/LibWeb/AccessibilityBridge/tests/test_roles.pyTests/LibWeb/AccessibilityBridge/tests/test_states.pyTests/LibWeb/AccessibilityBridge/tests/test_tables.pyTests/LibWeb/AccessibilityBridge/tests/test_text_content.pyTests/LibWeb/AccessibilityBridge/tests/test_tree_shape.pyTests/LibWeb/AccessibilityBridge/tests_orca/__init__.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_active_document.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_collection_fallback.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_focus_changed.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_hypertext_fallback.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_say_all.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_structural_navigation.pyTests/LibWeb/AccessibilityBridgeMac/CMakeLists.txtTests/LibWeb/AccessibilityBridgeMac/README.mdTests/LibWeb/AccessibilityBridgeMac/harness/__init__.pyTests/LibWeb/AccessibilityBridgeMac/harness/ladybird.pyTests/LibWeb/AccessibilityBridgeMac/harness/testcase.pyTests/LibWeb/AccessibilityBridgeMac/harness/tree.pyTests/LibWeb/AccessibilityBridgeMac/run_tests.shTests/LibWeb/AccessibilityBridgeMac/tests/__init__.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_lifecycle.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_regressions.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_roles.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_search_predicate.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_states.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_text_markers.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_tree_shape.pyTests/LibWeb/CMakeLists.txtTests/LibWeb/test-web/TestWeb.hTests/LibWeb/test-web/main.cppUI/AppKit/CMakeLists.txtUI/AppKit/Interface/LadybirdAccessibilityElement.hUI/AppKit/Interface/LadybirdAccessibilityElement.mmUI/AppKit/Interface/LadybirdAccessibilityViewProtocol.hUI/AppKit/Interface/LadybirdWebView.hUI/AppKit/Interface/LadybirdWebView.mmUI/Qt/AccessibilityInterface.cppUI/Qt/AccessibilityInterface.hUI/Qt/Application.cppUI/Qt/CMakeLists.txtUI/Qt/OrcaScripts/Ladybird/__init__.pyUI/Qt/OrcaScripts/Ladybird/script.pyUI/Qt/OrcaScripts/Ladybird/script_utilities.pyUI/Qt/OrcaScripts/orca_customizations_bootstrap.pyUI/Qt/Tab.cppUI/Qt/WebContentView.cppUI/Qt/WebContentView.hUI/Qt/WebContentViewAccessibility.hUI/Qt/WebContentViewAccessibility.mmUI/Qt/ladybird.qrcpyproject.toml
🚧 Files skipped from review as they are similar to previous changes (187)
- Tests/LibWeb/Accessibility/expected/aria-pressed.txt
- Tests/LibWeb/Accessibility/expected/links-and-buttons.txt
- Tests/LibWeb/Accessibility/expected/deeply-nested-landmarks.txt
- Tests/LibWeb/Accessibility/input/aria-description.html
- Tests/LibWeb/AccessibilityBridge/input/paragraphs.html
- Tests/LibWeb/Accessibility/input/separator-and-roles.html
- Tests/LibWeb/Accessibility/expected/radio-and-select.txt
- Tests/LibWeb/AccessibilityBridge/tests_orca/init.py
- Tests/LibWeb/Accessibility/input/accesskey.html
- Tests/LibWeb/Accessibility/input/multiple-tables.html
- Tests/LibWeb/Accessibility/input/complex-form.html
- Tests/LibWeb/Accessibility/expected/interactive-elements.txt
- Tests/LibWeb/Accessibility/input/inline-text.html
- Tests/LibWeb/Accessibility/expected/document-language.txt
- Tests/LibWeb/Accessibility/input/menu-and-menubar.html
- Tests/LibWeb/Accessibility/input/aria-roles.html
- Tests/LibWeb/Accessibility/input/tables.html
- Tests/LibWeb/Accessibility/expected/aria-current.txt
- Tests/LibWeb/Accessibility/expected/aria-labels.txt
- UI/Qt/OrcaScripts/Ladybird/init.py
- Tests/LibWeb/AccessibilityBridge/.gitignore
- Tests/LibWeb/Accessibility/expected/skip-nav.txt
- Tests/LibWeb/Accessibility/input/table-complex.html
- Tests/LibWeb/Accessibility/expected/form-controls.txt
- Tests/LibWeb/Accessibility/input/link-url.html
- Tests/LibWeb/Accessibility/expected/menu-and-menubar.txt
- Tests/LibWeb/AccessibilityBridge/input/listitems.html
- Tests/LibWeb/Accessibility/input/details-expanded.html
- Tests/LibWeb/AccessibilityBridgeMac/tests/init.py
- Tests/LibWeb/Accessibility/input/nested-structure.html
- Tests/LibWeb/Accessibility/input/aria-live-regions.html
- Tests/LibWeb/Accessibility/input/table-spans.html
- Tests/LibWeb/Accessibility/input/select-optgroup.html
- Tests/LibWeb/Accessibility/expected/basic-structure.txt
- Libraries/LibWebView/CMakeLists.txt
- Tests/LibWeb/Accessibility/expected/table-no-header.txt
- Tests/LibWeb/Accessibility/expected/headings.txt
- Tests/LibWeb/Accessibility/expected/multiple-tables.txt
- Tests/LibWeb/Accessibility/expected/empty-and-hidden.txt
- Tests/LibWeb/Accessibility/expected/aria-hidden-inheritance.txt
- Tests/LibWeb/Accessibility/expected/images.txt
- UI/AppKit/Interface/LadybirdAccessibilityElement.h
- Tests/LibWeb/AccessibilityBridge/input/abbreviations.html
- Tests/LibWeb/Accessibility/input/basic-structure.html
- Tests/LibWeb/Accessibility/expected/table-headers.txt
- Tests/LibWeb/Accessibility/expected/heading-and-emoji.txt
- Tests/LibWeb/Accessibility/expected/inline-text.txt
- UI/AppKit/CMakeLists.txt
- Libraries/LibWeb/DOM/AccessibilityTreeNode.h
- Tests/LibWeb/Accessibility/input/aria-labels.html
- Tests/LibWeb/Accessibility/input/text-formatting.html
- Tests/LibWeb/Accessibility/expected/table-single-row.txt
- Tests/LibWeb/Accessibility/input/table-headers.html
- Tests/LibWeb/Accessibility/input/links-and-buttons.html
- Tests/LibWeb/Accessibility/input/nested-lists.html
- Tests/LibWeb/Accessibility/expected/lists.txt
- Tests/LibWeb/Accessibility/input/checkbox-states.html
- Tests/LibWeb/Accessibility/input/table-no-header.html
- Tests/LibWeb/Accessibility/input/named-generic.html
- Tests/LibWeb/AccessibilityBridgeMac/run_tests.sh
- Tests/LibWeb/Accessibility/expected/complex-form.txt
- Libraries/LibWeb/DOM/Element.h
- Tests/LibWeb/Accessibility/input/form-controls.html
- Tests/LibWeb/Accessibility/expected/tables.txt
- Tests/LibWeb/Accessibility/expected/landmarks.txt
- Tests/LibWeb/Accessibility/expected/input-states.txt
- Tests/LibWeb/Accessibility/input/role-presentation.html
- .github/actions/setup/action.yml
- Tests/LibWeb/Accessibility/input/landmarks.html
- Libraries/LibWeb/Page/Page.h
- Libraries/LibWeb/HTML/HTMLTableElement.h
- Tests/LibWeb/Accessibility/expected/dialog.txt
- Tests/LibWeb/Accessibility/input/aria-multiselectable.html
- Tests/LibWeb/Accessibility/input/aria-live-assertive.html
- UI/Qt/ladybird.qrc
- Tests/LibWeb/Accessibility/expected/aria-description.txt
- Tests/LibWeb/Accessibility/input/image-alt.html
- Services/WebContent/WebContentServer.ipc
- Tests/LibWeb/Accessibility/input/skip-nav.html
- Tests/LibWeb/Accessibility/input/mixed-content.html
- Tests/LibWeb/Accessibility/expected/nested-lists.txt
- Tests/LibWeb/Accessibility/expected/aria-states.txt
- UI/AppKit/Interface/LadybirdAccessibilityViewProtocol.h
- Tests/LibWeb/Accessibility/expected/details-expanded.txt
- Tests/LibWeb/Accessibility/input/aria-hidden-inheritance.html
- Tests/LibWeb/Accessibility/expected/aria-described.txt
- Tests/LibWeb/Accessibility/input/radio-and-select.html
- pyproject.toml
- Services/WebContent/WebContentClient.ipc
- Tests/LibWeb/Accessibility/input/heading-and-emoji.html
- Tests/LibWeb/Accessibility/expected/link-url.txt
- Tests/LibWeb/AccessibilityBridge/input/roles.html
- Libraries/LibWebView/PageInfo.h
- Tests/LibWeb/Accessibility/expected/table-complex.txt
- Tests/LibWeb/Accessibility/expected/heading-levels.txt
- Tests/LibWeb/Accessibility/expected/nested-structure.txt
- Tests/LibWeb/Accessibility/input/input-states.html
- Tests/LibWeb/Accessibility/input/aria-pressed.html
- Tests/LibWeb/Accessibility/expected/mixed-content.txt
- Tests/LibWeb/Accessibility/input/generic-transparency.html
- Tests/LibWeb/Accessibility/input/empty-and-hidden.html
- Libraries/LibWebView/Forward.h
- Tests/LibWeb/AccessibilityBridge/input/tree_shape.html
- Tests/LibWeb/Accessibility/expected/separator-and-roles.txt
- Tests/LibWeb/Accessibility/input/document-language.html
- Tests/LibWeb/Accessibility/input/interactive-elements.html
- Tests/LibWeb/Accessibility/expected/form-validation.txt
- Tests/LibWeb/Accessibility/expected/aria-live-regions.txt
- Libraries/LibWeb/CSS/SelectorMatching.cpp
- Tests/LibWeb/Accessibility/input/aria-states.html
- Tests/LibWeb/Accessibility/input/aria-described.html
- Tests/LibWeb/Accessibility/input/lists.html
- Tests/LibWeb/AccessibilityBridge/tests_orca/test_collection_fallback.py
- Libraries/LibWebView/ViewImplementation.h
- Tests/LibWeb/Accessibility/expected/image-alt.txt
- Tests/LibWeb/Accessibility/input/table-single-row.html
- Tests/LibWeb/test-web/TestWeb.h
- Tests/LibWeb/Accessibility/expected/select-optgroup.txt
- Tests/LibWeb/Accessibility/input/hidden-content.html
- Tests/LibWeb/Accessibility/expected/checkbox-states.txt
- Libraries/LibWebView/AccessibilityNodeData.h
- Tests/LibWeb/Accessibility/input/headings.html
- Tests/LibWeb/AccessibilityBridge/tests/init.py
- Tests/LibWeb/Accessibility/input/aria-current.html
- Tests/LibWeb/Accessibility/expected/text-formatting.txt
- Tests/LibWeb/Accessibility/input/dialog.html
- Tests/LibWeb/AccessibilityBridge/input/tables.html
- .github/workflows/ci.yml
- Tests/LibWeb/Accessibility/input/semantic-sections.html
- Libraries/LibWeb/DOM/Node.cpp
- Tests/LibWeb/Accessibility/expected/hidden-content.txt
- UI/Qt/WebContentViewAccessibility.h
- Tests/LibWeb/AccessibilityBridge/tests/test_character_rect.py
- Tests/LibWeb/Accessibility/expected/accesskey.txt
- UI/AppKit/Interface/LadybirdWebView.h
- Tests/LibWeb/Accessibility/input/form-validation.html
- Tests/LibWeb/AccessibilityBridge/tests/test_lifecycle.py
- Tests/LibWeb/Accessibility/expected/named-generic.txt
- Tests/LibWeb/Accessibility/expected/aria-roles.txt
- Tests/LibWeb/Accessibility/input/deeply-nested-landmarks.html
- Tests/LibWeb/CMakeLists.txt
- UI/Qt/WebContentView.h
- Tests/LibWeb/AccessibilityBridge/tests/test_tables.py
- UI/Qt/CMakeLists.txt
- Tests/LibWeb/AccessibilityBridge/CMakeLists.txt
- Tests/LibWeb/Accessibility/expected/semantic-sections.txt
- Libraries/LibWebView/WebContentClient.cpp
- Libraries/LibWeb/DOM/Document.h
- Tests/LibWeb/Accessibility/expected/table-spans.txt
- Tests/LibWeb/Accessibility/expected/generic-transparency.txt
- Tests/LibWeb/AccessibilityBridgeMac/harness/testcase.py
- Services/WebContent/PageClient.h
- Tests/LibWeb/AccessibilityBridge/input/states.html
- Tests/LibWeb/Accessibility/input/images.html
- Tests/LibWeb/Accessibility/expected/role-presentation.txt
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_lifecycle.py
- Libraries/LibWebView/AccessibilityNodeData.cpp
- Services/WebContent/ConnectionFromClient.h
- Tests/LibWeb/Accessibility/expected/aria-live-assertive.txt
- Tests/LibWeb/AccessibilityBridge/tests_orca/test_active_document.py
- UI/Qt/Application.cpp
- Tests/LibWeb/AccessibilityBridge/tests/test_line_boundary.py
- Libraries/LibWebView/AccessibilityTreeManager.h
- Tests/LibWeb/Accessibility/input/values-range.html
- Libraries/LibWebView/ViewImplementation.cpp
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_search_predicate.py
- Services/WebContent/PageClient.cpp
- Tests/LibWeb/Accessibility/expected/values-range.txt
- Tests/LibWeb/AccessibilityBridge/tests_orca/test_structural_navigation.py
- UI/Qt/AccessibilityInterface.h
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_roles.py
- UI/Qt/Tab.cpp
- Libraries/LibWebView/WebContentClient.h
- UI/Qt/OrcaScripts/Ladybird/script.py
- Tests/LibWeb/AccessibilityBridge/tests/test_roles.py
- Tests/LibWeb/AccessibilityBridge/harness/testcase.py
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_text_markers.py
- Libraries/LibWebView/AccessibilityTreeManager.cpp
- Tests/LibWeb/AccessibilityBridgeMac/CMakeLists.txt
- Libraries/LibWeb/DOM/Element.cpp
- UI/Qt/WebContentViewAccessibility.mm
- Services/WebContent/ConnectionFromClient.cpp
- Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp
- UI/AppKit/Interface/LadybirdAccessibilityElement.mm
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_regressions.py
- Tests/LibWeb/AccessibilityBridge/tests/test_regressions.py
- UI/AppKit/Interface/LadybirdWebView.mm
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
.github/workflows/lagom-template.yml (1)
344-355: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid running the accessibility bridge suite twice.
The unrestricted Release
ctestat Lines 337-340 already includes registeredLibWebAccessibilityBridgetests. Remove this step, or exclude the suite from the general invocation if dedicated reporting is required.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/lagom-template.yml around lines 344 - 355, Remove the dedicated “Test - LibWeb AT-SPI2 bridge (Release)” step, since the unrestricted Release ctest invocation already runs the registered LibWebAccessibilityBridge tests. Preserve the existing general Release test flow and avoid adding duplicate execution.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Tests/LibWeb/AccessibilityBridge/harness/ladybird.py`:
- Around line 66-68: Update __enter__ in both
Tests/LibWeb/AccessibilityBridge/harness/ladybird.py (lines 66-68) and
Tests/LibWeb/AccessibilityBridgeMac/harness/ladybird.py (lines 85-87) to wrap
start() in a try/except BaseException, call stop() when entry fails, and
re-raise the original exception.
In `@Tests/LibWeb/AccessibilityBridge/README.md`:
- Line 47: Update the README description of run_tests.sh to identify the private
X display, fresh D-Bus session bus, and AT-SPI2 bus as isolated-mode behavior
only. Document that the default mode uses the host display and AT-SPI2 session,
while CI or the --isolated option enables the private environment.
In `@Tests/LibWeb/AccessibilityBridge/run_tests.sh`:
- Around line 114-127: Replace the fixed /tmp/xvfb.log path in the Xvfb startup
and error reporting with a uniquely created temporary log file using mktemp.
Ensure the generated log file is removed by the existing EXIT cleanup alongside
XVFB_PID, while preserving the failure message’s log path and Xvfb redirection
behavior.
In `@Tests/LibWeb/AccessibilityBridge/tests_orca/test_say_all.py`:
- Around line 54-57: Update the assertions in the sentence-content test around
the contents extraction to validate the complete spoken sequence, including the
link text. Require both link objects/names and the surrounding prose segments in
exact DOM order, rather than only checking that the prose fragments appear
somewhere in the joined string.
In `@Tests/LibWeb/AccessibilityBridge/tests/test_attributes.py`:
- Around line 31-40: Remove the broad exception suppression in both collectors:
update visit in Tests/LibWeb/AccessibilityBridge/tests/test_attributes.py lines
31-40 so role/attribute query failures propagate and fail the exhaustive tag
test, and update the XML-role collector at lines 90-97 so AT-SPI exceptions are
not silently omitted from the collected map.
In `@Tests/LibWeb/AccessibilityBridge/tests/test_interfaces.py`:
- Around line 71-91: Update test_named_section_advertises_text so it always
performs a meaningful assertion: either create/use a named section fixture with
no children and assert it supports the text interface, or assert the expected
interface behavior for the existing child-containing named section. Remove the
current conditional skip that allows the test to pass without checking anything.
In `@Tests/LibWeb/AccessibilityBridge/tests/test_states.py`:
- Around line 123-126: Update the is_disabled assertion in the disabled-button
state test so it passes only when both Atspi.StateType.SENSITIVE and
Atspi.StateType.ENABLED are absent; replace the current OR-based condition with
an equivalent requirement that validates both flags are missing.
- Around line 94-97: Update the heading assertion in the test method containing
find_all_by_role so it explicitly requires the fixture to return the expected h1
before iterating over hs. Remove the permissive empty-result behavior while
preserving the existing checks for the discovered heading’s state.
In `@Tests/LibWeb/AccessibilityBridgeMac/tests/test_tree_shape.py`:
- Around line 164-188: Update
test_no_ignored_role_node_is_an_accessibility_element to read AXSubrole and
exclude legitimate landmark groups from offenders. After that filtering, require
offenders to be empty rather than allowing an arbitrary threshold, while
preserving the existing child-role detection.
---
Nitpick comments:
In @.github/workflows/lagom-template.yml:
- Around line 344-355: Remove the dedicated “Test - LibWeb AT-SPI2 bridge
(Release)” step, since the unrestricted Release ctest invocation already runs
the registered LibWebAccessibilityBridge tests. Preserve the existing general
Release test flow and avoid adding duplicate execution.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7d3dac11-2d1a-4c67-9b62-2158288e10d1
📒 Files selected for processing (58)
.github/actions/setup/action.yml.github/workflows/ci.yml.github/workflows/lagom-template.ymlCMakeLists.txtDocumentation/ScreenReaderSupport.mdTests/LibWeb/AccessibilityBridge/.gitignoreTests/LibWeb/AccessibilityBridge/CMakeLists.txtTests/LibWeb/AccessibilityBridge/README.mdTests/LibWeb/AccessibilityBridge/harness/__init__.pyTests/LibWeb/AccessibilityBridge/harness/ladybird.pyTests/LibWeb/AccessibilityBridge/harness/orca.pyTests/LibWeb/AccessibilityBridge/harness/testcase.pyTests/LibWeb/AccessibilityBridge/harness/tree.pyTests/LibWeb/AccessibilityBridge/input/abbreviations.htmlTests/LibWeb/AccessibilityBridge/input/listitems.htmlTests/LibWeb/AccessibilityBridge/input/paragraphs.htmlTests/LibWeb/AccessibilityBridge/input/roles.htmlTests/LibWeb/AccessibilityBridge/input/states.htmlTests/LibWeb/AccessibilityBridge/input/tables.htmlTests/LibWeb/AccessibilityBridge/input/tree_shape.htmlTests/LibWeb/AccessibilityBridge/run_tests.shTests/LibWeb/AccessibilityBridge/tests/__init__.pyTests/LibWeb/AccessibilityBridge/tests/test_attributes.pyTests/LibWeb/AccessibilityBridge/tests/test_character_rect.pyTests/LibWeb/AccessibilityBridge/tests/test_hypertext_fallback.pyTests/LibWeb/AccessibilityBridge/tests/test_interfaces.pyTests/LibWeb/AccessibilityBridge/tests/test_lifecycle.pyTests/LibWeb/AccessibilityBridge/tests/test_line_boundary.pyTests/LibWeb/AccessibilityBridge/tests/test_regressions.pyTests/LibWeb/AccessibilityBridge/tests/test_roles.pyTests/LibWeb/AccessibilityBridge/tests/test_states.pyTests/LibWeb/AccessibilityBridge/tests/test_tables.pyTests/LibWeb/AccessibilityBridge/tests/test_text_content.pyTests/LibWeb/AccessibilityBridge/tests/test_tree_shape.pyTests/LibWeb/AccessibilityBridge/tests_orca/__init__.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_active_document.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_collection_fallback.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_focus_changed.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_hypertext_fallback.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_say_all.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_structural_navigation.pyTests/LibWeb/AccessibilityBridgeMac/CMakeLists.txtTests/LibWeb/AccessibilityBridgeMac/README.mdTests/LibWeb/AccessibilityBridgeMac/harness/__init__.pyTests/LibWeb/AccessibilityBridgeMac/harness/ladybird.pyTests/LibWeb/AccessibilityBridgeMac/harness/testcase.pyTests/LibWeb/AccessibilityBridgeMac/harness/tree.pyTests/LibWeb/AccessibilityBridgeMac/run_tests.shTests/LibWeb/AccessibilityBridgeMac/tests/__init__.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_lifecycle.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_regressions.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_roles.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_search_predicate.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_states.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_text_markers.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_tree_shape.pyTests/LibWeb/CMakeLists.txtpyproject.toml
🚧 Files skipped from review as they are similar to previous changes (35)
- Tests/LibWeb/AccessibilityBridge/tests_orca/init.py
- Tests/LibWeb/AccessibilityBridge/input/paragraphs.html
- Tests/LibWeb/AccessibilityBridgeMac/tests/init.py
- Tests/LibWeb/AccessibilityBridge/input/listitems.html
- Tests/LibWeb/AccessibilityBridge/input/tables.html
- Tests/LibWeb/AccessibilityBridge/tests/init.py
- Tests/LibWeb/CMakeLists.txt
- Tests/LibWeb/AccessibilityBridge/input/abbreviations.html
- Tests/LibWeb/AccessibilityBridgeMac/CMakeLists.txt
- Tests/LibWeb/AccessibilityBridge/.gitignore
- Tests/LibWeb/AccessibilityBridge/input/roles.html
- pyproject.toml
- Tests/LibWeb/AccessibilityBridge/tests_orca/test_structural_navigation.py
- Tests/LibWeb/AccessibilityBridgeMac/harness/testcase.py
- Tests/LibWeb/AccessibilityBridge/tests/test_character_rect.py
- Tests/LibWeb/AccessibilityBridge/harness/testcase.py
- Tests/LibWeb/AccessibilityBridgeMac/run_tests.sh
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_lifecycle.py
- .github/workflows/ci.yml
- Tests/LibWeb/AccessibilityBridge/tests/test_lifecycle.py
- CMakeLists.txt
- Tests/LibWeb/AccessibilityBridge/input/states.html
- Tests/LibWeb/AccessibilityBridge/CMakeLists.txt
- Tests/LibWeb/AccessibilityBridge/tests/test_tables.py
- Tests/LibWeb/AccessibilityBridge/tests/test_line_boundary.py
- .github/actions/setup/action.yml
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_regressions.py
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_text_markers.py
- Tests/LibWeb/AccessibilityBridge/tests_orca/test_collection_fallback.py
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_search_predicate.py
- Tests/LibWeb/AccessibilityBridge/tests/test_roles.py
- Tests/LibWeb/AccessibilityBridge/tests_orca/test_active_document.py
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_roles.py
- Tests/LibWeb/AccessibilityBridge/tests/test_regressions.py
- Tests/LibWeb/AccessibilityBridge/input/tree_shape.html
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/lagom-template.yml:
- Around line 219-228: The Accessibility grant currently targets shared system
Python interpreters, exposing AX access to unrelated repository-controlled
tests. Update this grant block to use a dedicated helper executable created for
this purpose, and grant access only to that helper while preserving the existing
non-self-hosted macOS scope and tccd refresh behavior.
In `@Services/WebContent/PageClient.cpp`:
- Around line 1348-1349: Update the DOM-mutation handling around
send_dom_mutation() so schedule_accessibility_tree_update() is triggered
independently of listen_for_dom_mutations() and DevTools mutation forwarding.
Preserve accessibility refreshes for pages with m_accessibility_tree_requested
even when mutation listening is disabled.
In `@Tests/LibWeb/AccessibilityBridge/harness/tree.py`:
- Around line 195-204: Update get_attributes_dict to stop catching and
suppressing exceptions from Atspi.Accessible.get_attributes; let query failures
propagate so tests fail when AT-SPI is unreachable. Preserve the existing
empty-dictionary result only when the call succeeds and returns None, and apply
the same behavior to the related helper at the referenced range.
In `@Tests/LibWeb/AccessibilityBridge/tests/test_line_boundary.py`:
- Around line 63-67: Update the test loop around Atspi.Text.get_string_at_offset
to iterate only valid character offsets, excluding the length boundary sentinel.
Remove the start_offset == -1 continue and assert that every queried valid
offset returns a valid range before checking the full "Banana" content.
In `@Tests/LibWeb/AccessibilityBridge/tests/test_states.py`:
- Around line 73-77: Update the fixture-node traversal in the test so it no
longer catches and ignores every exception around obj.get_name(). Remove the
broad exception suppression and allow unexpected AT-SPI/bridge traversal
failures to propagate and fail the test, while preserving the existing Editable
div matching behavior.
In `@Tests/LibWeb/AccessibilityBridgeMac/harness/__init__.py`:
- Around line 20-37: Reorder the symbols in the `__all__` list of the harness
module according to Ruff’s RUF022 convention, using Ruff’s autofix and
preserving every existing export.
In `@Tests/LibWeb/AccessibilityBridgeMac/harness/tree.py`:
- Around line 65-72: Update the exception handling in the _visit helper so it no
longer catches exceptions raised by the caller-supplied pred; keep _ax_attr’s
internal AX read-error handling unchanged, but limit any outer suppression to
the AX role lookup if needed, allowing predicate failures to propagate instead
of returning an empty result.
In `@Tests/LibWeb/AccessibilityBridgeMac/README.md`:
- Around line 31-33: Label both shell-command fenced code blocks in the
AccessibilityBridgeMac README with the console language identifier, including
the ctest command and run_tests.sh command, without changing their contents.
- Line 52: Align the README timeout statement with the 60-second default used by
LADYBIRD_AX_STARTUP_TIMEOUT in harness/ladybird.py, updating the documented
15-second budget to 60 seconds while preserving the guidance against extending
timeouts to mask flakiness.
In `@Tests/LibWeb/AccessibilityBridgeMac/tests/test_states.py`:
- Around line 130-137: Update test_disabled_button_still_advertises_press_action
to poll or wait for the disabled button to appear before inspecting its actions,
matching the synchronization approach used by neighboring tests. Keep the
existing AXPress assertion unchanged once _find_button_by_title returns the
populated accessibility node.
In `@UI/Qt/AccessibilityInterface.cpp`:
- Around line 225-233: Update the QAccessible::ActionInterface handling and its
corresponding action advertisement around setFocus so every element that exposes
setFocus returns an ActionInterface, including headings, paragraphs, and list
items. Extend pressAction() support to toggle roles such as switch and
menuitemcheckbox while preserving existing press-capable roles.
In `@UI/Qt/OrcaScripts/orca_customizations_bootstrap.py`:
- Around line 28-36: Update the exception handler in the custom script-loading
flow around _ladybird_importlib.import_module, get_script, and Script to log the
caught exception before preserving the fallback to
_ladybird_original_new_named_script. Use the module/script name in the
diagnostic and keep the existing fallback behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 25eadb7c-9a7a-4a7b-b097-2dd8ddeebd0d
📒 Files selected for processing (231)
.github/actions/setup/action.yml.github/workflows/ci.yml.github/workflows/lagom-template.ymlCMakeLists.txtDocumentation/ScreenReaderSupport.mdLibraries/LibWeb/CSS/SelectorMatching.cppLibraries/LibWeb/DOM/AccessibilityTreeNode.cppLibraries/LibWeb/DOM/AccessibilityTreeNode.hLibraries/LibWeb/DOM/Document.cppLibraries/LibWeb/DOM/Document.hLibraries/LibWeb/DOM/Element.cppLibraries/LibWeb/DOM/Element.hLibraries/LibWeb/DOM/Node.cppLibraries/LibWeb/HTML/HTMLTableElement.hLibraries/LibWeb/Page/Page.hLibraries/LibWebView/AccessibilityNodeData.cppLibraries/LibWebView/AccessibilityNodeData.hLibraries/LibWebView/AccessibilityTreeManager.cppLibraries/LibWebView/AccessibilityTreeManager.hLibraries/LibWebView/CMakeLists.txtLibraries/LibWebView/Forward.hLibraries/LibWebView/PageInfo.hLibraries/LibWebView/ViewImplementation.cppLibraries/LibWebView/ViewImplementation.hLibraries/LibWebView/WebContentClient.cppLibraries/LibWebView/WebContentClient.hServices/WebContent/ConnectionFromClient.cppServices/WebContent/ConnectionFromClient.hServices/WebContent/PageClient.cppServices/WebContent/PageClient.hServices/WebContent/WebContentClient.ipcServices/WebContent/WebContentServer.ipcTests/LibWeb/Accessibility/expected/accesskey.txtTests/LibWeb/Accessibility/expected/aria-checked-override.txtTests/LibWeb/Accessibility/expected/aria-current.txtTests/LibWeb/Accessibility/expected/aria-described.txtTests/LibWeb/Accessibility/expected/aria-description.txtTests/LibWeb/Accessibility/expected/aria-hidden-inheritance.txtTests/LibWeb/Accessibility/expected/aria-labels.txtTests/LibWeb/Accessibility/expected/aria-live-assertive.txtTests/LibWeb/Accessibility/expected/aria-live-regions.txtTests/LibWeb/Accessibility/expected/aria-multiselectable.txtTests/LibWeb/Accessibility/expected/aria-pressed.txtTests/LibWeb/Accessibility/expected/aria-roles.txtTests/LibWeb/Accessibility/expected/aria-states.txtTests/LibWeb/Accessibility/expected/basic-structure.txtTests/LibWeb/Accessibility/expected/checkbox-states.txtTests/LibWeb/Accessibility/expected/complex-form.txtTests/LibWeb/Accessibility/expected/deeply-nested-landmarks.txtTests/LibWeb/Accessibility/expected/details-expanded.txtTests/LibWeb/Accessibility/expected/dialog.txtTests/LibWeb/Accessibility/expected/display-contents.txtTests/LibWeb/Accessibility/expected/document-language.txtTests/LibWeb/Accessibility/expected/empty-and-hidden.txtTests/LibWeb/Accessibility/expected/form-controls.txtTests/LibWeb/Accessibility/expected/form-validation.txtTests/LibWeb/Accessibility/expected/generic-transparency.txtTests/LibWeb/Accessibility/expected/heading-and-emoji.txtTests/LibWeb/Accessibility/expected/heading-levels.txtTests/LibWeb/Accessibility/expected/headings.txtTests/LibWeb/Accessibility/expected/hidden-content.txtTests/LibWeb/Accessibility/expected/image-alt.txtTests/LibWeb/Accessibility/expected/images.txtTests/LibWeb/Accessibility/expected/inline-text.txtTests/LibWeb/Accessibility/expected/input-states.txtTests/LibWeb/Accessibility/expected/interactive-elements.txtTests/LibWeb/Accessibility/expected/landmarks.txtTests/LibWeb/Accessibility/expected/link-url.txtTests/LibWeb/Accessibility/expected/links-and-buttons.txtTests/LibWeb/Accessibility/expected/lists.txtTests/LibWeb/Accessibility/expected/menu-and-menubar.txtTests/LibWeb/Accessibility/expected/mixed-content.txtTests/LibWeb/Accessibility/expected/multiple-tables.txtTests/LibWeb/Accessibility/expected/named-generic.txtTests/LibWeb/Accessibility/expected/nested-lists.txtTests/LibWeb/Accessibility/expected/nested-structure.txtTests/LibWeb/Accessibility/expected/option-aria-label.txtTests/LibWeb/Accessibility/expected/radio-and-select.txtTests/LibWeb/Accessibility/expected/role-presentation-conflict.txtTests/LibWeb/Accessibility/expected/role-presentation.txtTests/LibWeb/Accessibility/expected/select-hidden-options.txtTests/LibWeb/Accessibility/expected/select-optgroup.txtTests/LibWeb/Accessibility/expected/semantic-sections.txtTests/LibWeb/Accessibility/expected/separator-and-roles.txtTests/LibWeb/Accessibility/expected/skip-nav.txtTests/LibWeb/Accessibility/expected/table-complex.txtTests/LibWeb/Accessibility/expected/table-headers.txtTests/LibWeb/Accessibility/expected/table-no-header.txtTests/LibWeb/Accessibility/expected/table-single-row.txtTests/LibWeb/Accessibility/expected/table-spans.txtTests/LibWeb/Accessibility/expected/tables.txtTests/LibWeb/Accessibility/expected/text-formatting.txtTests/LibWeb/Accessibility/expected/text-values.txtTests/LibWeb/Accessibility/expected/values-range.txtTests/LibWeb/Accessibility/input/accesskey.htmlTests/LibWeb/Accessibility/input/aria-checked-override.htmlTests/LibWeb/Accessibility/input/aria-current.htmlTests/LibWeb/Accessibility/input/aria-described.htmlTests/LibWeb/Accessibility/input/aria-description.htmlTests/LibWeb/Accessibility/input/aria-hidden-inheritance.htmlTests/LibWeb/Accessibility/input/aria-labels.htmlTests/LibWeb/Accessibility/input/aria-live-assertive.htmlTests/LibWeb/Accessibility/input/aria-live-regions.htmlTests/LibWeb/Accessibility/input/aria-multiselectable.htmlTests/LibWeb/Accessibility/input/aria-pressed.htmlTests/LibWeb/Accessibility/input/aria-roles.htmlTests/LibWeb/Accessibility/input/aria-states.htmlTests/LibWeb/Accessibility/input/basic-structure.htmlTests/LibWeb/Accessibility/input/checkbox-states.htmlTests/LibWeb/Accessibility/input/complex-form.htmlTests/LibWeb/Accessibility/input/deeply-nested-landmarks.htmlTests/LibWeb/Accessibility/input/details-expanded.htmlTests/LibWeb/Accessibility/input/dialog.htmlTests/LibWeb/Accessibility/input/display-contents.htmlTests/LibWeb/Accessibility/input/document-language.htmlTests/LibWeb/Accessibility/input/empty-and-hidden.htmlTests/LibWeb/Accessibility/input/form-controls.htmlTests/LibWeb/Accessibility/input/form-validation.htmlTests/LibWeb/Accessibility/input/generic-transparency.htmlTests/LibWeb/Accessibility/input/heading-and-emoji.htmlTests/LibWeb/Accessibility/input/heading-levels.htmlTests/LibWeb/Accessibility/input/headings.htmlTests/LibWeb/Accessibility/input/hidden-content.htmlTests/LibWeb/Accessibility/input/image-alt.htmlTests/LibWeb/Accessibility/input/images.htmlTests/LibWeb/Accessibility/input/inline-text.htmlTests/LibWeb/Accessibility/input/input-states.htmlTests/LibWeb/Accessibility/input/interactive-elements.htmlTests/LibWeb/Accessibility/input/landmarks.htmlTests/LibWeb/Accessibility/input/link-url.htmlTests/LibWeb/Accessibility/input/links-and-buttons.htmlTests/LibWeb/Accessibility/input/lists.htmlTests/LibWeb/Accessibility/input/menu-and-menubar.htmlTests/LibWeb/Accessibility/input/mixed-content.htmlTests/LibWeb/Accessibility/input/multiple-tables.htmlTests/LibWeb/Accessibility/input/named-generic.htmlTests/LibWeb/Accessibility/input/nested-lists.htmlTests/LibWeb/Accessibility/input/nested-structure.htmlTests/LibWeb/Accessibility/input/option-aria-label.htmlTests/LibWeb/Accessibility/input/radio-and-select.htmlTests/LibWeb/Accessibility/input/role-presentation-conflict.htmlTests/LibWeb/Accessibility/input/role-presentation.htmlTests/LibWeb/Accessibility/input/select-hidden-options.htmlTests/LibWeb/Accessibility/input/select-optgroup.htmlTests/LibWeb/Accessibility/input/semantic-sections.htmlTests/LibWeb/Accessibility/input/separator-and-roles.htmlTests/LibWeb/Accessibility/input/skip-nav.htmlTests/LibWeb/Accessibility/input/table-complex.htmlTests/LibWeb/Accessibility/input/table-headers.htmlTests/LibWeb/Accessibility/input/table-no-header.htmlTests/LibWeb/Accessibility/input/table-single-row.htmlTests/LibWeb/Accessibility/input/table-spans.htmlTests/LibWeb/Accessibility/input/tables.htmlTests/LibWeb/Accessibility/input/text-formatting.htmlTests/LibWeb/Accessibility/input/text-values.htmlTests/LibWeb/Accessibility/input/values-range.htmlTests/LibWeb/AccessibilityBridge/.gitignoreTests/LibWeb/AccessibilityBridge/CMakeLists.txtTests/LibWeb/AccessibilityBridge/README.mdTests/LibWeb/AccessibilityBridge/harness/__init__.pyTests/LibWeb/AccessibilityBridge/harness/ladybird.pyTests/LibWeb/AccessibilityBridge/harness/orca.pyTests/LibWeb/AccessibilityBridge/harness/testcase.pyTests/LibWeb/AccessibilityBridge/harness/tree.pyTests/LibWeb/AccessibilityBridge/input/abbreviations.htmlTests/LibWeb/AccessibilityBridge/input/listitems.htmlTests/LibWeb/AccessibilityBridge/input/paragraphs.htmlTests/LibWeb/AccessibilityBridge/input/roles.htmlTests/LibWeb/AccessibilityBridge/input/states.htmlTests/LibWeb/AccessibilityBridge/input/tables.htmlTests/LibWeb/AccessibilityBridge/input/tree_shape.htmlTests/LibWeb/AccessibilityBridge/run_tests.shTests/LibWeb/AccessibilityBridge/tests/__init__.pyTests/LibWeb/AccessibilityBridge/tests/test_attributes.pyTests/LibWeb/AccessibilityBridge/tests/test_character_rect.pyTests/LibWeb/AccessibilityBridge/tests/test_hypertext_fallback.pyTests/LibWeb/AccessibilityBridge/tests/test_interfaces.pyTests/LibWeb/AccessibilityBridge/tests/test_lifecycle.pyTests/LibWeb/AccessibilityBridge/tests/test_line_boundary.pyTests/LibWeb/AccessibilityBridge/tests/test_regressions.pyTests/LibWeb/AccessibilityBridge/tests/test_roles.pyTests/LibWeb/AccessibilityBridge/tests/test_states.pyTests/LibWeb/AccessibilityBridge/tests/test_tables.pyTests/LibWeb/AccessibilityBridge/tests/test_text_content.pyTests/LibWeb/AccessibilityBridge/tests/test_tree_shape.pyTests/LibWeb/AccessibilityBridge/tests_orca/__init__.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_active_document.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_collection_fallback.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_focus_changed.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_hypertext_fallback.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_say_all.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_structural_navigation.pyTests/LibWeb/AccessibilityBridgeMac/CMakeLists.txtTests/LibWeb/AccessibilityBridgeMac/README.mdTests/LibWeb/AccessibilityBridgeMac/harness/__init__.pyTests/LibWeb/AccessibilityBridgeMac/harness/ladybird.pyTests/LibWeb/AccessibilityBridgeMac/harness/testcase.pyTests/LibWeb/AccessibilityBridgeMac/harness/tree.pyTests/LibWeb/AccessibilityBridgeMac/run_tests.shTests/LibWeb/AccessibilityBridgeMac/tests/__init__.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_lifecycle.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_regressions.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_roles.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_search_predicate.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_states.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_text_markers.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_tree_shape.pyTests/LibWeb/CMakeLists.txtTests/LibWeb/test-web/TestWeb.hTests/LibWeb/test-web/main.cppUI/AppKit/CMakeLists.txtUI/AppKit/Interface/LadybirdAccessibilityElement.hUI/AppKit/Interface/LadybirdAccessibilityElement.mmUI/AppKit/Interface/LadybirdAccessibilityViewProtocol.hUI/AppKit/Interface/LadybirdWebView.hUI/AppKit/Interface/LadybirdWebView.mmUI/Qt/AccessibilityInterface.cppUI/Qt/AccessibilityInterface.hUI/Qt/Application.cppUI/Qt/CMakeLists.txtUI/Qt/OrcaScripts/Ladybird/__init__.pyUI/Qt/OrcaScripts/Ladybird/script.pyUI/Qt/OrcaScripts/Ladybird/script_utilities.pyUI/Qt/OrcaScripts/orca_customizations_bootstrap.pyUI/Qt/Tab.cppUI/Qt/WebContentView.cppUI/Qt/WebContentView.hUI/Qt/WebContentViewAccessibility.hUI/Qt/WebContentViewAccessibility.mmUI/Qt/ladybird.qrcpyproject.toml
🚧 Files skipped from review as they are similar to previous changes (189)
- Libraries/LibWebView/CMakeLists.txt
- Tests/LibWeb/Accessibility/expected/aria-states.txt
- Tests/LibWeb/Accessibility/expected/aria-live-regions.txt
- Tests/LibWeb/Accessibility/expected/basic-structure.txt
- Tests/LibWeb/Accessibility/expected/lists.txt
- Tests/LibWeb/Accessibility/input/inline-text.html
- Tests/LibWeb/Accessibility/expected/select-optgroup.txt
- Tests/LibWeb/Accessibility/input/radio-and-select.html
- Tests/LibWeb/Accessibility/expected/aria-roles.txt
- Tests/LibWeb/Accessibility/input/select-optgroup.html
- Tests/LibWeb/Accessibility/expected/radio-and-select.txt
- Tests/LibWeb/Accessibility/expected/aria-live-assertive.txt
- Tests/LibWeb/Accessibility/input/display-contents.html
- Tests/LibWeb/Accessibility/input/empty-and-hidden.html
- Tests/LibWeb/Accessibility/expected/aria-labels.txt
- Tests/LibWeb/Accessibility/expected/heading-and-emoji.txt
- Tests/LibWeb/Accessibility/expected/skip-nav.txt
- Tests/LibWeb/Accessibility/expected/separator-and-roles.txt
- Tests/LibWeb/Accessibility/expected/nested-structure.txt
- Tests/LibWeb/Accessibility/input/complex-form.html
- Tests/LibWeb/AccessibilityBridge/tests/init.py
- Tests/LibWeb/Accessibility/input/aria-hidden-inheritance.html
- Tests/LibWeb/Accessibility/expected/aria-hidden-inheritance.txt
- Tests/LibWeb/Accessibility/expected/table-single-row.txt
- Tests/LibWeb/Accessibility/expected/landmarks.txt
- Tests/LibWeb/Accessibility/input/skip-nav.html
- Tests/LibWeb/Accessibility/expected/aria-checked-override.txt
- Tests/LibWeb/Accessibility/expected/tables.txt
- Tests/LibWeb/Accessibility/input/text-values.html
- Tests/LibWeb/Accessibility/expected/empty-and-hidden.txt
- Tests/LibWeb/Accessibility/expected/interactive-elements.txt
- Tests/LibWeb/Accessibility/expected/table-no-header.txt
- Tests/LibWeb/Accessibility/input/aria-states.html
- Tests/LibWeb/Accessibility/input/aria-multiselectable.html
- pyproject.toml
- Tests/LibWeb/Accessibility/input/generic-transparency.html
- Tests/LibWeb/Accessibility/expected/dialog.txt
- Tests/LibWeb/Accessibility/input/heading-levels.html
- Tests/LibWeb/Accessibility/expected/table-headers.txt
- Tests/LibWeb/Accessibility/input/document-language.html
- Tests/LibWeb/Accessibility/input/nested-lists.html
- Tests/LibWeb/Accessibility/expected/accesskey.txt
- UI/Qt/WebContentViewAccessibility.h
- Tests/LibWeb/Accessibility/input/aria-labels.html
- Tests/LibWeb/Accessibility/expected/document-language.txt
- Tests/LibWeb/Accessibility/input/image-alt.html
- Tests/LibWeb/Accessibility/input/aria-current.html
- Tests/LibWeb/Accessibility/input/images.html
- Tests/LibWeb/Accessibility/input/multiple-tables.html
- Tests/LibWeb/Accessibility/expected/semantic-sections.txt
- Tests/LibWeb/Accessibility/expected/image-alt.txt
- Tests/LibWeb/Accessibility/input/form-controls.html
- Tests/LibWeb/Accessibility/input/aria-described.html
- Tests/LibWeb/Accessibility/input/mixed-content.html
- Tests/LibWeb/Accessibility/expected/images.txt
- Tests/LibWeb/Accessibility/expected/link-url.txt
- Tests/LibWeb/Accessibility/expected/nested-lists.txt
- Tests/LibWeb/Accessibility/expected/headings.txt
- Tests/LibWeb/Accessibility/input/named-generic.html
- Tests/LibWeb/Accessibility/input/role-presentation.html
- UI/AppKit/CMakeLists.txt
- Tests/LibWeb/Accessibility/expected/aria-pressed.txt
- UI/AppKit/Interface/LadybirdAccessibilityElement.h
- Tests/LibWeb/Accessibility/expected/table-spans.txt
- Tests/LibWeb/Accessibility/expected/values-range.txt
- Tests/LibWeb/test-web/TestWeb.h
- UI/Qt/ladybird.qrc
- Tests/LibWeb/Accessibility/expected/complex-form.txt
- Tests/LibWeb/AccessibilityBridge/.gitignore
- Tests/LibWeb/Accessibility/input/deeply-nested-landmarks.html
- Tests/LibWeb/Accessibility/input/separator-and-roles.html
- Tests/LibWeb/AccessibilityBridge/input/listitems.html
- Tests/LibWeb/AccessibilityBridge/input/roles.html
- Tests/LibWeb/Accessibility/input/table-single-row.html
- Tests/LibWeb/Accessibility/input/lists.html
- Tests/LibWeb/CMakeLists.txt
- Tests/LibWeb/Accessibility/input/aria-live-assertive.html
- Tests/LibWeb/Accessibility/input/values-range.html
- Tests/LibWeb/Accessibility/input/dialog.html
- Tests/LibWeb/AccessibilityBridge/tests_orca/init.py
- Tests/LibWeb/Accessibility/expected/inline-text.txt
- Tests/LibWeb/Accessibility/expected/role-presentation-conflict.txt
- Tests/LibWeb/Accessibility/input/table-spans.html
- Tests/LibWeb/Accessibility/input/link-url.html
- Tests/LibWeb/Accessibility/expected/aria-description.txt
- Tests/LibWeb/Accessibility/input/aria-checked-override.html
- Libraries/LibWeb/HTML/HTMLTableElement.h
- Tests/LibWeb/Accessibility/expected/checkbox-states.txt
- Tests/LibWeb/Accessibility/expected/table-complex.txt
- Services/WebContent/WebContentServer.ipc
- Tests/LibWeb/AccessibilityBridge/harness/testcase.py
- Tests/LibWeb/AccessibilityBridge/input/abbreviations.html
- Tests/LibWeb/AccessibilityBridgeMac/CMakeLists.txt
- Tests/LibWeb/Accessibility/expected/text-formatting.txt
- Tests/LibWeb/Accessibility/input/details-expanded.html
- Tests/LibWeb/Accessibility/expected/role-presentation.txt
- Tests/LibWeb/AccessibilityBridge/tests/test_lifecycle.py
- Tests/LibWeb/AccessibilityBridge/input/paragraphs.html
- Tests/LibWeb/Accessibility/input/aria-pressed.html
- Tests/LibWeb/Accessibility/expected/details-expanded.txt
- Tests/LibWeb/Accessibility/input/form-validation.html
- Libraries/LibWebView/Forward.h
- Tests/LibWeb/Accessibility/expected/form-controls.txt
- Tests/LibWeb/Accessibility/input/heading-and-emoji.html
- Tests/LibWeb/Accessibility/input/accesskey.html
- Tests/LibWeb/Accessibility/input/table-headers.html
- Tests/LibWeb/Accessibility/expected/aria-described.txt
- Tests/LibWeb/AccessibilityBridgeMac/tests/init.py
- Tests/LibWeb/Accessibility/expected/aria-multiselectable.txt
- Tests/LibWeb/Accessibility/input/landmarks.html
- Tests/LibWeb/Accessibility/expected/hidden-content.txt
- Tests/LibWeb/Accessibility/input/tables.html
- Tests/LibWeb/Accessibility/input/checkbox-states.html
- Tests/LibWeb/AccessibilityBridge/input/tree_shape.html
- Tests/LibWeb/Accessibility/input/links-and-buttons.html
- Tests/LibWeb/Accessibility/input/aria-description.html
- Tests/LibWeb/Accessibility/expected/menu-and-menubar.txt
- Libraries/LibWebView/ViewImplementation.h
- Tests/LibWeb/Accessibility/expected/select-hidden-options.txt
- Tests/LibWeb/Accessibility/input/select-hidden-options.html
- Tests/LibWeb/Accessibility/expected/links-and-buttons.txt
- Tests/LibWeb/Accessibility/expected/mixed-content.txt
- Tests/LibWeb/Accessibility/input/hidden-content.html
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_lifecycle.py
- Tests/LibWeb/AccessibilityBridge/input/states.html
- Tests/LibWeb/Accessibility/input/headings.html
- Tests/LibWeb/Accessibility/input/input-states.html
- Services/WebContent/PageClient.h
- Tests/LibWeb/Accessibility/expected/input-states.txt
- Tests/LibWeb/Accessibility/expected/heading-levels.txt
- Tests/LibWeb/AccessibilityBridge/tests/test_tables.py
- CMakeLists.txt
- Libraries/LibWebView/WebContentClient.h
- Libraries/LibWeb/DOM/AccessibilityTreeNode.h
- Tests/LibWeb/AccessibilityBridge/input/tables.html
- Tests/LibWeb/Accessibility/input/interactive-elements.html
- Tests/LibWeb/Accessibility/expected/aria-current.txt
- Tests/LibWeb/Accessibility/input/menu-and-menubar.html
- UI/Qt/OrcaScripts/Ladybird/init.py
- Libraries/LibWebView/AccessibilityNodeData.h
- Libraries/LibWeb/Page/Page.h
- Tests/LibWeb/Accessibility/input/semantic-sections.html
- Tests/LibWeb/Accessibility/input/role-presentation-conflict.html
- Tests/LibWeb/AccessibilityBridgeMac/run_tests.sh
- Tests/LibWeb/Accessibility/expected/display-contents.txt
- Tests/LibWeb/Accessibility/input/table-no-header.html
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_search_predicate.py
- Services/WebContent/WebContentClient.ipc
- Tests/LibWeb/AccessibilityBridge/CMakeLists.txt
- Libraries/LibWeb/CSS/SelectorMatching.cpp
- Libraries/LibWebView/AccessibilityTreeManager.h
- Libraries/LibWebView/AccessibilityNodeData.cpp
- Tests/LibWeb/Accessibility/expected/multiple-tables.txt
- UI/AppKit/Interface/LadybirdWebView.h
- Tests/LibWeb/Accessibility/input/text-formatting.html
- UI/Qt/CMakeLists.txt
- Tests/LibWeb/Accessibility/expected/generic-transparency.txt
- Tests/LibWeb/AccessibilityBridge/tests_orca/test_collection_fallback.py
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_text_markers.py
- Tests/LibWeb/test-web/main.cpp
- Tests/LibWeb/Accessibility/expected/form-validation.txt
- Tests/LibWeb/AccessibilityBridge/tests/test_character_rect.py
- Tests/LibWeb/Accessibility/expected/text-values.txt
- Libraries/LibWebView/WebContentClient.cpp
- Libraries/LibWebView/ViewImplementation.cpp
- Tests/LibWeb/Accessibility/input/aria-live-regions.html
- Tests/LibWeb/Accessibility/input/basic-structure.html
- UI/Qt/Tab.cpp
- .github/workflows/ci.yml
- UI/AppKit/Interface/LadybirdAccessibilityViewProtocol.h
- Tests/LibWeb/AccessibilityBridge/tests/test_regressions.py
- Tests/LibWeb/AccessibilityBridge/tests_orca/test_structural_navigation.py
- Libraries/LibWeb/DOM/Document.h
- Tests/LibWeb/AccessibilityBridge/tests/test_roles.py
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_roles.py
- Libraries/LibWebView/AccessibilityTreeManager.cpp
- Libraries/LibWeb/DOM/Node.cpp
- UI/Qt/OrcaScripts/Ladybird/script.py
- Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp
- UI/Qt/WebContentView.h
- UI/Qt/WebContentViewAccessibility.mm
- Tests/LibWeb/AccessibilityBridge/tests_orca/test_active_document.py
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_regressions.py
- Libraries/LibWeb/DOM/Element.cpp
- UI/Qt/AccessibilityInterface.h
- Services/WebContent/ConnectionFromClient.cpp
- UI/Qt/Application.cpp
- UI/AppKit/Interface/LadybirdWebView.mm
- UI/AppKit/Interface/LadybirdAccessibilityElement.mm
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Line 16: Update the workflow job name expression using matrix.name_suffix so
the property is defined for every matrix entry, or remove that interpolation if
no suffix is needed. Ensure actionlint validation passes and existing matrix
naming remains clear.
In @.github/workflows/lagom-template.yml:
- Around line 198-200: Update the PyObjC-unavailable branch in the AX preflight
to set fail=1 after emitting the existing warning. Ensure this prerequisite
failure propagates to the workflow’s preflight step, matching the failure
behavior of run_tests.sh.
In `@Tests/LibWeb/AccessibilityBridge/harness/orca.py`:
- Around line 132-134: Update OrcaSession.__enter__ to clean up process-global
Orca state when self.start() raises before context entry completes; ensure the
same teardown used by __exit__ runs on failure, then re-raise the original
exception, while preserving normal entry behavior when start() succeeds.
In `@Tests/LibWeb/AccessibilityBridge/tests_orca/test_collection_fallback.py`:
- Around line 40-50: Strengthen the assertions in
test_find_all_with_role_returns_links and the analogous list-item test to verify
the complete expected results from roles.html, including exact counts or
expected accessible names. Replace the permissive assertGreater checks so
missing links or list items cause the tests to fail.
In `@Tests/LibWeb/AccessibilityBridge/tests/test_states.py`:
- Around line 102-112: Update test_listitem_is_not_focusable and
test_list_is_not_focusable to assert that find_all_by_role returns at least one
matching element before iterating. Keep the existing non-FOCUSABLE assertions
for every discovered list item and list.
In `@Tests/LibWeb/AccessibilityBridgeMac/harness/ladybird.py`:
- Around line 114-163: Wrap the startup resource acquisition in the harness
initialization path, including creation of _stderr_file, subprocess.Popen,
AXUIElementCreateApplication, and the wait_for flow, in try/except. On any
exception, call stop() to terminate Ladybird and clean up the stderr file, then
re-raise the original exception; preserve the existing timeout diagnostics and
cleanup behavior.
In `@Tests/LibWeb/AccessibilityBridgeMac/tests/test_states.py`:
- Around line 66-70: Replace one-shot find_first_by_role searches with
wait_for_descendant_by_role in test_link_is_enabled, the other link test, and
the disabled-button action test. Ensure each test waits for its target before
reading accessibility attributes or performing actions, while preserving the
existing assertions and behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 410d0c23-ac1b-4c4e-915d-837c340b611a
📒 Files selected for processing (210)
.github/actions/setup/action.yml.github/workflows/ci.yml.github/workflows/lagom-template.ymlCMakeLists.txtDocumentation/ScreenReaderSupport.mdLibraries/LibWeb/CSS/SelectorMatching.cppLibraries/LibWeb/DOM/AccessibilityTreeNode.cppLibraries/LibWeb/DOM/AccessibilityTreeNode.hLibraries/LibWeb/DOM/Document.cppLibraries/LibWeb/DOM/Document.hLibraries/LibWeb/DOM/Element.cppLibraries/LibWeb/DOM/Element.hLibraries/LibWeb/DOM/Node.cppLibraries/LibWeb/HTML/HTMLTableElement.hLibraries/LibWeb/Page/Page.hLibraries/LibWebView/AccessibilityNodeData.cppLibraries/LibWebView/AccessibilityNodeData.hLibraries/LibWebView/AccessibilityTreeManager.cppLibraries/LibWebView/AccessibilityTreeManager.hLibraries/LibWebView/CMakeLists.txtLibraries/LibWebView/Forward.hLibraries/LibWebView/PageInfo.hLibraries/LibWebView/ViewImplementation.cppLibraries/LibWebView/ViewImplementation.hLibraries/LibWebView/WebContentClient.cppLibraries/LibWebView/WebContentClient.hServices/WebContent/ConnectionFromClient.cppServices/WebContent/ConnectionFromClient.hServices/WebContent/PageClient.cppServices/WebContent/PageClient.hServices/WebContent/WebContentClient.ipcServices/WebContent/WebContentServer.ipcTests/LibWeb/Accessibility/expected/accesskey.txtTests/LibWeb/Accessibility/expected/aria-checked-override.txtTests/LibWeb/Accessibility/expected/aria-current.txtTests/LibWeb/Accessibility/expected/aria-described.txtTests/LibWeb/Accessibility/expected/aria-description.txtTests/LibWeb/Accessibility/expected/aria-hidden-inheritance.txtTests/LibWeb/Accessibility/expected/aria-labels.txtTests/LibWeb/Accessibility/expected/aria-live-assertive.txtTests/LibWeb/Accessibility/expected/aria-live-regions.txtTests/LibWeb/Accessibility/expected/aria-multiselectable.txtTests/LibWeb/Accessibility/expected/aria-pressed.txtTests/LibWeb/Accessibility/expected/aria-roles.txtTests/LibWeb/Accessibility/expected/aria-states.txtTests/LibWeb/Accessibility/expected/basic-structure.txtTests/LibWeb/Accessibility/expected/checkbox-states.txtTests/LibWeb/Accessibility/expected/complex-form.txtTests/LibWeb/Accessibility/expected/deeply-nested-landmarks.txtTests/LibWeb/Accessibility/expected/details-expanded.txtTests/LibWeb/Accessibility/expected/dialog.txtTests/LibWeb/Accessibility/expected/display-contents.txtTests/LibWeb/Accessibility/expected/document-language.txtTests/LibWeb/Accessibility/expected/empty-and-hidden.txtTests/LibWeb/Accessibility/expected/form-controls.txtTests/LibWeb/Accessibility/expected/form-validation.txtTests/LibWeb/Accessibility/expected/generic-transparency.txtTests/LibWeb/Accessibility/expected/heading-and-emoji.txtTests/LibWeb/Accessibility/expected/heading-levels.txtTests/LibWeb/Accessibility/expected/headings.txtTests/LibWeb/Accessibility/expected/hidden-content.txtTests/LibWeb/Accessibility/expected/image-alt.txtTests/LibWeb/Accessibility/expected/images.txtTests/LibWeb/Accessibility/expected/inline-text.txtTests/LibWeb/Accessibility/expected/input-states.txtTests/LibWeb/Accessibility/expected/interactive-elements.txtTests/LibWeb/Accessibility/expected/landmarks.txtTests/LibWeb/Accessibility/expected/link-url.txtTests/LibWeb/Accessibility/expected/links-and-buttons.txtTests/LibWeb/Accessibility/expected/lists.txtTests/LibWeb/Accessibility/expected/menu-and-menubar.txtTests/LibWeb/Accessibility/expected/mixed-content.txtTests/LibWeb/Accessibility/expected/multiple-tables.txtTests/LibWeb/Accessibility/expected/named-generic.txtTests/LibWeb/Accessibility/expected/nested-lists.txtTests/LibWeb/Accessibility/expected/nested-structure.txtTests/LibWeb/Accessibility/expected/option-aria-label.txtTests/LibWeb/Accessibility/expected/radio-and-select.txtTests/LibWeb/Accessibility/expected/role-presentation-conflict.txtTests/LibWeb/Accessibility/expected/role-presentation.txtTests/LibWeb/Accessibility/expected/select-hidden-options.txtTests/LibWeb/Accessibility/expected/select-optgroup.txtTests/LibWeb/Accessibility/expected/semantic-sections.txtTests/LibWeb/Accessibility/expected/separator-and-roles.txtTests/LibWeb/Accessibility/expected/skip-nav.txtTests/LibWeb/Accessibility/expected/table-complex.txtTests/LibWeb/Accessibility/expected/table-headers.txtTests/LibWeb/Accessibility/expected/table-no-header.txtTests/LibWeb/Accessibility/expected/table-single-row.txtTests/LibWeb/Accessibility/expected/table-spans.txtTests/LibWeb/Accessibility/expected/tables.txtTests/LibWeb/Accessibility/expected/text-formatting.txtTests/LibWeb/Accessibility/expected/text-values.txtTests/LibWeb/Accessibility/expected/values-range.txtTests/LibWeb/Accessibility/input/accesskey.htmlTests/LibWeb/Accessibility/input/aria-checked-override.htmlTests/LibWeb/Accessibility/input/aria-current.htmlTests/LibWeb/Accessibility/input/aria-described.htmlTests/LibWeb/Accessibility/input/aria-description.htmlTests/LibWeb/Accessibility/input/aria-hidden-inheritance.htmlTests/LibWeb/Accessibility/input/aria-labels.htmlTests/LibWeb/Accessibility/input/aria-live-assertive.htmlTests/LibWeb/Accessibility/input/aria-live-regions.htmlTests/LibWeb/Accessibility/input/aria-multiselectable.htmlTests/LibWeb/Accessibility/input/aria-pressed.htmlTests/LibWeb/Accessibility/input/aria-roles.htmlTests/LibWeb/Accessibility/input/aria-states.htmlTests/LibWeb/Accessibility/input/basic-structure.htmlTests/LibWeb/Accessibility/input/checkbox-states.htmlTests/LibWeb/Accessibility/input/complex-form.htmlTests/LibWeb/Accessibility/input/deeply-nested-landmarks.htmlTests/LibWeb/Accessibility/input/details-expanded.htmlTests/LibWeb/Accessibility/input/dialog.htmlTests/LibWeb/Accessibility/input/display-contents.htmlTests/LibWeb/Accessibility/input/document-language.htmlTests/LibWeb/Accessibility/input/empty-and-hidden.htmlTests/LibWeb/Accessibility/input/form-controls.htmlTests/LibWeb/Accessibility/input/form-validation.htmlTests/LibWeb/Accessibility/input/generic-transparency.htmlTests/LibWeb/Accessibility/input/heading-and-emoji.htmlTests/LibWeb/Accessibility/input/heading-levels.htmlTests/LibWeb/Accessibility/input/headings.htmlTests/LibWeb/Accessibility/input/hidden-content.htmlTests/LibWeb/Accessibility/input/image-alt.htmlTests/LibWeb/Accessibility/input/images.htmlTests/LibWeb/Accessibility/input/inline-text.htmlTests/LibWeb/Accessibility/input/input-states.htmlTests/LibWeb/Accessibility/input/interactive-elements.htmlTests/LibWeb/Accessibility/input/landmarks.htmlTests/LibWeb/Accessibility/input/link-url.htmlTests/LibWeb/Accessibility/input/links-and-buttons.htmlTests/LibWeb/Accessibility/input/lists.htmlTests/LibWeb/Accessibility/input/menu-and-menubar.htmlTests/LibWeb/Accessibility/input/mixed-content.htmlTests/LibWeb/Accessibility/input/multiple-tables.htmlTests/LibWeb/Accessibility/input/named-generic.htmlTests/LibWeb/Accessibility/input/nested-lists.htmlTests/LibWeb/Accessibility/input/nested-structure.htmlTests/LibWeb/Accessibility/input/option-aria-label.htmlTests/LibWeb/Accessibility/input/radio-and-select.htmlTests/LibWeb/Accessibility/input/role-presentation-conflict.htmlTests/LibWeb/Accessibility/input/role-presentation.htmlTests/LibWeb/Accessibility/input/select-hidden-options.htmlTests/LibWeb/Accessibility/input/select-optgroup.htmlTests/LibWeb/Accessibility/input/semantic-sections.htmlTests/LibWeb/Accessibility/input/separator-and-roles.htmlTests/LibWeb/Accessibility/input/skip-nav.htmlTests/LibWeb/Accessibility/input/table-complex.htmlTests/LibWeb/Accessibility/input/table-headers.htmlTests/LibWeb/Accessibility/input/table-no-header.htmlTests/LibWeb/Accessibility/input/table-single-row.htmlTests/LibWeb/Accessibility/input/table-spans.htmlTests/LibWeb/Accessibility/input/tables.htmlTests/LibWeb/Accessibility/input/text-formatting.htmlTests/LibWeb/Accessibility/input/text-values.htmlTests/LibWeb/Accessibility/input/values-range.htmlTests/LibWeb/AccessibilityBridge/.gitignoreTests/LibWeb/AccessibilityBridge/CMakeLists.txtTests/LibWeb/AccessibilityBridge/README.mdTests/LibWeb/AccessibilityBridge/harness/__init__.pyTests/LibWeb/AccessibilityBridge/harness/ladybird.pyTests/LibWeb/AccessibilityBridge/harness/orca.pyTests/LibWeb/AccessibilityBridge/harness/testcase.pyTests/LibWeb/AccessibilityBridge/harness/tree.pyTests/LibWeb/AccessibilityBridge/input/abbreviations.htmlTests/LibWeb/AccessibilityBridge/input/listitems.htmlTests/LibWeb/AccessibilityBridge/input/paragraphs.htmlTests/LibWeb/AccessibilityBridge/input/roles.htmlTests/LibWeb/AccessibilityBridge/input/states.htmlTests/LibWeb/AccessibilityBridge/input/tables.htmlTests/LibWeb/AccessibilityBridge/input/tree_shape.htmlTests/LibWeb/AccessibilityBridge/run_tests.shTests/LibWeb/AccessibilityBridge/tests/__init__.pyTests/LibWeb/AccessibilityBridge/tests/test_attributes.pyTests/LibWeb/AccessibilityBridge/tests/test_character_rect.pyTests/LibWeb/AccessibilityBridge/tests/test_hypertext_fallback.pyTests/LibWeb/AccessibilityBridge/tests/test_interfaces.pyTests/LibWeb/AccessibilityBridge/tests/test_lifecycle.pyTests/LibWeb/AccessibilityBridge/tests/test_line_boundary.pyTests/LibWeb/AccessibilityBridge/tests/test_regressions.pyTests/LibWeb/AccessibilityBridge/tests/test_roles.pyTests/LibWeb/AccessibilityBridge/tests/test_states.pyTests/LibWeb/AccessibilityBridge/tests/test_tables.pyTests/LibWeb/AccessibilityBridge/tests/test_text_content.pyTests/LibWeb/AccessibilityBridge/tests/test_tree_shape.pyTests/LibWeb/AccessibilityBridge/tests_orca/__init__.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_active_document.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_collection_fallback.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_focus_changed.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_hypertext_fallback.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_say_all.pyTests/LibWeb/AccessibilityBridge/tests_orca/test_structural_navigation.pyTests/LibWeb/AccessibilityBridgeMac/CMakeLists.txtTests/LibWeb/AccessibilityBridgeMac/README.mdTests/LibWeb/AccessibilityBridgeMac/harness/__init__.pyTests/LibWeb/AccessibilityBridgeMac/harness/ladybird.pyTests/LibWeb/AccessibilityBridgeMac/harness/testcase.pyTests/LibWeb/AccessibilityBridgeMac/harness/tree.pyTests/LibWeb/AccessibilityBridgeMac/run_tests.shTests/LibWeb/AccessibilityBridgeMac/tests/__init__.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_lifecycle.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_regressions.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_roles.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_search_predicate.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_states.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_text_markers.pyTests/LibWeb/AccessibilityBridgeMac/tests/test_tree_shape.pyTests/LibWeb/CMakeLists.txtTests/LibWeb/test-web/TestWeb.hTests/LibWeb/test-web/main.cpp
🚧 Files skipped from review as they are similar to previous changes (168)
- Tests/LibWeb/AccessibilityBridgeMac/tests/init.py
- Tests/LibWeb/Accessibility/expected/headings.txt
- Tests/LibWeb/Accessibility/expected/aria-live-regions.txt
- Tests/LibWeb/Accessibility/input/deeply-nested-landmarks.html
- Tests/LibWeb/Accessibility/input/mixed-content.html
- Tests/LibWeb/Accessibility/input/aria-live-assertive.html
- Tests/LibWeb/Accessibility/input/aria-checked-override.html
- Tests/LibWeb/Accessibility/input/table-no-header.html
- Tests/LibWeb/Accessibility/expected/display-contents.txt
- Tests/LibWeb/Accessibility/input/select-hidden-options.html
- Tests/LibWeb/Accessibility/input/empty-and-hidden.html
- Tests/LibWeb/Accessibility/input/lists.html
- Tests/LibWeb/Accessibility/input/skip-nav.html
- Tests/LibWeb/Accessibility/input/display-contents.html
- Tests/LibWeb/Accessibility/input/accesskey.html
- Tests/LibWeb/Accessibility/input/headings.html
- Tests/LibWeb/AccessibilityBridge/input/abbreviations.html
- Tests/LibWeb/Accessibility/input/image-alt.html
- Tests/LibWeb/AccessibilityBridge/.gitignore
- Tests/LibWeb/Accessibility/expected/aria-current.txt
- Tests/LibWeb/Accessibility/expected/values-range.txt
- Tests/LibWeb/Accessibility/expected/menu-and-menubar.txt
- Tests/LibWeb/Accessibility/input/nested-structure.html
- Tests/LibWeb/Accessibility/expected/empty-and-hidden.txt
- Tests/LibWeb/Accessibility/expected/aria-labels.txt
- Tests/LibWeb/Accessibility/input/heading-levels.html
- Tests/LibWeb/Accessibility/input/tables.html
- Tests/LibWeb/Accessibility/input/links-and-buttons.html
- Tests/LibWeb/Accessibility/input/role-presentation.html
- Tests/LibWeb/Accessibility/input/complex-form.html
- Tests/LibWeb/Accessibility/input/checkbox-states.html
- Tests/LibWeb/Accessibility/expected/table-headers.txt
- Libraries/LibWebView/CMakeLists.txt
- Tests/LibWeb/Accessibility/input/inline-text.html
- Tests/LibWeb/AccessibilityBridge/tests_orca/init.py
- Tests/LibWeb/Accessibility/expected/role-presentation.txt
- Tests/LibWeb/Accessibility/input/aria-pressed.html
- Tests/LibWeb/Accessibility/expected/details-expanded.txt
- Tests/LibWeb/Accessibility/input/link-url.html
- Tests/LibWeb/Accessibility/input/semantic-sections.html
- Tests/LibWeb/Accessibility/input/aria-roles.html
- Tests/LibWeb/Accessibility/input/generic-transparency.html
- Tests/LibWeb/Accessibility/input/aria-description.html
- Tests/LibWeb/Accessibility/expected/table-complex.txt
- Tests/LibWeb/Accessibility/input/nested-lists.html
- Tests/LibWeb/Accessibility/expected/aria-described.txt
- Tests/LibWeb/Accessibility/expected/skip-nav.txt
- Services/WebContent/WebContentServer.ipc
- Tests/LibWeb/Accessibility/input/hidden-content.html
- Tests/LibWeb/Accessibility/input/input-states.html
- Tests/LibWeb/Accessibility/input/interactive-elements.html
- Tests/LibWeb/Accessibility/expected/multiple-tables.txt
- Tests/LibWeb/Accessibility/expected/named-generic.txt
- Tests/LibWeb/Accessibility/expected/tables.txt
- Tests/LibWeb/Accessibility/expected/images.txt
- Tests/LibWeb/Accessibility/expected/aria-live-assertive.txt
- Tests/LibWeb/Accessibility/input/heading-and-emoji.html
- Tests/LibWeb/Accessibility/input/aria-live-regions.html
- Libraries/LibWeb/DOM/Element.h
- Tests/LibWeb/Accessibility/input/table-single-row.html
- Tests/LibWeb/Accessibility/expected/aria-hidden-inheritance.txt
- Tests/LibWeb/Accessibility/expected/table-single-row.txt
- Tests/LibWeb/Accessibility/input/text-formatting.html
- Tests/LibWeb/Accessibility/expected/interactive-elements.txt
- Tests/LibWeb/Accessibility/input/images.html
- Tests/LibWeb/Accessibility/input/menu-and-menubar.html
- Tests/LibWeb/Accessibility/expected/semantic-sections.txt
- Tests/LibWeb/Accessibility/input/role-presentation-conflict.html
- Tests/LibWeb/Accessibility/input/details-expanded.html
- Tests/LibWeb/AccessibilityBridge/tests/init.py
- Tests/LibWeb/Accessibility/input/aria-states.html
- Tests/LibWeb/Accessibility/input/basic-structure.html
- Tests/LibWeb/Accessibility/expected/document-language.txt
- Tests/LibWeb/Accessibility/expected/form-controls.txt
- Tests/LibWeb/Accessibility/input/aria-multiselectable.html
- Tests/LibWeb/Accessibility/input/table-headers.html
- Tests/LibWeb/Accessibility/expected/heading-and-emoji.txt
- Tests/LibWeb/Accessibility/input/values-range.html
- Tests/LibWeb/AccessibilityBridge/input/roles.html
- Tests/LibWeb/Accessibility/expected/checkbox-states.txt
- Services/WebContent/WebContentClient.ipc
- Tests/LibWeb/Accessibility/expected/deeply-nested-landmarks.txt
- Tests/LibWeb/Accessibility/expected/link-url.txt
- Tests/LibWeb/Accessibility/expected/radio-and-select.txt
- Tests/LibWeb/Accessibility/expected/text-formatting.txt
- Tests/LibWeb/Accessibility/expected/landmarks.txt
- Tests/LibWeb/Accessibility/expected/links-and-buttons.txt
- Tests/LibWeb/Accessibility/expected/dialog.txt
- Tests/LibWeb/Accessibility/expected/aria-description.txt
- Tests/LibWeb/Accessibility/expected/role-presentation-conflict.txt
- Tests/LibWeb/AccessibilityBridge/input/listitems.html
- Tests/LibWeb/Accessibility/expected/complex-form.txt
- Tests/LibWeb/Accessibility/input/select-optgroup.html
- Tests/LibWeb/Accessibility/expected/inline-text.txt
- Tests/LibWeb/Accessibility/expected/aria-roles.txt
- Tests/LibWeb/Accessibility/expected/aria-checked-override.txt
- Tests/LibWeb/Accessibility/input/separator-and-roles.html
- Tests/LibWeb/Accessibility/expected/table-spans.txt
- Tests/LibWeb/Accessibility/input/form-validation.html
- Tests/LibWeb/Accessibility/expected/form-validation.txt
- Tests/LibWeb/Accessibility/input/multiple-tables.html
- Tests/LibWeb/Accessibility/input/table-spans.html
- Tests/LibWeb/Accessibility/expected/generic-transparency.txt
- Tests/LibWeb/Accessibility/expected/basic-structure.txt
- Tests/LibWeb/Accessibility/input/radio-and-select.html
- Tests/LibWeb/Accessibility/expected/aria-multiselectable.txt
- Tests/LibWeb/Accessibility/expected/nested-structure.txt
- Libraries/LibWeb/HTML/HTMLTableElement.h
- Tests/LibWeb/AccessibilityBridgeMac/harness/testcase.py
- Tests/LibWeb/Accessibility/expected/table-no-header.txt
- Libraries/LibWebView/Forward.h
- Tests/LibWeb/Accessibility/expected/separator-and-roles.txt
- Libraries/LibWebView/ViewImplementation.h
- Tests/LibWeb/Accessibility/expected/lists.txt
- Tests/LibWeb/Accessibility/expected/heading-levels.txt
- Tests/LibWeb/Accessibility/input/aria-current.html
- Tests/LibWeb/AccessibilityBridge/tests/test_tables.py
- Libraries/LibWeb/CSS/SelectorMatching.cpp
- Tests/LibWeb/Accessibility/expected/text-values.txt
- Libraries/LibWebView/PageInfo.h
- Tests/LibWeb/AccessibilityBridge/input/paragraphs.html
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_lifecycle.py
- Tests/LibWeb/Accessibility/expected/select-hidden-options.txt
- Tests/LibWeb/Accessibility/input/dialog.html
- Tests/LibWeb/Accessibility/input/table-complex.html
- Tests/LibWeb/AccessibilityBridge/tests/test_character_rect.py
- Tests/LibWeb/AccessibilityBridge/tests/test_lifecycle.py
- Tests/LibWeb/Accessibility/input/option-aria-label.html
- Libraries/LibWebView/AccessibilityTreeManager.h
- Libraries/LibWeb/DOM/AccessibilityTreeNode.h
- Tests/LibWeb/AccessibilityBridge/input/tables.html
- Tests/LibWeb/Accessibility/input/aria-described.html
- Tests/LibWeb/Accessibility/input/document-language.html
- .github/actions/setup/action.yml
- Libraries/LibWebView/WebContentClient.h
- Services/WebContent/PageClient.h
- Tests/LibWeb/Accessibility/expected/mixed-content.txt
- Libraries/LibWeb/DOM/Document.h
- Libraries/LibWebView/AccessibilityNodeData.cpp
- Tests/LibWeb/Accessibility/expected/select-optgroup.txt
- Tests/LibWeb/AccessibilityBridge/harness/testcase.py
- Tests/LibWeb/Accessibility/expected/input-states.txt
- Tests/LibWeb/AccessibilityBridge/tests_orca/test_structural_navigation.py
- Tests/LibWeb/AccessibilityBridge/input/tree_shape.html
- Tests/LibWeb/AccessibilityBridge/tests_orca/test_active_document.py
- Services/WebContent/ConnectionFromClient.h
- Tests/LibWeb/Accessibility/expected/aria-pressed.txt
- Tests/LibWeb/AccessibilityBridgeMac/CMakeLists.txt
- Libraries/LibWeb/Page/Page.h
- Tests/LibWeb/test-web/main.cpp
- Libraries/LibWebView/WebContentClient.cpp
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_search_predicate.py
- Tests/LibWeb/AccessibilityBridge/CMakeLists.txt
- Tests/LibWeb/AccessibilityBridge/tests/test_regressions.py
- Libraries/LibWebView/AccessibilityTreeManager.cpp
- Libraries/LibWebView/ViewImplementation.cpp
- Libraries/LibWebView/AccessibilityNodeData.h
- Tests/LibWeb/AccessibilityBridge/tests/test_line_boundary.py
- Tests/LibWeb/AccessibilityBridge/tests/test_roles.py
- Libraries/LibWeb/DOM/Element.cpp
- Libraries/LibWeb/DOM/AccessibilityTreeNode.cpp
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_roles.py
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_regressions.py
- Libraries/LibWeb/DOM/Node.cpp
- Tests/LibWeb/AccessibilityBridgeMac/tests/test_text_markers.py
- Tests/LibWeb/AccessibilityBridge/run_tests.sh
- Tests/LibWeb/AccessibilityBridgeMac/run_tests.sh
- Services/WebContent/ConnectionFromClient.cpp
This change adds a mechanism for accessibility-tree serialization to a toolkit-neutral data model, plus the IPC and view plumbing necessary for delivering updates from it to the UI layer — along with tests.
This change wires up our Qt and AppKit backends to expose/serve our accessibility tree to screen readers and other platform AT.
This change adds custom Orca scripts for making Orca interaction with Ladybird work as expected for end users — by runtime monkey-patching Orca internals to work around limitations in the Qt AT-SPI2 bridge.
This change suppresses some log messages that get logged for every single Orca navigation action a user makes — warning about upstream Qt gaps in scroll-related behavior we can't fix from our side. Otherwise, without this change, those messages end up flooding the console when Orca is in use.
This change fixes a problem that prevents Orca and other AT clients from seeing our accessibility tree. The problem was latent but got triggered by 1bf03c2 (“CMake: Add GTK4 as a GUI framework option”).
This change adds a test harness to catch regressions at a layer we can't see with tests that just examine a tree dump of the accessibility tree: our Qt AT-SPI2 bridge’s actual output over D-Bus. Each test launches a Ladybird process against an HTML fixture, connects as an AT-SPI2 client via Python gi.repository.Atspi, and asserts on what the bridge emits.
This change adds a test harness that launches Ladybird, imports Orca and our custom Orca script, monkey-patches Orca to capture utterances, then drives Orca commands directly, and asserts on what Orca would have said. With all that, we can catch regressions that are invisible to our other AT/screen-reader test harnesses. This change also sets up CI to run all the Orca and Qt AT-SPI2 tests.
This change adds a test harness to catch regressions at a layer we can't see with tests that just examine a tree dump of the accessibility tree: our actual macOS NSAccessibility output. Each test launches a Ladybird process on a fixture HTML page, connects to it as an NSAccessibility client via PyObjC + AXUIElement, and asserts on what the AX surface actually exposes — and on what VoiceOver would see while walking that. This change also sets up CI to run this test harness on its tests.
When Ladybird never registers an AXWebArea, the harness raised a bare timeout with no indication of why — whether the helper process died, never opened a window, or built a tree without the expected web area. Dump what is on the AX surface into the timeout error — process liveness, window count, and a depth-capped AXRole/AXTitle tree — and keep the head of Ladybird’s stderr, where startup failures log.
This makes Orca and VoiceOver work with Ladybird (AppKit UI on macOS, and Qt UI on both Linux and macOS).
The architecture follows the Chromium/Firefox model: the accessibility tree is serialized from WebContent via IPC, cached in a platform-agnostic
AccessibilityTreeManager(in LibWebView), and presented through platform-specific wrapper objects in the UI process. The IPC serialization, tree management, and WebContent-side tree-building are shared across platforms (macOS and Linux) and across UIs (AppKit and Qt).macOS (AppKit and Qt) — VoiceOver
LadybirdAccessibilityElementimplementsNSAccessibilitydirectly. Both the AppKit UI and the (macOS) Qt UI share it: the Qt build uses an NSView overlay with three very-small, simple, targeted swizzles onQMacAccessibilityElementto connect Qt’s bridge to the shared elements.Linux (Qt) — Orca
AccessibilityInterfaceimplementsQAccessibleInterface, exposing the tree through Qt’s built-in AT-SPI2 bridge — with a custom Orca script which adds patches that work around limitations in that bridge. (The scripts are embedded as Qt resources and auto-installed on Ladybird startup.)Tests
test-web --filter "Accessibility/*".Also included in the PR branch are fixes for several bugs in the existing ARIA/ACCNAME infrastructure that were discovered and fixed during development:
Element::is_actually_disabled()is now used for theis_disabledfieldElement::exclude_from_accessibility_tree()handlesaria-hidden="true"Element::exclude_from_accessibility_tree()handlesvisibility:hidden/visibility:collapseElement::is_referenced()checksaria-describedbyin addition toaria-labelledbyaccessible_description()usesNameOrDescription::Nameto compute alternatives foraria-describedbyrefsNode::build_accessibility_tree()skips descendants of excluded elements entirelyrole=none/presentationbut global ARIA attributes are not excludedThe PR also adds a
Documentation/ScreenReaderSupport.mdfile which documents design and implementation details.