Skip to content

Emit a note backlink only when its target is in the output - #678

Merged
JSv4 merged 4 commits into
mainfrom
fix/dangling-note-backrefs
Sep 2, 2026
Merged

Emit a note backlink only when its target is in the output#678
JSv4 merged 4 commits into
mainfrom
fix/dangling-note-backrefs

Conversation

@JSv4

@JSv4 JSv4 commented Sep 2, 2026

Copy link
Copy Markdown
Owner

The bug

WmlToHtmlConverter gave every footnote and endnote definition it rendered a return-arrow hyperlink back to the reference that cites it — including definitions that nothing cites.

TestFiles/NVCA-Model-COI.docx is the in-repo case: word/endnotes.xml carries a blank w:endnote with w:id="1", and no w:endnoteReference for it exists in any story. Word keeps orphans like this and the converter renders them, which is the right call for package fidelity. But the arrow it rendered alongside them pointed at #en-ref-1, an id the conversion never emitted. Standalone export then reported

fragment_target_unavailable: Fragment target #en-ref-1 is unavailable in the final page tree.

for markup the converter had authored itself. Under a strict unsupported-content policy that is not a warning, it is a failed export.

Why it happened

The two halves of a note's navigation come from different code paths reading different sources. The marker (#fn-N) is emitted from a w:footnoteReference in a rendered story; the return arrow (#fn-ref-N) is emitted from a w:footnote definition in footnotes.xml. Neither half checks that the other exists, and the relationship fails in both directions — an orphaned definition has no reference, and a reference can outlive its definition.

The fix

One post-pass, RemoveUnresolvableNoteLinks, alongside the two whole-tree passes the converter already runs after the transform.

It resolves against the finished tree, not against the numbering tracker. That matters. FootnoteNumberingTracker only scans the main document part, so a reference living in a header or footer never reaches it — a tracker-based check would have stripped a backlink whose target genuinely does exist. Asking "is this id in the output?" is the question the acceptance criterion actually states, and it is story-agnostic by construction.

The two directions are repaired differently, because they are not the same kind of thing:

  • A return arrow is navigation chrome. An unresolvable one is removed outright, along with the separator space rendered with it, so an orphaned note keeps no trailing gap.
  • A marker carries the note's visible number, which is document content. An unresolvable one keeps its text and loses only its dead href, becoming the inert label the issue's second option describes.

