PP-4531: footnote VoiceOver labels never applied — namespaced-attribute selector - #1487
Closed
mauricecarrier7 wants to merge 1 commit into
Closed
mauricecarrier7 wants to merge 1 commit into
mauricecarrier7 wants to merge 1 commit into
Conversation
🏗️ CodeAtlas Ledger Analysis✅ All Checks Passed♿ Accessibility (via AccessLint)
✅ No accessibility issues detected 🏛️ Architecture Analysis
🔄 Dependency Cycles (2)2 distinct dependency cycles (largest spans 10 components).
Cycle 1 — 10 components, 16 edges between them
edgesCycle 2 — 3 components, 4 edges between them
edgesDiscounted 📋 Architecture Findings
🔍 Reachability Analysis
ℹ️ No architecture-relevant files in this diff — reachability not evaluated. 📊 0 files analyzed | 📦 Download Full Report Powered by CodeAtlas Ledger |
…ector (PP-4531)
PP-4531 shipped in build 480 and has labelled zero elements ever since. QA on
build 502 hears "1, link" where "Footnote 1, link" is expected.
`annotationJavaScript()` selected elements with
`querySelectorAll('[epub\:type],[role]')`. A CSS attribute selector with no
namespace component matches ONLY attributes in no namespace. Readium serves
spine resources as `application/xhtml+xml` (MediaType.swift:271), so WKWebView
parses them as XML and `xmlns:epub="http://www.idpf.org/2007/ops"` puts
`epub:type` in the OPS namespace — the selector matched nothing. Measured
against the real chapter QA read (moby-dick-preview-collection.epub
OPS/preface_001.xhtml), running the production script:
application/xhtml+xml (production) 0 labelled -> "1, link"
text/html 2 labelled -> "Footnote 1, link"
`getAttribute('epub:type')` matches by QUALIFIED name and does work, which is
why the Swift classifier looked correct: the elements never reached it.
Resolve the attribute namespace-agnostically — `getAttributeNS(OPS,'type')`
first, `getAttribute('epub:type')` as the HTML-parsed fallback — and stop
filtering selection on the namespaced attribute at all. Also concatenate
`epub:type` and `role` into one token list instead of `||`-chaining them, so an
element carrying an unrelated `epub:type` alongside `role="doc-noteref"` still
classifies.
Why no test caught it: the 16 existing tests cover the Swift classifier and
composer. The sole JS test substring-matched the script's own SOURCE
(`js.contains("noteref")`) and never executed it, so it could not fail on this.
The new `TPPReaderFootnoteAccessibilityDOMTests` runs the production script in a
real WKWebView against both parse modes and asserts the resulting aria-labels.
Confirmed RED before this fix (5 of 6 failed; XHTML noteref aria-label "" vs
"Footnote 1") and green after.
**Scope:** the DOM annotation path for footnotes only.
**Not done:** QA's "audio stops / does not resume" is NOT this defect and is not
addressed here — moving VoiceOver focus stops Speak All by OS design, and
swiping right speaking the next element is normal VoiceOver navigation.
**Deferred:** books with no `doc-backlink` still have no way back to the reading
position; there is no link history in the reader or in Readium's EPUB navigator
(goBackward is a directional page turn). Filed as PP-5150.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015xqE9uAiVMUc7kKHsXT17w
mauricecarrier7
force-pushed
the
fix/PP-4531-footnote-selector-namespace
branch
from
September 16, 2026 15:49
b6628a8 to
f37fb08
Compare
🧪 Unit Test Results📊 View Full Interactive Report ❌ 2 TESTS FAILED8731 tests | 8713 passed | 2 failed | 14 skipped | ⏱️ 12m 40s | 📊 99.8% | 📈 48.7% coverage All 994 classes — 2 with failures (full matrix)
📊 Full interactive matrix: report ❌ Failed testsNames only — open the interactive report above for messages + stack frames. 📊 Testing Coverage BreakdownUnit Test Line Coverage (testable surfaces): 48.7% Total coverage incl. UI/lifecycle: 47.4% (17 files excluded from testable denominator — see
📈 What changed vs. baseTest count: +200
|
| Artifact | Description |
|---|---|
| test-report | 📄 Markdown + HTML reports |
| test-data | 📊 JSON data for tooling |
| test-results | 🔍 Full xcresult (open in Xcode) |
Contributor
Author
|
Closing in favour of #1488, which merged the identical fix into The change reaches No work is lost: Re-open if |
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
1 participant
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.
What
PP-4531 (footnote VoiceOver labels) shipped in build 480 and has labelled zero elements in every book ever since. QA on build 502 hears "1, link" where "Footnote 1, link" is expected.
Root cause
TPPReaderFootnoteAccessibility.annotationJavaScript()selected elements with:A CSS attribute selector with no namespace component matches only attributes in no namespace. Readium serves spine resources as
application/xhtml+xml(MediaType.swift:271), so WKWebView parses them as XML andxmlns:epub="http://www.idpf.org/2007/ops"putsepub:typein the OPS namespace. The selector matched nothing.getAttribute('epub:type')matches by qualified name and does work — which is exactly why the Swift classifier looked correct and stayed green. The elements never reached it.Measured against the real chapter QA read (
moby-dick-preview-collection.epubOPS/preface_001.xhtml, which contains<a epub:type="noteref" href="#n1">1</a>), running the production script:application/xhtml+xml(production)text/htmlThe fix
epub:typenamespace-agnostically:getAttributeNS(OPS,'type')first,getAttribute('epub:type')as the HTML-parsed fallback.querySelectorAll('*'), which cannot silently match nothing.epub:typeandroleinto one token list instead of||-chaining. The old expression never consultedrolewhen an unrelatedepub:typewas present (e.g.epub:type="chapter" role="doc-noteref").Why no test caught it
The 16 existing tests cover the Swift classifier and label composer. The only test of the JavaScript asserted
js.contains("noteref")against the script's own source text — a gate that cannot fail on any DOM defect. The original intent file recorded the runtime leg as "Runtime (pending)" and it shipped on unit-only evidence.TPPReaderFootnoteAccessibilityDOMTests(new) executes the production script in a realWKWebViewagainst both parse modes and asserts the resultingaria-labels, including a role-only reference, the mixedepub:type+rolecase, a negative control, and idempotency. It also asserts a ground-truth count computed without the production selector, so a fixture that failed to parse cannot masquerade as the defect.Verification
aria-label""vs"Footnote 1", 2 of 5 elements labelled (exactly the tworole-based ones the old selector could still see). With the fix, 6/6.verify-pr.sh --quick --diff-baseline): 8731 tests, 8711 passed, 4 failed across 3 classes that all pass in isolation (AccountsManagerStateMachineWiringTests— the Renamed project, classes and properties #1 known flake at 16%,ParserFuzzTests,TestTargetHermeticityRegressionTests); none related to this diff. Zero timeout/restart lines. All 38 legs accounted for,coverage_floorspass.Scope
Footnote DOM annotation only.
TPPReaderBlockNavigationis unaffected — it selects on plain tag names and[role=…], neither of which is namespaced.Not done: QA also reported "the audio just stops / no audio resumes". That is not this defect — moving VoiceOver focus stops Speak All by OS design, and swiping right speaking the next element is normal VoiceOver navigation. No change made.
Deferred: books with no
doc-backlinkstill have no way back to the reading position. There is no link history in the reader or in Readium's EPUB navigator (goBackwardis a directional page turn). Of the two footnote-bearing titles in our test data neither has a backlink, and The Waste Land puts 50 endnotes ~31k characters past their references. Filed as PP-5150.🤖 Generated with Claude Code
https://claude.ai/code/session_015xqE9uAiVMUc7kKHsXT17w