Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .changeset/great-eagles-jam.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
'@svelte-vitals/core': patch
'svelte-vitals': patch
'@svelte-vitals/vite': patch
---

Two more corrections from the a11y rule-validity review.

- **`a11y/no-missing-id-ref` read a text fragment as an id reference.** `href="#:~:text=hello%20world"`
is a shipped web-platform feature — everything from `:~:` on instructs the user agent to find
text, and names no element — so the rule reported a missing id, printing the percent-decoded form
and sending the reader to look for a string that is not in their source. The directive is now
stripped in both modes, and any element fragment before it is still checked: `#section:~:text=hi`
resolves against `id="section"` exactly as `#section` would.
- **`a11y/interactive-nesting` did not say which element it meant.** A finding read
`<button> is nested inside interactive <div>`, which is unactionable on a page with many divs.
When the container is a container because of its `role`, the message now names it:
`<button> is nested inside interactive <div role="button">`.
2 changes: 1 addition & 1 deletion docs/blume.translations.json
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
"ja": "6d1982aab4ad28c9"
},
"src/content/docs/rules/a11y/no-missing-id-ref.md": {
"ja": "89f34b548e77be54"
"ja": "6dd40fe0369c0309"
},
"src/content/docs/rules/a11y/placeholder-label-option.md": {
"ja": "f3a20fe65f6fe0a5"
Expand Down
2 changes: 1 addition & 1 deletion docs/src/content/docs/ja/rules/a11y/no-missing-id-ref.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ description: id参照は、合成後のルートのどこかに存在する id

`for`・`aria-labelledby`・`aria-describedby`・`aria-controls`・`aria-activedescendant` の各属性、または同一ページ内を指す `href="#…"` について、参照先の `id` がルートを構成するレイアウトチェーン(`+layout.svelte` から `+page.svelte` まで)・そこから解決したローカルコンポーネント・`src/app.html` シェルのどこにも存在しない場合に検出します。

`href="#top"` は対象外です。この断片には `id="top"` を持つ要素が存在しなくても、ブラウザがドキュメント先頭までスクロールするため、参照切れにはなりません。
`href="#top"` は対象外です。この断片には `id="top"` を持つ要素が存在しなくても、ブラウザがドキュメント先頭までスクロールするため、参照切れにはなりません。**テキストフラグメントのディレクティブ**は照合前に取り除きます。`:~:` 以降はテキストを探すためのユーザーエージェント向けの指示で要素を指さないため `href="#:~:text=…"` は何も参照しませんが、`href="#section:~:text=…"` は `href="#section"` と同じく `id="section"` に対して照合されます。

**このルールは、他のルート単位 a11y ルールに比べて実行されるルート数がかなり少なくなります。** 他のルート単位 a11y ルールはいずれも「これが2つ以上存在する」という主張であり、解決できないコンポーネントが何を持っていようとその主張は揺らぎません(すべてのルートで実行され、解決できないコンポーネントは偽陰性のコストで済みます)。一方このルールは「どこにもこの id を定義する要素が存在しない」という逆の主張であり、解決できないコンポーネントこそがまさにその id を持っている可能性があります。そのため閉世界を必要とし、ルートの構成全体が完全に解決済みであると確認できた場合にのみ実行します——推移的閉包に含まれるすべてのコンポーネントが実際に解決済み(`node_modules`/ライブラリからのインポート、`.svelte` ではなく `index.ts` に解決される barrel インポート、動的に選ばれるコンポーネントのいずれも無い)、深さ制限による打ち切りがない、`{@html}` がない、スプレッド属性がない、合成対象のどのファイルにも動的な `id` がない、という条件です。いずれか一つでも満たさないルートは**丸ごとスキップ**され、そのルートについては何も出力しません(偽の「合格」にはしません)。

Expand Down
2 changes: 1 addition & 1 deletion docs/src/content/docs/rules/a11y/no-missing-id-ref.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ description: An id reference should point to an id that exists somewhere in the

Flags a literal `for`, `aria-labelledby`, `aria-describedby`, `aria-controls`, or `aria-activedescendant` attribute, or a same-page `href="#…"`, whose referenced `id` does not exist anywhere in the route's composed layout chain (every `+layout.svelte` up to the route's `+page.svelte`), its resolved local components, or the `src/app.html` shell.

`href="#top"` is exempt — the browser scrolls to the document top for that fragment even when no element has `id="top"`, so it is never a missing reference.
`href="#top"` is exempt — the browser scrolls to the document top for that fragment even when no element has `id="top"`, so it is never a missing reference. A **text-fragment directive** is stripped before the lookup: everything from `:~:` on instructs the user agent to find text and names no element, so `href="#:~:text=…"` references nothing — while `href="#section:~:text=…"` still resolves against `id="section"`, exactly as `href="#section"` would.

