Commit graph

1 commit

Author SHA1 Message Date
f04f8ba34e feat(pdf): implement annotation editing (Phase 8)
Some checks failed
PDF engine / engine (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / fuzz (push) Has been cancelled
Phase 8 feature 2 of 10, designed in
REVIEWS/adr/0004-pdf-annotation-editing.md. Its prerequisites are "document
model, appearance generation, incremental save"; the first two landed in
Phase 3 and the third in ADR 0003, so this was the ready one.

Chosen ahead of the other unblocked feature, AcroForm full support, because
the review lists that one as needing JavaScript actions. Running
document-supplied code is a large new dependency and a security surface that
deserves its own ADR and threat review rather than arriving as a side effect
of finishing a form feature.

Annotations were strictly read-only: the module had public fields and
from_dict, and not one mutator or &mut self method. The viewer could report
a click on a link but could not move a highlight, restyle a square or delete
a stamp.

New pdf-document/src/annotation_edit.rs, deliberately the same shape as
DocumentFormEditor so a caller wiring a drag gesture does not have to learn
a second contract:

- AnnotationEdit covers Move, Resize, SetColor, SetInteriorColor,
  SetBorderWidth, SetOpacity, SetContents, SetFlags and Delete.
- Every edit is validated before anything changes, so a rejected edit leaves
  the annotation untouched. Degenerate and non-finite rectangles, colours
  outside 0..1, negative border widths and out-of-range opacities are all
  refused with typed errors.
- A degenerate rectangle is refused rather than silently normalised: it
  usually means a bug in the UI upstream, and quietly fixing it hides that.
  An inverted but valid rectangle is normalised on store, so hit testing and
  appearance sizing never see one upside down.
- Read-only annotations refuse edits unless the caller opts in through an
  explicit allowing_read_only(), with one exception: clearing the read-only
  flag itself is permitted, or a locked annotation could never be unlocked.
- Colour reading converts the grey and CMYK forms of /C to RGB, since the
  array length selects the space.

Identity: PdfAnnotation gains an obj_ref, because an index into /Annots is
not stable across a save. Populating it exposed a real bug in
page_annotations: it called self.resolve() on the /Annots array, which
recurses and replaced every entry with its dictionary, destroying the
references. It now resolves only the array itself.

Saving: save_annotation_edits appends a revision through the ADR 0003
writer. Deleting rewrites the page dictionary so the reference leaves
/Annots, because an object that stops existing while the page still points
at it produces a file other readers reject.

Out of scope and recorded in the ADR rather than implied: creating new
annotations, appearance generation for types this crate cannot draw (a
Stamp keeps its existing /AP rather than being blanked), applying
redactions, and rich text.

Tests: 23 unit tests plus 13 corpus acceptance tests covering the ADR merge
criteria. The round trips reparse from the written bytes rather than reusing
in-memory state, assert a deleted annotation is gone from the reparsed
page's /Annots and not merely from the model, that unrelated annotations
survive a deletion, that two saves chain, and that editing then saving never
panics on the malformed corpus.

Validation:
  TEST_TARGET=pdf ./tools/test-rust-clean.sh      (354 tests)
  TEST_TARGET=pdf-ui ./tools/test-rust-clean.sh   (398 tests, 6 ignored)
Both rustfmt and clippy -D warnings clean.
2026-07-28 17:12:40 +00:00