Skip to content

Render tracked changes and comments - #187

Open
ikashapov wants to merge 4 commits into
nerdy-pro:mainfrom
ikashapov:issue-154-tracked-changes
Open

ikashapov wants to merge 4 commits into
nerdy-pro:mainfrom
ikashapov:issue-154-tracked-changes

Conversation

@ikashapov

Copy link
Copy Markdown
Contributor

Render tracked changes and comments

Closes #154. Three commits, each one revision-layer capability:

1. w:strike / w:dstrike actually draw (b78f729)

§17.3.2.37/§17.3.2.9 were parsed and style-cascaded, then silently dropped at
the RunProperties→FontProps seam — struck text painted plain (sample1's own
body literally says "underlined and struck out text", rendered un-struck).
The underline path was the template: strike metrics come from Skia's
strikeout values (positive above the baseline, 0.3×ascent fallback), filled
by the same decoration pass, emitted as DrawCommand::Line — two straddling
lines for dstrike — in the run's color. Zero painter changes: Line and
Underline already share an arm. Groundwork for the deletion marks below.

2. Tracked-change marks per the document's own view (afca99f)

The parse kept a hard-coded "accept all changes" view: <w:ins> flattened to
plain text, <w:del> dropped with its content. Both wrappers now flatten
with their runs stamped (TextRun.revision — kind + author), and the
display decision moves to the renderer, keyed on w:settings/w:revisionView:

  • Markup view (the default): deletions struck through, insertions
    underlined, each in a per-author color assigned by first appearance in
    document order (resolve::revision — the palette is engine-defined; Word's
    is user-configurable).
  • Final view (revisionView hides insDel): deleted runs filtered out
    beside the w:vanish filter; insertions plain.

Deleted pictures cannot carry a run stamp and stay dropped in every view —
exactly the pre-change behavior, now scoped to what a run cannot express.
Nested wrappers shadow (inserted-then-deleted is a deletion); moveFrom
stays dropped so moved text isn't duplicated, moveTo renders plain.

3. Comments: range wash, margin balloons, connectors (650daf1)

word/comments.xml (§11.3.1) parses through the notes-part machinery with
author/initials kept. Three things reach the page when the view shows
comments:

  • Range wash — commentRangeStart/End (paragraph and block level)
    toggle a parse-time stamp on the runs between them, so a range survives
    paragraph boundaries; stamped runs get a pale wash. Word's print view
    brackets the range instead — the wash is a documented engine choice.
  • Anchors — each w:commentReference becomes a zero-width fragment
    (the bookmark pattern) resolved to a page position at line emission.
  • Balloons — a post-layout pass (shaped like the header/footer second
    pass) consumes each page's anchors and draws into the right margin band:
    author label and body in the author's palette color (comment authors join
    the tracked-change palette, so one person keeps one color across both),
    border, background, connector line from the anchor, later balloons
    stacking below earlier ones. A margin too narrow for readable text (< 24pt
    of band) draws nothing, with a warning — Word widens the sheet for its
    markup area, which a fixed page cannot.

w:revisionView w:comments="0" suppresses all three.

Adversarial review round

A four-dimension adversarial review (parse-state leaks, view semantics,
balloon geometry, docs/test honesty) confirmed 21 findings; all are fixed in
the review commit (d595617):

  • w:delInstrText — Word's spelling of a deleted complex field — was a hard
    parse error failing the whole document; it now parses.
  • The deleted-content filter is an explicit retain-list: a deleted symbol,
    note reference, dynamic field, or mc:AlternateContent-wrapped shape can
    no longer leak into either view (a deleted DATE field used to re-evaluate
    and paint a fresh value in the final view).
  • Deletion is sticky across nesting (<w:ins> inside <w:del> stays
    deleted); a wholly deleted paragraph (§17.13.5.15 mark deletion, now
    modelled) merges away in the final view instead of leaving a blank line.
  • Comment ranges: table- and row-level markers now toggle the stamp in
    document order; a commentReference closes its own range (the repair for
    a lost end marker); orphaned range ids wash nothing; notes get a fresh
    conversion context each so an unbalanced marker cannot leak across notes.
  • Balloons: one balloon per comment document-wide (repeated w:tblHeader
    rows no longer duplicate it), the endnote page gets its own balloon pass,
    page-bottom overflow and tables-in-comments are warned about.
  • Docs: the README Strikethrough row flips to ✅ (77 ✅ / 11 ⚠️ / 10 ❌), the
    per-author-colors and row-deletion scope lines are stated, and three stale
    schema/module doc claims are corrected.