**This rule runs on far fewer routes than the other route-scoped a11y rules.** The other route-scoped a11y rules each claim "two of these exist", which stays true no matter what an unresolved component might also contribute — they run on every route, an unresolved component only costs a false negative. This rule claims the opposite — "no element anywhere defines this id" — and an unresolved component could be exactly where that id lives, so it needs a closed world: it runs only on a route whose entire composition is verified fully resolved — every component in the transitive closure actually resolved (no `node_modules`/library import, no barrel import that resolves to an `index.ts` rather than a `.svelte` file, no dynamically chosen component, no depth truncation), no `{@html}`, no spread attribute, and no dynamic `id` anywhere in any composed file. A route that fails any of these conditions is **skipped entirely** — the rule emits nothing for it, not a false "pass".

Expand Down
6 changes: 4 additions & 2 deletions packages/cli/src/providers/source/parse.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { AST } from 'svelte/compiler';
import type { BranchStep, HeadTag } from '@svelte-vitals/core/internal';
import {
stripTextDirective,
parseSvelte,
CHILD_NODE_KEYS,
lineOf,
Expand Down Expand Up @@ -435,9 +436,10 @@ function collectA11y(fragment: AST.Fragment, source: string): ParsedA11y {
else if (v === 'static') emit(ctx, { kind: 'id', key: attrTextOf(attr)!, line });
} else if (attr.name === 'href') {
const href = attrTextOf(attr);
if (href?.startsWith('#') && href.length > 1) {
const fragment = href?.startsWith('#') ? stripTextDirective(href.slice(1)) : '';
if (fragment) {
// Navigation percent-decodes the fragment before matching an id (#caf%C3%A9 → café).
emit(ctx, { kind: 'idref', key: decodeFragmentId(href.slice(1)), line, attr: 'href' });
emit(ctx, { kind: 'idref', key: decodeFragmentId(fragment), line, attr: 'href' });
}
} else if (IDREF_ATTR_SET.has(attr.name)) {
for (const token of splitTokens(attrTextOf(attr))) {
Expand Down
11 changes: 11 additions & 0 deletions packages/core/src/a11y.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,14 @@ export const IDREF_ATTRS: readonly string[] = [
export function isTopFragment(id: string): boolean {
return id.toLowerCase() === 'top';
}

/**
* A fragment with its text directive removed. Everything from the first `:~:` on is user-agent
* instructions for finding text and names no element, while anything before it is still an
* ordinary element fragment — `#section:~:text=hi` targets `id="section"`, `#:~:text=hi` targets
* nothing. Returns an empty string when the fragment is a directive and nothing else.
*/
export function stripTextDirective(fragment: string): string {
const i = fragment.indexOf(':~:');
return i === -1 ? fragment : fragment.slice(0, i);
}
25 changes: 21 additions & 4 deletions packages/core/src/component-parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import {
parseSvelte
} from './svelte-ast.js';
import { isInteractiveElement, isInteractiveContainer, type ElementAttr } from './rules/a11y/interactive.js';
import { splitTokens } from './a11y.js';
import { resolveRole } from './rules/a11y/aria-data.js';

// The Svelte AST is structurally complex and only partially typed for our needs,
// so traversal uses `any`. The node-type strings below are verified against
Expand Down Expand Up @@ -1151,11 +1153,22 @@ function elementAttrs(attributes: Node[]): ElementAttr[] {
/**
* Interactive elements nested inside another interactive container (a11y/interactive-nesting).
* Only `RegularElement` is checked, so `<svelte:element this="button">` is out of static reach
* — same convention as `collectAriaElements`. `stack` tracks the tags of enclosing containers
* — same convention as `collectAriaElements`. `stack` tracks the enclosing containers
* (`isInteractiveContainer`); any interactive element entering while it is non-empty records
* one fact at the descendant's line, keyed to the nearest enclosing container.
*/
function collectInteractiveNestings(node: Node, source: string, acc: InteractiveNestingFact[], stack: string[]): void {
/** The literal role that made an element a container, for the finding's message. */
function containerRoleOf(attrs: ElementAttr[]): string | undefined {
const role = attrs.find((a) => a.name === 'role')?.literal;
return role ? resolveRole(splitTokens(role)) : undefined;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

function collectInteractiveNestings(
node: Node,
source: string,
acc: InteractiveNestingFact[],
stack: { tag: string; role?: string }[]
): void {
if (Array.isArray(node)) {
for (const child of node) collectInteractiveNestings(child, source, acc, stack);
return;
Expand All @@ -1173,14 +1186,18 @@ function collectInteractiveNestings(node: Node, source: string, acc: Interactive
if (node.type === 'RegularElement' && Array.isArray(node.attributes)) {
const attrs = elementAttrs(node.attributes);
if (stack.length > 0 && isInteractiveElement(node.name, attrs)) {
const container = stack[stack.length - 1]!;
acc.push({
containerTag: stack[stack.length - 1]!,
containerTag: container.tag,
...(container.role ? { containerRole: container.role } : {}),
descendantTag: node.name,
line: lineOf(source, node.start)
});
}
if (isInteractiveContainer(node.name, attrs)) {
stack.push(node.name);
// A `<div role="button">` reads as an anonymous div in a finding unless the role is carried.
const role = node.name === 'button' || node.name === 'a' ? undefined : containerRoleOf(attrs);
stack.push({ tag: node.name, ...(role ? { role } : {}) });
opened = true;
}
}
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ export interface BasePathLinkFact {
* container (e.g. `<a href>`) (a11y/interactive-nesting). */
export interface InteractiveNestingFact {
containerTag: string;
/** The container's literal `role`, when that is what made it a container rather than its tag. */
containerRole?: string;
descendantTag: string;
/** 1-based source line of the descendant, or 0 if unknown. */
line: number;
Expand Down
10 changes: 9 additions & 1 deletion packages/core/src/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,15 @@ export type { HeadTag, ResolvedHead, HeadProvider } from './head.js';
export type { ImageInfo, ResolvedImages } from './images.js';
export type { HeadingInfo, ResolvedHeadings } from './headings.js';
export type { BranchStep, A11yOccurrenceInfo, ResolvedA11y } from './a11y.js';
export { foldOccurrences, decodeFragmentId, splitTokens, isTopFragment, LANDMARK_ROLES, IDREF_ATTRS } from './a11y.js';
export {
foldOccurrences,
decodeFragmentId,
splitTokens,
isTopFragment,
stripTextDirective,
LANDMARK_ROLES,
IDREF_ATTRS
} from './a11y.js';
export type {
EachBlockFact,
EffectFact,
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/rules/a11y/interactive-nesting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ export const a11yInteractiveNesting = componentRule({
bad: (c) =>
(c.interactiveNestings ?? []).map((f) => ({
line: f.line,
message: `<${f.descendantTag}> is nested inside interactive <${f.containerTag}>`
message: `<${f.descendantTag}> is nested inside interactive <${f.containerTag}${f.containerRole ? ` role="${f.containerRole}"` : ''}>`
}))
});
14 changes: 14 additions & 0 deletions packages/core/test/component-parse.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1306,6 +1306,20 @@ describe('parseComponentFacts — interactiveNestings (a11y/interactive-nesting)
const src = ['<div role="gridcell"><button>Edit</button></div>', '<div role="combobox"><input /></div>'].join('\n');
expect(parseComponentFacts(src, 'C.svelte').interactiveNestings ?? []).toEqual([]);
});
it('carries the role that made an element a container', () => {
// The role reported is the one a user agent resolves to, not the first token.
expect(
parseComponentFacts('<div role="future-role button"><button>x</button></div>', 'C.svelte').interactiveNestings
).toEqual([{ containerTag: 'div', containerRole: 'button', descendantTag: 'button', line: 1 }]);
const c = parseComponentFacts('<div role="checkbox switch"><a href="/y">z</a></div>', 'C.svelte');
expect(c.interactiveNestings).toEqual([
{ containerTag: 'div', containerRole: 'checkbox', descendantTag: 'a', line: 1 }
]);
// A tag that is a container on its own carries no role — the message would read oddly.
expect(parseComponentFacts('<a href="/x"><button>y</button></a>', 'C.svelte').interactiveNestings).toEqual([
{ containerTag: 'a', descendantTag: 'button', line: 1 }
]);
});
it('resolves a fallback list to its first concrete role', () => {
// `future-role button` is a button: the unknown token is skipped, not applied.
const c = parseComponentFacts('<div role="future-role button"><button>x</button></div>', 'C.svelte');
Expand Down
6 changes: 4 additions & 2 deletions packages/vite/src/providers/rendered/parse-html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
decodeFragmentId,
splitTokens,
isTopFragment,
stripTextDirective,
LANDMARK_ROLES,
IDREF_ATTRS
} from '@svelte-vitals/core/internal';
Expand Down Expand Up @@ -119,10 +120,11 @@ function collectA11y(root: HTMLElement): CollectedA11y {
if (id && id.trim()) ids.push(id);

const href = el.getAttribute('href');
if (href && href.startsWith('#') && href.length > 1) {
const target = href?.startsWith('#') ? stripTextDirective(href.slice(1)) : '';
if (target) {
// Navigation percent-decodes the fragment before matching an id (#caf%C3%A9 → café), and
// the "top of the document" check compares the decoded form too (#%74op === #top).
const fragment = decodeFragmentId(href.slice(1));
const fragment = decodeFragmentId(target);
if (!isTopFragment(fragment)) idRefs.push({ id: fragment, attr: 'href' });
}
for (const attr of IDREF_ATTRS) {
Expand Down
8 changes: 8 additions & 0 deletions packages/vite/test/parse-html.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,14 @@ describe('parse-html: a11y capture (rendered landmark/id parity)', () => {
);
});

it('strips a text-fragment directive and keeps any element fragment before it', () => {
// `:~:` onward is user-agent instructions; `#section:~:text=…` still targets id="section".
const { idRefs } = parseHtmlHead(
doc('<a href="#:~:text=hello%20world">a</a><a href="#section:~:text=hi">b</a><a href="#real">c</a>')
);
expect(idRefs.map((r) => r.id)).toEqual(['section', 'real']);
});

it('excludes #top by its decoded form and skips whitespace-only ids', () => {
const { idRefs, ids } = parseHtmlHead(doc('<a href="#%74op">up</a><a href="#TOP">up</a><p id=" ">x</p>'));
expect(idRefs).toEqual([]);
Expand Down