feat(cua-driver): expose action names in get_window_state elements - #3377
feat(cua-driver): expose action names in get_window_state elements#3377Wangxiaoxiaoa wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
thanks for fixing this across all three platforms. please address these items before this lands:
- make the linux and windows tests call production code. they currently rebuild the json, so they pass even if the real
actionsfield is removed. - list
actionsin all threeget_window_statedescriptions. - filter blank linux action names; at-spi can otherwise produce
actions: [""]. - reuse the existing macos test helper and remove the unused
build_elements_arrayshim. - preserve the macos contributor credit from #2622 with a github-linked
co-authored-bytrailer and note that it was salvaged from that pr.
for example, extracting each linux and windows element-entry closure into a small platform-local function would let the tests exercise the real path without adding a shared wrapper.
eba848c to
1481a8d
Compare
|
@injaneity thanks for the review — comments addressed, PTAL.
Verified: |
injaneity
left a comment
There was a problem hiding this comment.
thanks for carrying this across all three platforms. the main approach is right: the walkers already collect actions, so adding them while each platform builds structuredContent.elements fixes the actual gap.
there is one macos regression to fix before this lands:
_observation_onlyintentionally does not register a snapshot. before this change, that path used the tokenless builder. it now callsbuild_elements_array_with_token(&r.nodes, 0), so every returned element gets ans0:*token that was never registered and cannot resolve. these tokens look usable but always fail. please replace the two macos builders with one builder that acceptsOption<u32>and only addselement_tokenforSome(snapshot_id). passNonefor observation-only results, and add a test that confirms actions are present whileelement_tokenis absent in that case. this keeps the old wrapper removed without losing its required behavior.
please also provide the native validation promised in the description. the current checks only cover contributor attribution and release metadata, so neither the macos nor windows test is visible as having run. run the focused tests on those platforms, or state the concrete environment limitation if a native lane is unavailable.
with the macos token path corrected, the change stays focused: keep the platform-local entry builders, emit actions only when non-empty, and avoid adding a shared wrapper because the three native node types have different fields.
4c33bf9 to
fa8dec1
Compare
|
Hi @injaneity, thanks for the re-review. Changes since last round:
Validation:
|
fa8dec1 to
a649e79
Compare
injaneity
left a comment
There was a problem hiding this comment.
thanks for the updates here. the main approach is right, and the earlier comments are mostly addressed: the linux and windows tests now use the production builders, all three descriptions include actions, the macos wrapper is gone, observation-only macos results no longer get unusable tokens, the regression test is present, and the #2622 contributor credit is preserved.
there are a few remaining items before this can land safely.
rebase and preserve the new capture-only path
this branch now conflicts with main. #3516 changed the same linux and windows get_window_state sections to support include_accessibility_tree:false, so the conflict resolution will touch the exact production path changed here.
please rebase onto current main and keep both behaviors:
- only build action-bearing elements when an accessibility tree was requested and returned;
- keep
include_accessibility_tree:falsefree of the at-spi, ax, or uia walk; - retain the macos
option<u32>behavior so observation-only results have actions but noelement_token; - keep query projection after the action-bearing entries are built.
please rerun the focused tests on linux, macos, and windows after resolving the conflicts. the existing reported runs cover the old base, not the final merged code. the visible github checks currently show only attribution and release metadata, so please record the new native results in the pr after the rebase.
filter whitespace-only linux action names
linux currently uses .filter(|a| !a.is_empty()). this removes "", but values such as " " still produce an actions field with no usable action name.
please use .filter(|a| !a.trim().is_empty()) and extend the blank-action test with a whitespace-only value. the original string can stay unchanged when it is valid.
update the release title
this adds a new public structured field on all three platforms, and the linked issue also describes it as a new capability. under the repository release rules, this should use feat(cua-driver), not fix(cua-driver).
please update the title after the final rebase, for example:
feat(cua-driver): expose action names in get_window_state elements
once the branch is rebased, the capture-only behavior is preserved, the linux filter handles whitespace, and the native tests are rerun on the final code, this should be in good shape.
a649e79 to
1a67c1d
Compare
|
Hi @injaneity, thanks for the follow-up. I've addressed the remaining items:
Validation after the rebase:
|
|
@Wangxiaoxiaoa you will need to fix the failing ci for docs, thanks! |
Expose AT-SPI/AX/UIA action names in the structured `elements` array of `get_window_state` on Linux, macOS, and Windows. The action list is omitted when empty, and Linux filters blank/whitespace-only names. macOS observation-only snapshots (`_observation_only`) continue to emit action names but do not emit `element_token`, since no snapshot is registered in that path. Refs trycua#3376. Salvaged from trycua#2622 (macOS). Co-authored-by: Haoqing Wang <hqhq1025@users.noreply.github.com>
d77f8ac to
fc2160b
Compare
|
@injaneity fixed the failing docs CI by regenerating docs/content/docs/reference/cua-driver/mcp-tools.mdx with pnpm --dir docs docs:generate:cua-driver. Verified locally with docs:check:cua-driver. The branch is now squashed to a single commit. Please re-review. |
|
thanks @Wangxiaoxiaoa — we're carrying this forward in #3617 at maintainer request, preserving your authorship and the #2622 coauthor credit. the docs generator currently depends on the host platform, while docs CI runs on macOS; that is our contributor-workflow limitation to handle. we'll regenerate on macOS and record validation in the linked draft. this has not shipped yet. |
Expose AT-SPI/AX/UIA action names in the structured `elements` array of `get_window_state` on Linux, macOS, and Windows. The action list is omitted when empty, and Linux filters blank/whitespace-only names. macOS observation-only snapshots (`_observation_only`) continue to emit action names but do not emit `element_token`, since no snapshot is registered in that path. Refs #3376. Salvaged from #2622 (macOS). Co-authored-by: Haoqing Wang <hqhq1025@users.noreply.github.com> (cherry picked from commit fc2160b) Salvaged from #3377.
…3617) * feat(cua-driver): expose action names in get_window_state elements Expose AT-SPI/AX/UIA action names in the structured `elements` array of `get_window_state` on Linux, macOS, and Windows. The action list is omitted when empty, and Linux filters blank/whitespace-only names. macOS observation-only snapshots (`_observation_only`) continue to emit action names but do not emit `element_token`, since no snapshot is registered in that path. Refs #3376. Salvaged from #2622 (macOS). Co-authored-by: Haoqing Wang <hqhq1025@users.noreply.github.com> (cherry picked from commit fc2160b) Salvaged from #3377. * docs(cua-driver): regenerate action reference on macOS --------- Co-authored-by: Wangxiaoxiaoa <102247755+Wangxiaoxiaoa@users.noreply.github.com>
|
closing as superseded by #3617 |
Supersedes #3307.
The Linux, macOS, and Windows accessibility walkers in cua-driver already collect action names, but none of the platform
get_window_statetools surfaced them in the structuredelementsarray. This caused callers to see missing/empty actions eventhough the underlying element was actionable.
This PR adds an optional
actionsfield (omitted when empty) to the structured element entry on all three platforms:AtspiNode.actionsinGetWindowStateTool.AXNode.actionsinbuild_elements_array_with_token.UiaNode.actionsinGetWindowStateTool.Apologies to @injaneity for the noise: the original #3307 branch was accidentally recreated while adding the requested macOS
and Windows coverage. This PR contains the full cross-platform change.
Validation
cargo fmt -- --checkcargo check -p platform-linuxcargo check -p platform-macoscargo check -p platform-windowscargo test -p platform-linux get_window_state_actions_testsmacOS and Windows tests are written but cannot be run locally on Linux; they will be exercised by CI.
Related work
Fixes #3376
Refs #2622 (macOS-only prior attempt)
Refs #3307 (superseded)