fix(editor): persist page-setup changes on save (data-loss bug)

SCRATCHPAD.md

@@ -23,15 +23,15 @@ re-add properly inside the owning feature slice rather than restoring the dead p
 ## Page setup lives outside ProseMirror
 Page setup (size/orientation/margins) is a model-level doc attribute (`finalSectionProperties`, no
 PM representation), applied via `usePageSetupControls` → `handleDocumentChange`. Two consequences:
-- `Page-setup changes are dropped by selective save` **[high — data loss]**: the default save is
-  *selective* (`useFileIO` → `agent.toBuffer({ selective })`), and `attemptSelectiveSave`
-  (`docx/selectiveSave.ts`) only patches the `changedParaIds` it gets from the PM editor state and
-  never re-emits the body sectPr. A page-setup change is not a PM transaction, so `changedParaIds`
-  is empty and the original sectPr is kept → the margin/orientation/size change is NOT written to
-  the `.docx`. Only a full repack (`selective:false` → `repackDocx`, which serializes
-  `finalSectionProperties`, documentSerializer.ts:149-150) persists it. Confirmed by inspection +
-  user report. *Interim fix: force a full repack when section properties changed (mirror the
-  `hasInjectedReplies` force in `useFileIO`). Proper fix: make section properties PM-native.*
+- `Page-setup changes are dropped by selective save` **[FIXED — interim, 2026-07-01]**: the default
+  save is *selective* (`useFileIO` → `agent.toBuffer({ selective })`), and `attemptSelectiveSave`
+  only patches `changedParaIds` and never re-emits the body sectPr; a page-setup change isn't a PM
+  transaction so `changedParaIds` is empty → the margin/orientation/size change was NOT written to
+  the `.docx`. **Interim fix shipped:** `usePageSetupControls` flips a `sectionPropsDirtyRef`, which
+  `useFileIO` reads to force `structuralChange` → full repack (mirrors the `hasInjectedReplies`
+  force); regression test in `selectiveSave.test.ts`. **Proper fix still open:** make section
+  properties PM-native (a `doc` attribute → undoable transaction the change tracker sees), which
+  also resolves the undo-stack point below and lets selective save persist them without a full repack.
 - `Page setup not on the Ctrl+Z undo stack` **[low — working-as-intended]**: not revertible via
   Ctrl+Z (PM undo only covers content transactions) — treated as a doc attribute, not content
   (deliberate; the old global-keydown history that reverted it was the A1 bug, removed in slice 1).
@@ -96,7 +96,7 @@ one uniform `--docx-*` token namespace, single-source; the host `index.css` wire
 Deferred items surfaced during it:
 - `DEAD CSS SWEEP`: `styles/editor.css` `.paged-editor__decoration-overlay` + `.ProseMirror-yjs-cursor` reference the removed React PluginHost/`DecorationLayer` + y-prosemirror; and 5 dead `.docx-editor-vue__pages-viewport` scrollbar selectors (no Vue adapter). Verify no DOM emits these, then cut.
 - `DEAD-EXPORT SWEEP of docx-editor-core/src/utils`: `selectionHighlight.ts` turned out ENTIRELY dead (~578 lines, removed in #162); knip is exempt for the vendored editor, so other `utils/*` modules likely hide large unused surfaces. Worth a dedicated pass.
-- `OVERLAY ACCENT COLOUR` (design, not refactor): the selection/image overlay accents are Google-blue (`--docx-selection-rect` / `--docx-image-accent`), while text-selection + chrome are emerald (`--primary`-derived). Now tokens → aligning them to emerald is a one-line change. *Michael's call.*
+- (Overlay accent colour: DECIDED — keep the selection/image accents a distinct blue, not emerald; tokenised as `--docx-selection-rect` / `--docx-image-accent`, self-documented in tokens.css. No action.)
 
 ## Provenance / docs framing (one pass once the rework settles)
 

packages/docx-editor-core/src/docx/selectiveSave.test.ts

@@ -977,3 +977,48 @@ describe('attemptSelectiveSave — watermark header parts', () => {
     expect(result).toBeNull();
   });
 });