Only note links are touched (#fn-…, #en-…). Dangling cross-reference or bookmark links are a different question with a different right answer, and comment backlinks were already made target-aware in #563.

Validation

  • On the NVCA fixture, the conversion goes from one dangling fragment target (en-ref-1) to none, while all 94 cited footnotes keep working navigation in both directions.
  • Six new tests in WmlToHtmlConverterNoteLinkTests cover: the orphaned endnote still renders but without an arrow; the whole NVCA conversion has zero dangling fragment targets; cited footnotes keep both directions; a fixture carrying both an uncited footnote and an uncited endnote produces no dangling links; and the mirror case — a cited note whose definition was deleted — keeps its marker text but loses its href.
  • Non-vacuous: with the post-pass disabled, five of the six fail. The sixth asserts the fixture really does carry an uncited endnote, so the others cannot pass by testing nothing.
  • Full .NET suite: 3931 passed, 0 failed, 3 skipped.

Scope note

The issue's last two acceptance bullets concern @docxodus/export. This change removes the cause — the converter no longer authors a link to a target it did not emit — so the export warning cannot be raised for this document. The export path's own handling of a genuinely dangling link (strip the href, mark it aria-disabled, warn or fail per policy) is unchanged and still correct for links it did not author.

Closes #667

🤖 Generated with Claude Code

https://claude.ai/code/session_01VSnwkK1Nx6zZb2RnnoxKdx

JSv4 added 3 commits September 2, 2026 13:15
WmlToHtmlConverter rendered a return arrow for every footnote and endnote
definition it emitted, including definitions that no story cites. Word keeps such
orphans — TestFiles/NVCA-Model-COI.docx carries an uncited w:endnote with no
matching w:endnoteReference anywhere — and rendering them is right for package
fidelity, but the arrow's href pointed at an id nothing had emitted. Standalone
export then reported fragment_target_unavailable for markup the converter had
authored itself, which under a strict unsupported-content policy fails the export
rather than merely warning.

The two halves of a note's navigation are authored by different code paths over
different sources: the marker comes from a w:footnoteReference in a rendered
story, the return arrow from a w:footnote in footnotes.xml. Neither guarantees
the other exists, in either direction.

Resolves both halves against the finished tree rather than against the numbering
tracker. That asks the only question that matters — is the target id in the
output? — and stays correct for references in stories the tracker never scans,
such as headers and footers, where a tracker-based test would have removed a
backlink whose target does exist.

The directions are repaired differently. A return arrow is navigation chrome, so
an unresolvable one is removed outright, along with the separator space rendered
with it. A marker carries the note's visible number, which is document content,
so it keeps its text and only loses the dead href, becoming an inert label.

On the NVCA fixture this takes the conversion from one dangling fragment target
to none while leaving all 94 cited footnotes navigable in both directions. Five
of the six new tests fail without the fix; the sixth guards the fixture premise.

Closes #667
The sweep asks "is this note link's target id in the output?" That is a fair
question of a complete document and a wrong one of a fragment.

The editor's incremental renderer builds a shell whose body holds only the blocks
it asked for. A citing paragraph rendered on its own therefore has no notes
section to point at, and the sweep stripped the href from its citation marker on
every re-render. Downstream, reconcile hung the browser's main thread outright —
npm/tests/editor-reconcile.spec.ts "reconciled edits survive save/reopen
losslessly" timed out at insertFootnote, in CI and locally, on all retries.

Localized by rendering the same anchors straight through the bridge, which
returned in milliseconds and handed back the marker as
`<a id="fn-ref-1" class="footnote-ref"><sup>1</sup></a>` — no href. The render was
never the slow part; the missing href was.

Adds the internal WmlToHtmlConverterSettings.RendersDocumentFragment, set by
BuildBlockConverterSettings, the single owner of the block-render profile. It
sits next to SkipFormattingPartsSimplification, the existing knob for "this is
the incremental path, not the full one". Completeness checks over the finished
tree are skipped when it is set; nothing else changes.

The regression test renders a citing block through the incremental path and
asserts the marker keeps both its class and its href. Flipping the flag back to
false fails it.
Keeps both [Unreleased] entries; the test-project warning baseline moves to 691
for this branch's one new test file.
@JSv4

JSv4 commented Sep 2, 2026

Copy link
Copy Markdown
Owner Author

Follow-up: the first version of this fix hung the editor

CI caught it, and it was a real defect in my change, not a flake: npm/tests/editor-reconcile.spec.tsreconciled edits survive save/reopen losslessly — timed out at insertFootnote, on all three retries, and reproduced locally every time. main runs the same spec in 15s.

The mistake. The sweep asks "is this note link's target id in the output?" That is a fair question of a complete document and the wrong question of a fragment. The editor's incremental renderer builds a shell whose body holds only the blocks it asked for, so a citing paragraph rendered on its own has no notes section to point at — and the sweep stripped the href off its citation marker on every re-render.

How it was localized. Calling RenderBlocksHtmlForReview straight through the bridge with the same anchors returned in 6–33 ms and handed back the marker as <a id="fn-ref-1" class="footnote-ref"><sup>1</sup></a> — no href. So the render was never slow; the missing href was what sent the reconciler into a spin that blocked the browser's main thread outright (a waitForFunction for a plain variable could not run either). The equivalent .NET call had always returned in ~0.6 s, which is what made this look WASM-specific at first.

The fix. A new internal WmlToHtmlConverterSettings.RendersDocumentFragment, set by BuildBlockConverterSettings — the single owner of the block-render profile, and already home to SkipFormattingPartsSimplification, the existing "this is the incremental path, not the full one" knob. Completeness checks over the finished tree are skipped when it is set. Nothing else changes, and the full-document render — the one the reported export bug lives on — is unaffected.

Verification

  • editor-reconcile.spec.ts: 7 passed in 15.0s, matching main's timing.
  • Full .NET suite: 3932 passed, 0 failed, 3 skipped.
  • New regression test renders a citing block through the incremental path and asserts the marker keeps both its class and its href. Flipping RendersDocumentFragment back to false fails it, so it pins this bug specifically rather than the general sweep.

Keeps both [Unreleased] entries; the test-project warning baseline moves 691 → 692
for this branch's new test file, on top of the one #679 added.
@JSv4
JSv4 merged commit 727b644 into main Sep 2, 2026
14 checks passed
@JSv4
JSv4 deleted the fix/dangling-note-backrefs branch September 2, 2026 22:51
JSv4 pushed a commit that referenced this pull request Sep 2, 2026
#678 resolves footnote and endnote links against the finished HTML tree and
drops the ones whose target was never emitted. That lands on the demo's path:
Act II inserts a footnote, and the full+HTML depth renders the whole document,
which is exactly where the new sweep runs (BuildBlockConverterSettings opts the
incremental renderer out via RendersDocumentFragment; the theater does not).

Verified rather than assumed. All 14 browser assertions pass, including the two
that read both halves of the note — the citation wrapped in w:ins and the
definition content recording alongside it — and the reject-all check that the
side-letter text is gone once the redline is reversed. 61 node checks pass.

The sweep is two extra full Descendants() passes over the output tree, so it is
a fair question whether it shows up in the conversion stage. It does not.
Decomposing full minus redline across two pooled sessions puts that stage at
41-45 ms against 46 ms in the previous session -- flat, with oneCall steady at
43-46 ms confirming the redline row underneath it. Every absolute drifted down
together again, which is the machine and not the engine, so the published
figures stay where they are.
JSv4 added a commit that referenced this pull request Sep 2, 2026
npm/tests/tmp-measure492.spec.ts and tmp-verify492.spec.ts were throwaway probes
used to diagnose the Freedoom autopilot flake (#492). They reached main through a
`git add -A` on PR #678's merge commit and were never meant to be committed.

They are not harmless clutter: Playwright picks up everything under npm/tests, and
both drive the arcade under CDP CPU throttling with multi-minute waits — together
roughly 15 minutes added to every Playwright run, for coverage that duplicates
nothing and asserts nothing.

The diagnosis they were written for is preserved where it belongs: in #492's
frame-budget fix and its PR description.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Converter emits dangling backlinks for uncited endnote definitions

1 participant