Verification

  • Full battery green: fmt, clippy -D warnings, 46 test suites, doc,
    --no-default-features.
  • Corpus pixel-diff against the base commit shows exactly three diffs,
    each explained
    : the comment-reference.docx comment now renders (the
    feature), and the struck runs of sample1 / numbering-direct-indent
    now paint struck (the strike fix).
  • New fixtures, all generator-built, OPC-verified, and paired with a
    view-flipped twin: tracked-changes.docx / -final.docx (the strike
    paragraph in both is the discriminator between a revision mark and strike
    formatting), comments.docx / comments-hidden.docx (two authors, a
    multi-paragraph balloon, a cross-paragraph range). The Word-authored
    corpus fixture comment-reference.docx (Cyrillic author, narrow margin,
    modern-comments sibling parts) is exercised end-to-end.
  • Mutation checks on the strike emission and metrics sign, the revision
    overlay and final-view suppression, the wash, the range stamp, and the
    balloon pass — every deliberate break was caught by exactly the tests
    written for it.
  • Rendering verified visually against LibreOffice for both fixture families.

README coverage rows flip to ✅ (77 ✅ / 11 ⚠️ / 10 ❌; FAQ gap list drops
"tracked changes and comments"); AGENTS.md corpus table documents the new
fixtures and the now-rendering comment fixture.

@ikashapov
ikashapov force-pushed the issue-154-tracked-changes branch 6 times, most recently from 34d1a89 to b2c7147 Compare September 12, 2026 09:48
@ikashapov
ikashapov force-pushed the issue-154-tracked-changes branch 2 times, most recently from f4938ba to ec2dea6 Compare September 18, 2026 02:49
§17.3.2.37/§17.3.2.9 were modelled and style-resolved but discarded at the
RunProperties→FontProps seam, so struck text painted as plain text. The
underline path is the template: FontProps gains strike_lines (0/1/2 — the
tri-state collapses like underline's bool) plus metrics filled by the same
decoration pass from Skia's strikeout values (positive above the baseline;
0.3×ascent fallback), and line_emit draws one DrawCommand::Line through the
run — two straddling lines a thickness apart for dstrike — in the run's
color. The painter already draws Line and Underline identically, so no
painter change.

Groundwork for nerdy-pro#154: a deleted run's markup view is struck text.
Half of nerdy-pro#154. The parse kept a hard-coded "accept all changes" view:
<w:ins> content flattened as plain text, <w:del> content dropped. Both
wrappers now flatten with their runs stamped (TextRun.revision — kind and
author), keeping unaccepted deletions in the model, and the display
decision moves to the renderer, keyed on w:settings/w:revisionView:

- Markup view (the default when the element is absent): deletions paint
  struck through and insertions underlined, each in a per-author color
  assigned by first appearance in document order (resolve::revision —
  the palette is this engine's own; Word's is user-configurable).
- Final view (revisionView hides insDel): deleted runs are filtered out
  beside the w:vanish filter, insertions paint plain.

Deleted pictures cannot carry a run stamp and stay dropped in every view,
exactly as all deleted content was before. Nested wrappers shadow: text
inserted then deleted carries the deletion. <w:moveFrom> stays dropped
(rendering both copies would duplicate the moved text) and <w:moveTo>
stays plain — move marks are their own display family.

Fixtures: tracked-changes.docx / tracked-changes-final.docx share one
body and differ only in revisionView; the strike-formatting paragraph in
both is what tells a revision mark from ordinary §17.3.2.37 formatting.
Completes nerdy-pro#154. The comments part (§11.3.1) now parses — located by the
already-modelled Comments relationship, converted through the ordinary
body machinery like the notes parts, with author and initials kept — and
three things reach the page when the document's w:revisionView shows
comments (the default):

- The commented range gets a pale wash: commentRangeStart/End markers,
  at paragraph and block level, toggle a parse-time stamp on the runs
  between them (the same shape as the revision stamp, and the reason a
  range can span paragraphs). Word's print view brackets the range
  instead; the wash is this engine's choice, recorded at the site.
- Each w:commentReference becomes a zero-width anchor fragment (the
  bookmark pattern), resolved to a page position by line emission.
- A balloon pass — shaped like the header/footer second pass — consumes
  the anchors per page and draws the balloon into the right margin band:
  author label and body in the author's palette color (comment authors
  join the tracked-change palette, so an author keeps one color across
  both), background wash, border, connector line from the anchor, later
  balloons stacking below earlier ones. A margin too narrow for
  readable text draws nothing, with a warning: Word widens the sheet
  for its markup area, which a fixed page cannot.