+
+// ============================================================================
+// Page setup (body section properties) in selective save
+// ============================================================================
+
+describe('attemptSelectiveSave — page setup / section properties', () => {
+  test('a page-setup change is dropped by selective save but persisted by a full repack', async () => {
+    const buffer = await loadFixture('example-with-image.docx');
+    const doc = await parseDocx(buffer, { preloadFonts: false });
+
+    const sectPr = doc.package.document.finalSectionProperties;
+    if (!sectPr || typeof sectPr.pageWidth !== 'number') {
+      console.log('Fixture has no numeric sectPr pageWidth, skipping');
+      return;
+    }
+    const originalWidth = sectPr.pageWidth;
+    const newWidth = originalWidth + 720; // a page-setup change (0.5in wider)
+    sectPr.pageWidth = newWidth;
+
+    // The bug: a page-setup change is not a paragraph edit, so a selective save
+    // (empty changedParaIds, no structural change) never re-emits the body sectPr
+    // — document.xml is untouched, so the change is silently dropped on reopen.
+    const selective = await attemptSelectiveSave(doc, buffer, {
+      changedParaIds: new Set(),
+      structuralChange: false,
+      hasUntrackedChanges: false,
+    });
+    expect(selective).not.toBeNull();
+    const afterSelective = await parseDocx(selective!, { preloadFonts: false });
+    expect(afterSelective.package.document.finalSectionProperties?.pageWidth).toBe(originalWidth);
+
+    // The fix: useFileIO forces structuralChange when page setup changed, so
+    // attemptSelectiveSave bails and the caller repacks — which serializes the
+    // updated sectPr, so the change survives the round-trip.
+    const bailed = await attemptSelectiveSave(doc, buffer, {
+      changedParaIds: new Set(),
+      structuralChange: true,
+      hasUntrackedChanges: false,
+    });
+    expect(bailed).toBeNull();
+    const repacked = await repackDocx(doc);
+    const afterRepack = await parseDocx(repacked, { preloadFonts: false });
+    expect(afterRepack.package.document.finalSectionProperties?.pageWidth).toBe(newWidth);
+  });
+});

packages/docx-editor-react/src/editor/docx-editor.tsx

@@ -226,6 +226,9 @@ export const DocxEditor = forwardRef<DocxEditorRef, DocxEditorProps>(function Do
   // Refs (pagedEditorRef is declared earlier — useCommentManagement needs it)
   const agentRef = useRef<DocumentAgent | null>(null);
   const containerRef = useRef<HTMLDivElement>(null);
+  // Set when page setup changes (model-level, not a PM transaction); read by the
+  // save path to force a full repack so the body sectPr actually persists.
+  const sectionPropsDirtyRef = useRef(false);
   // Save the last known selection for restoring after toolbar interactions
   const lastSelectionRef = useRef<{ from: number; to: number } | null>(null);
   const editorContentRef = useRef<HTMLDivElement>(null);
@@ -336,6 +339,7 @@ export const DocxEditor = forwardRef<DocxEditorRef, DocxEditorProps>(function Do
     loadBuffer,
     getActiveEditorView,
     focusActiveEditor,
+    sectionPropsDirtyRef,
   });
 
   // Mirror PM state on each external document load (mount-time view creation
