fix(converter): preserve numbering.xml definitions on round-trip (SD-2911)#3231
Open
tupizz wants to merge 1 commit into
Open
fix(converter): preserve numbering.xml definitions on round-trip (SD-2911)#3231tupizz wants to merge 1 commit into
tupizz wants to merge 1 commit into
Conversation
…2911) #exportNumberingFile filtered out every w:num whose numId wasn't referenced from the exported document parts (including headers, footers, and footnotes). Word's tentative numbering, where a document carries definitions for lists the user hasn't applied yet, was therefore silently wiped: the active-numbering fixture lost 7 of 8 definitions and the tentative fixture lost them all. The filter was introduced for the lists.delete document-api operation, but it couldn't distinguish "user just deleted a list in this session" from "definition was always unused in the source file" — both arrived at the export with no referencing paragraph and both were dropped. Word tolerates unused definitions, so the safe default on export is to emit every abstractNum and num the importer captured. The strip-orphaned helper is removed entirely; the inline write in #exportNumberingFile is now four lines. Verified: both fixtures round-trip byte-identical (after pretty-print) at the numbering.xml level. New integration test covers both the active and tentative variants.
superdoc-bot
Bot
added
review: thorough
review: careful
and removed
review: thorough
labels
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
2 participants
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.
Summary
#exportNumberingFileno longer filtersnumbering.xmlby whichnumIds are referenced from the document body. Every<w:abstractNum>and<w:num>the importer captured is emitted on export.strip-orphaned-numbering.js+ its unit tests; the inline write is now four lines.Why
Customer fixture in SD-2911:
numIdused in the body):<w:abstractNum>dropped from 8 → 1,<w:num>from 8 → 1. Live numbering definitions Word would otherwise expose in the Multilevel List dropdown were silently lost.numIdreferenced):numbering.xmlwas effectively wiped (2 → 0 abstractNum, 1 → 0 num).The filter was added by PR #2223 for the
lists.deletedocument-api operation — when a user deletes a list, the dead definition should be cleaned up. But the filter ran on every export and had no way to tell "user just deleted in this session" from "definition was always unused in the source file" (Word's tentative numbering). Both reached it with no referencing paragraph and were treated identically. Word tolerates unused definitions, so the safe default on round-trip is to preserve everything.Test plan
sd-2911-numbering-roundtrip.test.js) round-trips both customer fixtures and asserts<w:abstractNum>/<w:num>counts and ids match the input exactly.pnpm testforsuper-editor(12 695 / 12 708 pass — the 13 deletions are the removedstrip-orphaned-numbering.test.jsunit tests).numbering.xmlbyte-identical to input after pretty-print (8 / 8 preserved).numbering.xmlbyte-identical (2 / 1 preserved).Verified diffs on the customer fixtures
<w:abstractNum><w:num><w:abstractNum><w:num>What about the lists.delete flow that originally motivated the filter?
If
lists.deleteneeds to remove the definition for the list it deletes, that should happen at the operation site — where the deletion intent is known — not as an opportunistic GC pass at every export. There's no existing test in the repo that gates this behavior end-to-end against the document-api; adding one is out of scope for this fix.