🐛 Prefer anchor and role="link" text over ancestor ARIA labels for RUM action names - #5013
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d7031770d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| element.nodeName === 'A' || | ||
| element.getAttribute('role') === 'button' || | ||
| element.getAttribute('role') === 'link' |
There was a problem hiding this comment.
issue: When an <a> or role="link" has both text and its own aria-label or aria-labelledby (for example, <a aria-label="Close">×</a>), this widened strategy now returns the text before reaching the later ARIA strategies, changing the recorded action name from Close to ×. Handle links in a separate strategy after the two ARIA strategies; their text will still be selected before walking to an ancestor, without overriding the link's own accessible name.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
It's coherent with the "button" case. We might consider later to follow more closely the accname spec.
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: 93a80e2 | Docs | View more details | Give us feedback! |
3d70317 to
93a80e2
Compare
|
Thanks for the review @bdibon. FYI I've amended my original commit in this PR with a signature given the branch protection rule ("commits must have verified signatures"). |
3763df2
into
DataDog:main
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: done
and removed
mergequeue-status: in_progress
labels
2 participants
Motivation
Addresses bug report #5007
getActionNameFromElementruns everypriorityStrategiesentry against the target and up to 10 ancestors before ever falling back to text content of the target. When an anchor sits inside an ancestor with anaria-labeloraria-labelledby(for example arole="dialog"whosearia-labelledbypoints at a visually-hidden title), RUM records the ancestor's label as the action name instead of the visible anchor text the user actually clicked.This is common with accessible dialog primitives that require a title element on the dialog root, and it produces surprising click names like
click on This is a modal windowon every anchor inside the modal.Changes
The ("BUTTON, LABEL or button-like element text") priority strategy now also matches
<a>androle="link", mirroring the existing symmetry with<button>androle="button". The text content of the clicked anchor is picked at iteration 0 of the walk, so ancestor ARIA attributes no longer beat it.Behavior change to flag: an
<a>(orrole="link") that carries both a visible text label and its ownaria-labelwill now resolve to the visible text instead of thearia-label. That matches how<button>has always worked, but any team relying on the previous behavior will see a name shift. Not gated behind a config flag but happy to do so if desired.Test instructions
Checklist