@@ -566,6 +570,7 @@ export const DocxEditor = forwardRef<DocxEditorRef, DocxEditorProps>(function Do
     document: docState.state,
     readOnly,
     handleDocumentChange,
+    sectionPropsDirtyRef,
   });
 
   const {

packages/docx-editor-react/src/editor/fileOpen.test.tsx

@@ -33,6 +33,7 @@ function FileIOHarness({
   const agentRef = useRef<DocumentAgent | null>(null);
   const pagedEditorRef = useRef<PagedEditorRef | null>(null);
   const containerRef = useRef<HTMLDivElement | null>(null);
+  const sectionPropsDirtyRef = useRef(false);
   const fileIO = useFileIO({
     agentRef,
     pagedEditorRef,
@@ -42,6 +43,7 @@ function FileIOHarness({
     loadBuffer,
     getActiveEditorView: () => null,
     focusActiveEditor: () => {},
+    sectionPropsDirtyRef,
   });
 
   return (

packages/docx-editor-react/src/editor/lifecycle/use-file-io.ts

@@ -35,6 +35,7 @@ export function useFileIO({
   loadBuffer,
   getActiveEditorView,
   focusActiveEditor,
+  sectionPropsDirtyRef,
 }: {
   agentRef: React.RefObject<DocumentAgent | null>;
   pagedEditorRef: React.RefObject<PagedEditorRef | null>;
@@ -44,6 +45,8 @@ export function useFileIO({
   loadBuffer: (buffer: DocxInput) => Promise<void>;
   getActiveEditorView: () => EditorView | null | undefined;
   focusActiveEditor: () => void;
+  /** Set when page setup changed — forces a full repack so the body sectPr persists. */
+  sectionPropsDirtyRef: React.RefObject<boolean>;
 }) {
   const imageInputRef = useRef<HTMLInputElement>(null);
   const docxInputRef = useRef<HTMLInputElement>(null);
@@ -52,6 +55,12 @@ export function useFileIO({
     async (options?: { selective?: boolean }): Promise<ArrayBuffer | null> => {
       if (!agentRef.current) return null;
 
+      // Consume the page-setup dirty flag SYNCHRONOUSLY, before any await — a
+      // change applied during an in-flight save would otherwise be clobbered by
+      // a post-await reset (a set flag already forces this save to full-repack).
+      const sectionPropsChanged = sectionPropsDirtyRef.current;
+      sectionPropsDirtyRef.current = false;
+
       try {
         const agentDoc = agentRef.current.getDocument();
 
@@ -83,12 +92,17 @@ export function useFileIO({
           // Force full repack if any reply comments exist (both comment replies and
           // tracked-change replies need range markers injected into document.xml,
           // which selective save can't handle since the affected paragraphs may not
-          // be in changedParaIds)
+          // be in changedParaIds), or if page setup changed. A page-setup change
+          // isn't a PM transaction, so it's not in changedParaIds and selective
+          // save never re-emits the body sectPr — only a full repack persists it.
           const hasInjectedReplies = comments.some((c) => c.parentId != null);
           selectiveOptions = {
             selective: {
               changedParaIds: getChangedParagraphIds(editorState),
-              structuralChange: hasStructuralChanges(editorState) || hasInjectedReplies,
+              structuralChange:
+                hasStructuralChanges(editorState) ||
+                hasInjectedReplies ||
+                sectionPropsChanged,
               hasUntrackedChanges: hasUntrackedChanges(editorState),
             },
           };
@@ -103,10 +117,13 @@ export function useFileIO({
 
         return buffer;
       } catch {
+        // The save failed, so the sectPr change wasn't persisted — restore the
+        // dirty flag so the next save still forces a full repack.
+        if (sectionPropsChanged) sectionPropsDirtyRef.current = true;
         return null;
       }
     },
-    [agentRef, pagedEditorRef, comments]
+    [agentRef, pagedEditorRef, comments, sectionPropsDirtyRef]
   );
 
   const handleDirectPrint = useCallback(() => {

packages/docx-editor-react/src/features/page-setup/use-page-setup-controls.test.tsx

@@ -0,0 +1,54 @@
+import { GlobalRegistrator } from '@happy-dom/global-registrator';
+import { afterAll, beforeAll, describe, expect, test } from 'bun:test';
+
+beforeAll(() => GlobalRegistrator.register());
+afterAll(() => GlobalRegistrator.unregister());
+
+import { renderHook } from '@testing-library/react';
+import { useRef } from 'react';
+import type { Document } from '@eigenpal/docx-editor-core/types/document';
+import { usePageSetupControls } from './use-page-setup-controls';
+
+function makeDoc(): Document {
+  return {
+    package: { document: { finalSectionProperties: { pageWidth: 12240 } } },
+  } as unknown as Document;
+}
+
+function setup(readOnly: boolean) {
+  const changes: Document[] = [];
+  const { result } = renderHook(() => {
+    const sectionPropsDirtyRef = useRef(false);
+    const api = usePageSetupControls({
+      document: makeDoc(),
+      readOnly,
+      handleDocumentChange: (d) => changes.push(d),
+      sectionPropsDirtyRef,
+    });
+    return { api, sectionPropsDirtyRef };
+  });
+  return { result, changes };
+}
+
+describe('usePageSetupControls', () => {
+  test('applying page setup marks section props dirty (so the save forces a full repack)', () => {
+    const { result, changes } = setup(false);
+
+    result.current.api.handlePageSetupApply({ pageWidth: 15840 });
+
+    // The dirty flag is what useFileIO reads to force a full repack — without it
+    // the selective save silently drops the sectPr change.
+    expect(result.current.sectionPropsDirtyRef.current).toBe(true);
+    expect(changes).toHaveLength(1);
+    expect(changes[0].package.document.finalSectionProperties?.pageWidth).toBe(15840);
+  });
+
+  test('a read-only apply is a no-op and never dirties the flag', () => {
+    const { result, changes } = setup(true);
+
+    result.current.api.handlePageSetupApply({ pageWidth: 15840 });
+
+    expect(result.current.sectionPropsDirtyRef.current).toBe(false);
+    expect(changes).toHaveLength(0);
+  });
+});

packages/docx-editor-react/src/features/page-setup/use-page-setup-controls.ts

@@ -9,15 +9,22 @@ import type { Document, SectionProperties } from '@eigenpal/docx-editor-core/typ
  * not on the Ctrl+Z undo stack (`finalSectionProperties` is a model-level field
  * with no ProseMirror representation; PM's undo covers content transactions).
  * Re-open the dialog to change it back.
+ *
+ * A page-setup change is NOT a ProseMirror transaction, so the change tracker
+ * never sees it and a selective save (which patches only changed paragraphs)
+ * would drop it — the body `sectPr` is only re-emitted by a full repack. So we
+ * flip `sectionPropsDirtyRef`, which `useFileIO` reads to force a full repack.
  */
 export function usePageSetupControls({
   document,
   readOnly,
   handleDocumentChange,
+  sectionPropsDirtyRef,
 }: {
   document: Document | null;
   readOnly: boolean;
   handleDocumentChange: (doc: Document) => void;
+  sectionPropsDirtyRef: React.RefObject<boolean>;
 }) {
   const [showPageSetup, setShowPageSetup] = useState(false);
   const handleOpenPageSetup = useCallback(() => setShowPageSetup(true), []);
@@ -38,9 +45,10 @@ export function usePageSetupControls({
           },
         },
       };
+      sectionPropsDirtyRef.current = true;
       handleDocumentChange(newDoc);
     },
-    [document, readOnly, handleDocumentChange]
+    [document, readOnly, handleDocumentChange, sectionPropsDirtyRef]
   );
 
   return { showPageSetup, setShowPageSetup, handleOpenPageSetup, handlePageSetupApply };