Repository navigation
fix(web): scope shadow landmarks to outer sections - #943
okwasniewski merged 3 commits into
Conversation
Signed-off-by: Rudy Celekli <rudy@gradiahq.com>
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/web/src/in-page/read-semantics.ts">
<violation number="1" location="packages/web/src/in-page/read-semantics.ts:136">
P2: A header slotted into a shadow-root `<section>` is still classified as a page-level `banner`, because this walk never follows its composed-tree slot ancestor. Walk element parents individually and follow `assignedSlot` before crossing to the shadow host.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Turn on auto-fix | Re-trigger cubic
| if (candidate.closest(SECTIONING_SCOPE) !== null) return false; | ||
| const root = candidate.getRootNode(); | ||
| candidate = root instanceof ShadowRoot ? root.host : null; |
There was a problem hiding this comment.
P2: A header slotted into a shadow-root <section> is still classified as a page-level banner, because this walk never follows its composed-tree slot ancestor. Walk element parents individually and follow assignedSlot before crossing to the shadow host.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/web/src/in-page/read-semantics.ts, line 136:
<comment>A header slotted into a shadow-root `<section>` is still classified as a page-level `banner`, because this walk never follows its composed-tree slot ancestor. Walk element parents individually and follow `assignedSlot` before crossing to the shadow host.</comment>
<file context>
@@ -131,7 +131,14 @@ export const readSemanticsFunction = <Mode extends SemanticMode>(
- const isPageLevel = (el: Element): boolean => el.closest(SECTIONING_SCOPE) === null;
+ const isPageLevel = (el: Element): boolean => {
+ for (let candidate: Element | null = el; candidate !== null;) {
+ if (candidate.closest(SECTIONING_SCOPE) !== null) return false;
+ const root = candidate.getRootNode();
+ candidate = root instanceof ShadowRoot ? root.host : null;
</file context>
| if (candidate.closest(SECTIONING_SCOPE) !== null) return false; | |
| const root = candidate.getRootNode(); | |
| candidate = root instanceof ShadowRoot ? root.host : null; | |
| if (candidate.matches(SECTIONING_SCOPE)) return false; | |
| if (candidate.assignedSlot !== null) { | |
| candidate = candidate.assignedSlot; | |
| continue; | |
| } | |
| if (candidate.parentElement !== null) { | |
| candidate = candidate.parentElement; | |
| continue; | |
| } | |
| const root = candidate.getRootNode(); | |
| candidate = root instanceof ShadowRoot ? root.host : null; |
There was a problem hiding this comment.
[gpt-6.1-sol] responding on behalf of rudycelekli
I checked this with native Chromium and Playwright 1.63. A header/footer assigned to a slot inside a shadow <section> is sectionheader/sectionfooter in Chrome's AX tree, while the public Playwright getByRole('banner', { name: 'Card' }) and getByRole('contentinfo', { name: 'Card footer' }) each return one match. Its aria snapshot also includes those landmark roles inside the region. The current observation follows the public role locator's DOM/host ancestry here; following the slot would change that relationship. This PR fixes section ancestry across shadow hosts where the oracles agree. I am leaving this thread open for the maintainer's decision on which oracle contract slotted landmark observations should follow, rather than claiming the disagreement is fixed.
|
Thanks! I merged main and dropped the |
The shadow section landmarks fixture added with tester-army#943 pinned the scoping for two of the ten shapes isPageLevel scopes a header or footer to: an article element and a region role. The other eight (aside, main, nav, section, and the article, complementary, main, and navigation roles) had no shadow-nested host in any fixture, so a regression in one of them would pass the cross-check until another fixture happened to trip it. The fixture now nests a header and footer two shadow roots deep under each of the ten scoping shapes, with the page-level control beside them. The run stays green with no new expected.txt lines: the tree and both oracles agree on every header and footer the added hosts carry.
The shadow section landmarks fixture added with tester-army#943 pinned the scoping for two of the ten shapes isPageLevel scopes a header or footer to: an article element and a region role. The other eight (aside, main, nav, section, and the article, complementary, main, and navigation roles) had no shadow-nested host in any fixture, so a regression in one of them would pass the cross-check until another fixture happened to trip it. The fixture now nests a header and footer two shadow roots deep under each of the ten scoping shapes, with the page-level control beside them. The run stays green with no new expected.txt lines: the tree and both oracles agree on every header and footer the added hosts carry.
What changed
Keep implicit header/footer landmarks scoped by article, section, navigation, aside, and region ancestors across shadow hosts.
isPageLevelpreviously checked only the current shadow tree, so a nested card header inside an article became a page banner in the agent's observation. Top-level shadow headers and footers retain their page landmark roles.Adds native open/closed shadow regressions and a Chrome/Playwright/axe crosscheck fixture. Updates the browser docs and adds a patch changeset. No public type or wire format changes.
Verified
Ran it locally: yes.
pnpm checkremains blocked by existing dead-code/mobile dependency failures. The Next.js benchmark and Android runtime are unavailable in this lean local environment; hosted checks are pending and are not claimed green.Screened current open PR files. #871 also touches role semantics but changes presentational roles on focusable controls, leaving this helper unchanged; #926 changes text flow. The existing role fallback, ARIA state, and select choice PRs address different defects.
AI assistance: implemented and verified with Codex on behalf of Rudy Celekli; the exact signed patch was reviewed in a separate source-review context before publication.