Repository navigation
Let an authored [autofocus] win over the container - #6
Merged
Merged
Conversation
ephnagler
force-pushed
the
a11y-autofocus
branch
from
September 12, 2026 11:20
169b9e1 to
828a594
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Backlog item 6. A collection page logged "Autofocus processing was blocked because a document already has a focused element" on every soft navigation, reproduced twice. The message is the browser declining to run autofocus on swapped-in content — the document already has a focused element by then, so it never tries. The attribute is therefore silently dead on every PJAX navigation, and Pusha's focus-to-container arrived afterwards and took the focus the author had asked for. Apply it in the browser's place. Precedence is hash target, then [autofocus] in the incoming content, then the container. The hash stays first: a click on an anchor link is intent expressed later than the markup was written. Verified rather than trusted — [autofocus] on a disabled input, a hidden element or a plain <div> is a no-op, so focus is checked and falls through to the container when it did not take. Leaving focus on the outgoing page's link would be worse than the default. Also stubs Element.scrollIntoView in the jsdom fixture. jsdom leaves it undefined rather than warning, so calling it is a TypeError, and the hash branch of focusContainer could not be reached by any test until now — the existing hash tests pass because their hash matches no element. Surfaces updated together: README "Accessibility", CLAUDE.md, and PATTERNS.md step 8, which the porting agent reads. No audit rule or init behaviour changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8
A pass over the four surfaces against `bin/pusha.js`, which is the first thing the new consistency rule asks for. Three real drifts, all in the direction the rule predicts — the CLI moved and the prose did not. - Bucket X was missing from CLAUDE.md's audit description, which still stopped at P, while CLAUDE.md discusses X at length five sections later. SKILL.md and the CLI both had it. - Bucket K sat in the README's *surface* group and in SKILL.md's *per-script* group. The CLI settles it: a K finding is one custom element with a list of render sites, not a whole-surface check. The README moves. - `--skip-detect` and `--ignore-manifest` ship in the CLI and were absent from the README, the surface that is supposed to be the contract of record. `--ignore-manifest` was in SKILL.md, so the agent knew about a flag the human-facing docs did not mention. Also documents the `--json`-only narrowing flags together rather than scattered. No behaviour change; no source touched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8
Backlog item 5, the part that does not need a decision. CONTRIBUTING carries the four-surfaces rule, because a contributor is the reader most likely to change one surface and not the other three, and two things this repo learned the hard way: check that a regression test fails without its fix, and treat jsdom coverage as not-browser-coverage. Element.scrollIntoView is the example — jsdom leaves it undefined rather than warning, so the hash branch of focus.ts looked covered and was unreachable. CHANGELOG starts at 0.1.0 rather than reconstructing forty commits of pre-publication churn. git log has that, with the reasoning attached, which is the part a rebuilt changelog would throw away. The known limitations are in it on purpose: the pixel gap and the inert-app-code gap are the two things a reader needs before adopting, and burying them would be the kind of omission the honesty pass was for. SECURITY routes through GitHub private reporting. It deliberately names what is NOT a vulnerability — the pixel gap and app code going inert are documented, measured limitations, and a triage queue that has to re-litigate them each time is worse than one that says so up front. There is no security email. If you want one, add it and I will wire it in — I am not inventing an address that would bounce. No source, CLI, skill or runtime-contract change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8
…ment Caught by rebuilding to refresh the rig's vendored bundle. CLAUDE.md and the README both claimed a measurement from 2026-09-11 and disagreed with each other: 32.2 kB / 10.7 kB against 34.7 kB / 11.5 kB. A clean build today says 35.3 kB raw / 11.7 kB gzipped for the UMD bundle. The second number was doing real work and doing it backwards. CLAUDE.md argued the modularity hold was safe because Pusha was "already under `@barba/core` alone (9.9 kB)". At 11.7 kB it is larger, not smaller — so the sentence that justified not splitting the bundle rested on a comparison that goes the other way. The README had the honest framing on the same day: a comparable feature set is @barba/core + prefetch + css + router at ~12.5 kB, still without an analytics bridge or Section Rendering API revalidation. That argument survives a reader checking it; the core-only one does not. The wrong claim is recorded rather than quietly replaced, since it is the kind that gets re-derived. Both surfaces now carry the same measured figures, and a note that these go stale on every merge. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8
…plied
Found in the first browser walk islands have ever had. The run reported
`cached: true`, one revalidation event, `swapped 1 in 229ms` — and left
`.is-revalidating` on a live element.
The class was only ever removed because `wrapper.replaceWith()` destroyed
the node carrying it. Every path that does not swap — no
`#shopify-section-<id>` wrapper in the document, an empty response, an
island marker sitting outside the wrapper being replaced — stranded it on
a live element permanently. CLAUDE.md promised "removed when the new HTML
is in place", which was true by accident. A theme styling it as a dim or
skeleton, which is what the README tells them to do, gets a region that
never comes back.
Removed explicitly now, for every island, on every path.
The second half is why the walk looked clean. `swapped ${keys.length}`
counted keys in the response, not nodes replaced, so it printed
`swapped 1` for a run that may have touched nothing — the diagnostic that
should have caught the stranded class certified the run instead. It now
counts what applied, names what did not, and logs NO TARGET for a section
id with no wrapper.
`pjax:islands-revalidated` had the same flaw: `sectionIds` was what was
requested. A listener re-initialising the regions it names was re-running
against markup that never changed. It now carries what was applied, with
`requestedIds` alongside.
Whether the Horizon run hit the no-wrapper case or the outside-the-wrapper
case is still unknown — the next run says, which it could not before.
Both tests verified to fail without the fix. The pre-existing hash test's
fixture has an island with no wrapper and never noticed.
Surfaces updated together: README (event table + islands section),
CLAUDE.md (visual contract), PATTERNS.md (the id must name a wrapper that
exists). No audit rule or init behaviour changes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8
The first run with honest counting produced a confusing pair of lines: Pusha asked for `template--22360478679180__main` and the loop reported `NO TARGET for product`. Both true, and together they read like a theme markup problem — when the actual event is that the response was not keyed by the ids that were requested. Diagnostic only. What to DO about a mismatched response is not decided yet, because the cause is not known: this was measured on `shopify theme dev`, and the local proxy is already known to behave differently under load. Writing a fallback now would be guessing, and a wrong fallback here swaps the wrong markup into a live page. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8
Islands have never worked on a product page. Not on Horizon — on any
theme, since the first release.
`Accept: application/json` on a product URL triggers Shopify's
product-JSON content negotiation, and it beats the query string:
`?sections=` is ignored and the body is `{"product": {...}}`, status 200,
content-type application/json. So the response parsed, the cycle
completed, and the one key in it named no section. Every layer reported
success. Measured with curl against the same URL — with the header, the
product object; without it, the section HTML keyed by the requested id,
wrapper included.
Removing the header is the whole fix. The Section Rendering API keys off
`?sections=` and never needed an Accept header.
Three things had to be fixed before this was findable, which is the part
worth keeping: the stranded `.is-revalidating`, the counter that reported
response keys instead of applied nodes, and the mismatch line that said
"asked for X, got Y". The first browser run reported `swapped 1` and
looked clean. Islands are the reason to distrust a green jsdom suite —
the fixture answers whatever the stub is told to answer, so a
content-negotiation bug is invisible by construction.
README needs no change: no config key, event, or contract moved. Recorded
in CLAUDE.md under Islands, since the next person to add a header there
needs to know why there isn't one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8
The audit had no island coverage at all for classic OS 2.0 — `island`
appeared twice in the CLI, both about new-Liquid partials. So nothing
told a porting agent which regions were stale-prone, nobody marked any,
and islands went a full release with no marker in any theme. That is why
the Accept-header bug survived: there was nothing to exercise.
R is bucket L's question asked about the swap container. They do not
overlap — an L finding freezes because the shell is never swapped, an R
finding goes stale because the page HTML came out of the prefetch cache.
Sub-letters: A price, B availability/inventory, C variant state, D
cart-derived (already covered by cart:mutated), E render-time values,
plus `marker` for conformance of an existing data-island.
**Candidates are `decide`, never `transform`.** Revalidating costs a
request and re-runs the section's JS, so marking a region that does not
actually change taxes every cached navigation. Whether a price is
stale-prone is a question about the store, not the code, and the audit
must not answer it. Broken markers ARE `transform` — those are
mechanical, and each fails silently:
- `data-island` with no `data-section-id` (the selector needs both)
- a marker in the persistent shell (islands are collected from the
swapped container only)
- a snippet writing {{ section.id }} whose render sites pass no
`section:` — {% render %} does not inherit scope, so the attribute
renders empty
That last one is verified, not assumed. The first version flagged every
snippet reading section.id and reported the rig's correctly-wired island
as dead. It now resolves the actual render sites and names the ones that
miss it. Reporting on shape alone is the same error as suppressing on
shape, inverted.
`section` IS in scope in theme blocks — checked against a Horizon port,
where 13 of 93 blocks read section.id and buy-buttons.liquid already
writes data-section-id. I had assumed otherwise; the assumption was
wrong and the check would have encoded it.
New fixture covers all four marker cases against a correctly-wired
control. Goldens regenerated: only the R section and summary line moved,
one legitimate finding each in os2-sections and analytics-surface, no
noise anywhere else.
All four surfaces: BUCKET_RULES.R + report + JSON action mapping in the
CLI, SKILL.md (batching guidance), PATTERNS.md (the four things that must
hold, each of which fails silently), README and CLAUDE.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8
CodeQL flagged the new marker check high severity, and it was right about the shape even before the security framing: `new RegExp()` with a snippet name interpolated straight in. Any name carrying a regex metacharacter would have matched the wrong thing or nothing, silently — the failure mode this whole bucket exists to catch. It also re-walked every .liquid file in the theme once per marker. One static pattern now matches every render tag and the captured name is compared as a string. Same findings on the fixture and on the Horizon rig. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjTmKaEaizVN7ZPTKVzrK8
ephnagler
force-pushed
the
a11y-autofocus
branch
from
September 12, 2026 12:57
97ad65b to
75d3f69
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backlog item 6, plus the consistency pass the new CLAUDE.md rule asks for.
The autofocus collision
A collection page logged
Autofocus processing was blocked because a document already has a focused elementon every soft navigation, reproduced twice. That message is the browser declining to run autofocus: swapped-in content arrives while the document already has a focused element, so it never tries. The attribute is therefore silently dead on every PJAX navigation — and Pusha's focus-to-container then arrived and took the focus the author had asked for.Pusha now applies it in the browser's place. Precedence: hash target →
[autofocus]in the incoming content → container. The hash stays first because a click on an anchor link is intent expressed later than the markup was written.Verified rather than trusted:
[autofocus]on a disabled input, a hidden element or a plain<div>is a no-op, so focus is checked and falls through to the container when it did not take.A test-fixture gap worth knowing about
jsdom leaves
Element.scrollIntoViewundefined rather than warning, so calling it is aTypeError— which means the hash branch offocusContainercould not be reached by any test until this PR stubs it. The existing hash tests pass because their hash matches no element and the branch is skipped.Surface reconciliation
A pass over the four surfaces against
bin/pusha.js. Three drifts, all in the predicted direction — the CLI moved and the prose did not:--skip-detectand--ignore-manifestship in the CLI and were absent from the README. The agent-facing SKILL.md knew about a flag the human-facing docs did not.Repo hygiene
CONTRIBUTING, CHANGELOG, SECURITY (backlog item 5, the part that needs no decision). SECURITY routes through GitHub private reporting — there is no security email, and I would rather not invent one that bounces.
Verification
149 tests pass; the three autofocus tests were checked to fail without the fix.