w:revisionView w:comments="0" (or w:markup="0") suppresses all three.

The corpus pixel-diff shows exactly three explained changes: the
comment-reference.docx comment now renders (this feature), and the
struck runs of sample1 and numbering-direct-indent now paint struck
(the strike fix two commits back — sample1's own text reads
"underlined and struck out text").
Twenty-one confirmed findings across four review dimensions, each fixed
with a spec-first test or a documented scope line:

Parse. w:delInstrText — Word's spelling of a deleted complex field's code
— was a hard parse error failing whole documents; it now parses. The
deleted-content filter became an explicit retain-list with no catch-all:
a deleted symbol, note reference, dynamic field, or AlternateContent-
wrapped shape is dropped in both views (a deleted DATE field used to
re-evaluate and paint a fresh value in the final view; a deleted footnote
laid its whole body out at the page bottom). Deletion is sticky across
nesting: <w:ins> inside <w:del> stays deleted. The §17.13.5.15 paragraph-
mark deletion is modelled, and the final view merges a wholly deleted
paragraph away instead of leaving a phantom blank line.

Comments. Table- and row-level commentRangeStart/End now toggle the run
stamp in document order (the collectors carry markers instead of dropping
them); a commentReference closes its own range, so a document that lost
its end marker cannot wash everything after it; stamps whose id has no
comment behind them are cleared at resolve; each note converts in a fresh
context so an unbalanced marker cannot leak across notes.

Balloons. One balloon per comment document-wide — §17.4.49 repeated
header rows replay their anchor on every continuation page and only the
first draws; the endnote page, born after the per-section passes, gets
its own pass; stacking past the page bottom and tables in comment bodies
are warned about rather than silent.

Docs. The README Strikethrough row flips to ✅ (77/11/10), the comments
row states the palette and row-deletion scope, and three stale claims
(schema final-view wording, comments-not-rendered, an invented catch-all)
are corrected.
@ikashapov
ikashapov force-pushed the issue-154-tracked-changes branch from ec2dea6 to 6982b34 Compare September 22, 2026 09:39
ikashapov added a commit to ikashapov/dxpdf that referenced this pull request Oct 14, 2026
One squashed commit carrying the tree of integration/2026-10-14, which was
previously a chain of ten merges ending at ac5787e (kept locally as
archive/integration-2026-10-14-merges). Base: upstream main 3a5c7ca.

Integrated, each at the head of its open upstream pull request:

- nerdy-pro#252  fix-layout-defects-228-232 (a40d5ce) — issues nerdy-pro#228–nerdy-pro#232: bookmark-only
        paragraphs, nested-table width, list instances over one abstract
        numbering, table width resolution (pct and tcW autofit), column
        balancing and separators
- nerdy-pro#183  issue-150-wmf-svg (6e5aa99) — WMF/EMF and SVG images
- nerdy-pro#184  issue-151-pgborders-docgrid (27204b3) — page borders, document grid
- nerdy-pro#186  issue-153-shaped-clusters (65739f6) — shaped text clusters
- nerdy-pro#187  issue-154-tracked-changes (6982b34) — tracked changes and comments
- nerdy-pro#185  issue-155-smartart-charts (3683223) — SmartArt and charts
- nerdy-pro#256  issue-253-del-instr-text (0656a52) — w:delInstrText tolerated
- nerdy-pro#257  issue-250-image-hyperlinks (cb5ce2b) — a:hlinkClick on pictures
- nerdy-pro#258  issue-188-font-absent-cost (9a63bf0) — no deep catalogue tier before
        the substitution table, no theme script-font preload
- nerdy-pro#259  issue-158-auto-hyphenation (9bfbae2) — w:autoHyphenation

Beyond the union of those branches, the integration itself changed:
- README keeps one row per feature after the merges; the matrix tally is
  recomputed from the rows.
- nerdy-pro#253's tests assert that a deleted field code is never drawn and never run
  text, because nerdy-pro#154 keeps w:del runs instead of dropping them at parse.
- nerdy-pro#250's push_link_over sits after nerdy-pro#151's resolve_line_metrics; nerdy-pro#158's
  HyphenationCtx.measure and rejoin_hyphenated take &dyn SplitMeasure (nerdy-pro#153).

Verified on this exact tree: cargo fmt, clippy and doc clean; cargo test
2977 passed / 0 failed (skipping the capi library freshness guards);
--no-default-features and --features capi build; the twelve-document
harness passes 12/12.

This branch has not been deployed

No deployments
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.

Render tracked changes and comments

1 participant