diff --git a/CHANGELOG.md b/CHANGELOG.md index 197c6407e..4ca31e12c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,9 @@ The release run heads these entries with the version and opens a fresh - The rendered pdf view exposes `odr.annotation`: the five tools, live preview and undo, whose `getAnnotations()` produces exactly what `annotate` takes. +- `PdfFile::is_annotatable` answers for the file what the `annotate` capability + answers for the format, and narrows it. Encrypted and repaired pdfs say no. + - **Breaking**: `html::edit` becomes `Document::edit`, in every binding — java's `Html.edit(document, diff)` becomes `document.edit(diff)`, and so on. `Text::set_content` is unchanged. diff --git a/apple/include/OdrCoreObjC/ODRFile.h b/apple/include/OdrCoreObjC/ODRFile.h index 6e3c48989..8639f5b06 100644 --- a/apple/include/OdrCoreObjC/ODRFile.h +++ b/apple/include/OdrCoreObjC/ODRFile.h @@ -405,6 +405,10 @@ NS_SWIFT_NAME(DocumentFile) /// A decoded PDF — `odr::PdfFile`. NS_SWIFT_NAME(PdfFile) @interface ODRPdfFile : ODRDecodedFile +/// Whether this file can take annotations — the counterpart of +/// `ODRDocument.isEditable`. `NO` for a pdf declaring an `/Encrypt`, or whose +/// cross-reference table had to be rebuilt by scanning. +@property(nonatomic, readonly) BOOL isAnnotatable; /// Applies markup annotations — the payload the rendered page's /// `odr.annotation.getAnnotations()` collects — and returns the annotated pdf. - (nullable NSData *)annotate:(NSString *)annotations diff --git a/apple/src/ODRFile.mm b/apple/src/ODRFile.mm index 497d4fdbe..85b33b045 100644 --- a/apple/src/ODRFile.mm +++ b/apple/src/ODRFile.mm @@ -649,6 +649,10 @@ - (nullable ODRDocument *)documentWithError:(NSError **)error { @implementation ODRPdfFile +- (BOOL)isAnnotatable { + return self.handle.as_pdf_file().is_annotatable() ? YES : NO; +} + - (nullable NSData *)annotate:(NSString *)annotations error:(NSError **)error { return guarded(error, [&]() -> NSData * { std::ostringstream out; diff --git a/docs/design/pdf-annotation.md b/docs/design/pdf-annotation.md index c482bde3a..af845bae1 100644 --- a/docs/design/pdf-annotation.md +++ b/docs/design/pdf-annotation.md @@ -1,10 +1,8 @@ # PDF annotation design -Status: **landed.** This records the architecture for adding markup -annotations — text highlight and freehand drawing first — to an existing PDF, -the alternatives weighed, and the effort it costs. The format model is -validated against four viewers, and every phase has landed: the browser draws -the markup, the writer appends it, and every binding can apply it. +Status: **landed** (#843–#850). This records why the markup annotation feature +is built the way it is — the decisions, and the alternatives they beat — for +whoever changes it next. It is a record, not a plan. Scope is **markup only**: draw on top of a page, highlight/underline/strike text. Editing or removing the *existing* text of a PDF is explicitly out — that @@ -229,122 +227,44 @@ Three things the spike did **not** settle, and Phase 1 and 2 owe tests for each: whose newest section is an xref stream.** The spike's fixture had neither; Phase 1's tests cover both. -## Implementation plan - -Ordered so each step is verifiable on its own. Estimates are working days. - -### Phase 0 — serialization correctness — **done** (#843) - -`Transform2D::inverse`; escaping for `StandardString`, `Name` and dictionary -keys; reals through `util::number::to_string_significant`, since `{:.4g}` both -rounded to four significant digits and reached for an exponent form 7.3.3 has -no syntax for. Pinned by a round trip through `ObjectParser`. - -### Phase 0.5 — the parse facts a writer needs — **done** (#844) - -Appending needs four things about the file, and `DocumentParser` computed all -four while keeping one. `xref()`/`trailer()` were reachable; the newest -section's offset, its kind, and the recovery flag were not. Now -`start_xref_position()`, `xref_kind()`, `is_recovered()` and -`highest_object_id()`. - -The first two are `std::optional` and recovery clears them — a rebuilt table -has no section of the file's own to chain onto, so the missing value and -decision 2's refusal gate are the same fact. - -### Phase 1 — the incremental writer — **done** (#845) - -`pdf/pdf_writer.{hpp,cpp}`: `IncrementalWriter` pipes the source through -untouched, appends the objects it collected, and closes with a cross-reference -section naming only their ids and a trailer chaining back through `/Prev`. - -- **Matches the file's xref flavor** (`xref_kind()`), classic table or - cross-reference stream — the latter minting an id and an entry for the stream - object itself. -- **Refuses** a recovered file (decision 2) and an encrypted one (decision 6), - resolving both gates in the constructor so nothing downstream re-asks. -- **`/ID[1]` is derived from the update's own bytes**, not from a clock, so - writing the same update twice gives the same file and a test can pin it. -- Only the update is buffered; the source is piped, so appending to a large - file does not hold it in memory. - -Verified in the order the plan asked for — a no-op update that re-parses -identically first, then a `/Rotate` rewrite. `qpdf --check` passes, and -ghostscript, CoreGraphics and our own renderer all honour the new rotation -(the page box turns 8.5×11in into 11×8.5in). - -A page dictionary living inside an object stream is rewritten uncompressed in -the new section, the newer type-1 entry winning over the older type-2 one. - -### Phases 2 and 3 — text markup and ink — **done** (#847) - -`pdf/pdf_annotation.{hpp,cpp}`: `write_text_markup` covers `/Highlight`, -`/Underline`, `/StrikeOut` and `/Squiggly`; `write_ink` covers `/Ink`, its -strokes smoothed Catmull-Rom → cubic bezier. `append_page_annotations` puts -them on the page, rewriting the `/Annots` array itself where it is indirect. - -Only the highlight multiplies (11.6.4.1) — it is a wash over the text, where -the others are marks drawn on top of it. Opacity rides on the annotation's -`/CA` alone, which a viewer applies to the whole appearance; setting `ca` in -the appearance's own state as well would square it. - -**`/QuadPoints` ordering is settled** against two appearance-less files that -force a viewer to synthesize one: ghostscript draws the Z-order as a clean -rectangle and 12.5.6.10's counterclockwise order as a twisted blob. -CoreGraphics synthesizes nothing at all, so it is no oracle here. - -### Phase 4 — public API (1 d, ~130 lines) - -`PdfFile::annotate(std::string_view json, std::ostream &out, const Logger &)`, -throwing per the repo's fail-fast rule. A `FileTypeCapabilities` bit for it, and -the `file_type_table` row (the capability test fails if the declaration exceeds -what the engine does). - -### Phase 5 — browser layer — **done** (#849) - -`pdf_annotation_js` and `pdf_annotation_css` in `frontend.cpp`, alongside -`viewport_js`/`search_js`, exposing `odr.annotation`. - -Each page div carries `data-odr-page` and `data-odr-space`, the latter being -`to_box⁻¹` — which is what `Transform2D::inverse` was added for. A viewport -point divides out the zoom (`rect.width / offsetWidth`), converts css pixels to -points, and goes through that matrix; the model keeps page-box points and maps -to user space only in `getAnnotations()`. - -**Two overlays per page.** A `mix-blend-mode` on a shape *inside* an svg -composites against the svg's own canvas, not against the page, so a highlight -painted that way covers the glyphs instead of letting them through. The blend -belongs on the overlay element, and the washes therefore need an overlay of -their own (`svg.an-m`) separate from the marks drawn on top (`svg.an`). - -The overlay captures pointer events only for ink; the text tools leave the -selection layer alone, which is what makes selecting text to highlight work. - -Checks in `test/browser/annotation/`, run by hand as the repo's other emitted -scripts are. - -### Phase 6 — bindings — **done** (#850) - -`annotate` and the `annotate` capability across wasm, JNI, python and Apple. -Each returns the annotated bytes rather than writing a file: none of these -callers has a filesystem the caller would want written to. - -### Phase 7 — corpus and interop (2 d, ~600 test lines) - -Reference-output snapshot entries; interop check of our output in Acrobat, -Preview and pdf.js. - -**Total ≈ 15–20 days, ≈2,700–3,300 lines** — about 1,000 C++ in `src/`, 600 C++ -test, 780 JS/CSS, 470 bindings. Per-app UI (droid/ios toolbars) is on top and -outside this repo. - -**Narrower MVP** — highlight and ink, wasm only, unencrypted, no delete — -**6–8 days, ~1,400 lines**, and shippable, because the render side already -exists. +## How it landed + +| | | +|---|---| +| #843 | Object serialization made writable — escaping, and reals through `to_string_significant` rather than `{:.4g}`, which both rounded to four significant digits and reached for an exponent form 7.3.3 has no syntax for. `Transform2D::inverse`. | +| #844 | The parse facts a writer needs: `start_xref_position()`, `xref_kind()`, `is_recovered()`, `highest_object_id()`. The first two are optional and recovery clears them, so the missing value and decision 2's refusal gate are the same fact. | +| #845 | `IncrementalWriter`. Verified plumbing-first: a no-op update that re-parses identically, then a `/Rotate` rewrite, before any annotation semantics existed to blame. | +| #846 | The object-stream page rewrite, which modern producers make the common case. It already worked. | +| #847 | `write_text_markup`, `write_ink`, `append_page_annotations`. | +| #848 | `PdfFile::annotate` and the wire format above; the `annotate` capability. | +| #849 | `odr.annotation` and the page attributes it reads. | +| #850 | python, java, swift and wasm. | + +Two things cost more than the estimate said, and both were found by looking +rather than by testing: + +- **`mix-blend-mode` on a shape inside an svg composites against that svg's own + canvas**, not against the page, so the first highlight overlay painted over + the glyphs — the exact failure `/BM /Multiply` exists to prevent. The blend + belongs on the overlay element, which forces a separate overlay for the + washes. +- **`selectionchange` fires on every character a drag covers.** Marking on the + first one and clearing the selection mid-gesture turned one intended + highlight into six fragments. The mark now waits for the pointer to come up. + +## Verified against + +Six engines read what we write: ghostscript, PDFium (Chrome), CoreGraphics +(Preview), pdf.js, qpdf's structural check, and our own renderer — which is the +self-verifying one, since it paints only from `/AP /N`. Acrobat itself has not +been tried; nothing in the file is Acrobat-specific, but that is an assumption +rather than a result. + +The reference-output snapshot covers every rendered pdf page. ## What the writer unlocks next -Nearly free once Phases 0–2 land, all reusing the same appearance machinery: +Nearly free now, all reusing the same appearance machinery: - **Underline / StrikeOut / Squiggly** — the highlight path with a different appearance and subtype. @@ -365,18 +285,21 @@ Medium: - **AcroForm field fill** — the writer makes it possible, but regenerating appearances from `/V` and `/DA` is the real work, and `pdf/AGENTS.md` scopes form interactivity out today. +- **Deleting a foreign annotation** — we remove only what we wrote, identified + by its `/NM`; removing someone else's means proving nothing references it. ## Open questions -- **Link overlays vs. the highlight tool.** `` overlays already sit above the - `.sel` layer and block selection (`pdf/AGENTS.md` roadmap). A - selection-driven highlight tool makes that conflict user-visible rather than - theoretical — does this feature force the reverted `elementFromPoint` - workaround (commit `5cfa8a09`) back onto the table? +- **Link overlays vs. the highlight tool.** `` overlays sit above the `.sel` + layer and block selection (`pdf/AGENTS.md` roadmap), so text under a link + cannot be highlighted by selecting it. The markup tools capture no pointer + events, so this is the link overlay's problem rather than the annotator's — + but it is user-visible now rather than theoretical. Does it force the reverted + `elementFromPoint` workaround (commit `5cfa8a09`) back onto the table? - **Annotating a linearized file** breaks its linearization: the `/Linearized` dictionary then describes a prefix that is no longer the whole file. Viewers cope and Acrobat does the same — do we say so and move on, or de-linearize? -- **Encrypted files**: is refusing acceptable for the app's real corpus, or does - the `Decryptor` key accessor need to land in v1 after all? +- **Encrypted files** are refused (decision 6). Is that acceptable for the + app's real corpus, or does the `Decryptor` key accessor need to land? - **Where does the pending-annotation state live across a reload** in the mobile WebView — the browser only, or does the host persist the payload? diff --git a/jni/java/app/opendocument/core/PdfFile.java b/jni/java/app/opendocument/core/PdfFile.java index c5b359434..00f3066e4 100644 --- a/jni/java/app/opendocument/core/PdfFile.java +++ b/jni/java/app/opendocument/core/PdfFile.java @@ -12,6 +12,11 @@ public PdfFile decrypt(String password) { return new PdfFile(decryptPdfFileNative(handle(), password)); } + /** Whether this file can take annotations. */ + public boolean isAnnotatable() { + return isAnnotatableNative(handle()); + } + /** * Applies markup annotations and returns the annotated pdf. * @@ -25,4 +30,6 @@ public byte[] annotate(String annotations) { private native long decryptPdfFileNative(long handle, String password); private native byte[] annotateNative(long handle, String annotations); + + private native boolean isAnnotatableNative(long handle); } diff --git a/jni/src/jni_file.cpp b/jni/src/jni_file.cpp index cd329664e..8a7f36403 100644 --- a/jni/src/jni_file.cpp +++ b/jni/src/jni_file.cpp @@ -366,6 +366,15 @@ Java_app_opendocument_core_PdfFile_decryptPdfFileNative(JNIEnv *env, jobject, }); } +extern "C" JNIEXPORT jboolean JNICALL +Java_app_opendocument_core_PdfFile_isAnnotatableNative(JNIEnv *env, jobject, + jlong handle) { + return guarded(env, [&] { + return static_cast( + decoded(handle).as_pdf_file().is_annotatable()); + }); +} + extern "C" JNIEXPORT jbyteArray JNICALL Java_app_opendocument_core_PdfFile_annotateNative(JNIEnv *env, jobject, jlong handle, diff --git a/python/src/bind_file.cpp b/python/src/bind_file.cpp index ab338ea60..3d1826927 100644 --- a/python/src/bind_file.cpp +++ b/python/src/bind_file.cpp @@ -328,6 +328,8 @@ void odr_python::bind_file(py::module_ &m) { .def("document", &odr::DocumentFile::document); py::class_(m, "PdfFile") + .def("is_annotatable", &odr::PdfFile::is_annotatable, + "Whether this file can take annotations.") .def( "annotate", [](const odr::PdfFile &file, const std::string &annotations) { diff --git a/src/odr/file.cpp b/src/odr/file.cpp index 70a3f3650..90726737f 100644 --- a/src/odr/file.cpp +++ b/src/odr/file.cpp @@ -145,6 +145,8 @@ FileTypeCapabilities DecodedFile::capabilities() const { result.translate_html && encryption_state() != EncryptionState::encrypted; // there is no scheme without html result.color_scheme = result.color_scheme && result.translate_html; + // a file we cannot append to cannot be annotated, whatever the format can do + result.annotate = result.annotate && m_impl->annotatable(); // `edit`/`save`/`encrypt` stay as declared — resolving them would mean // decoding the document; ask `Document` for the precise answer @@ -354,6 +356,8 @@ PdfFile PdfFile::decrypt(const std::string &password) const { return DecodedFile::decrypt(password).as_pdf_file(); } +bool PdfFile::is_annotatable() const noexcept { return m_impl->annotatable(); } + void PdfFile::annotate(const std::string_view annotations, std::ostream &out, const Logger &logger) const { m_impl->annotate(annotations, out, logger); diff --git a/src/odr/file.hpp b/src/odr/file.hpp index a166ff866..4ff7e08e5 100644 --- a/src/odr/file.hpp +++ b/src/odr/file.hpp @@ -204,7 +204,9 @@ struct FileTypeCapabilities final { bool edit{}; ///< @ref Document::is_editable can be `true` bool save{}; ///< @ref Document::save is supported bool encrypt{}; ///< @ref Document::save with a password is supported - bool annotate{}; ///< @ref PdfFile::annotate is supported + bool annotate{}; ///< @ref PdfFile::annotate is supported; a concrete file + ///< still answers for itself with + ///< @ref PdfFile::is_annotatable }; /// Collection of encryption states. @@ -529,6 +531,16 @@ class PdfFile final : public DecodedFile { [[nodiscard]] PdfFile decrypt(const std::string &password) const; + /// @brief Whether this file can take annotations. + /// + /// The counterpart of @ref Document::is_editable, and the question to ask + /// before offering the user an annotate button: @ref FileTypeCapabilities + /// answers for the *format*, this one for the file in hand. False for a pdf + /// declaring an `/Encrypt` — including an owner-locked one that opened with + /// the empty password and so reports itself unencrypted — and for one whose + /// cross-reference table had to be rebuilt by scanning. + [[nodiscard]] bool is_annotatable() const noexcept; + /// @brief Applies markup @p annotations, writing the annotated pdf to /// @p out. /// @@ -537,8 +549,7 @@ class PdfFile final : public DecodedFile { /// source is copied and the annotations appended, so nothing else about the /// file changes. /// @throws std::invalid_argument if @p annotations is malformed. - /// @throws std::runtime_error if the file cannot take them — its - /// cross-reference table was recovered, or it is encrypted. + /// @throws std::runtime_error if @ref is_annotatable is false. void annotate(std::string_view annotations, std::ostream &out, const Logger &logger = Logger::null()) const; diff --git a/src/odr/internal/abstract/file.hpp b/src/odr/internal/abstract/file.hpp index cd0262962..c731d075b 100644 --- a/src/odr/internal/abstract/file.hpp +++ b/src/odr/internal/abstract/file.hpp @@ -52,6 +52,11 @@ class DecodedFile { [[nodiscard]] virtual EncryptionState encryption_state() const noexcept { return EncryptionState::not_encrypted; } + /// Whether this particular file can take annotations. Not the same question + /// as `encryption_state()`: an owner-locked pdf opens with the empty + /// password and reports itself unencrypted, yet still carries the `/Encrypt` + /// that stops us appending to it. + [[nodiscard]] virtual bool annotatable() const noexcept { return false; } [[nodiscard]] virtual std::shared_ptr decrypt([[maybe_unused]] const std::string &password) const { return nullptr; @@ -140,6 +145,7 @@ class PdfFile : public DecodedFile { } /// Apply `annotations` and write the result to `out`. + /// @throws std::runtime_error when `annotatable()` is false. virtual void annotate(std::string_view annotations, std::ostream &out, const Logger &logger) const = 0; }; diff --git a/src/odr/internal/pdf/AGENTS.md b/src/odr/internal/pdf/AGENTS.md index 320b48408..dc10ee53a 100644 --- a/src/odr/internal/pdf/AGENTS.md +++ b/src/odr/internal/pdf/AGENTS.md @@ -4,10 +4,13 @@ The **why** behind the `pdf/` module and the roadmap. What is concretely implemented is in the code; this file keeps the rationale, the non-obvious invariants, and where things live. Reference links live in [`README.md`](README.md). -**Goal.** Faithful read-only HTML for common real-world PDFs through a -pure-serialization pipeline (no native renderer). The file-format, -text-extraction, font and graphics foundations are in place; what remains is -interaction & navigation plus a tail of known gaps (see *Roadmap*). +**Goal.** Faithful HTML for common real-world PDFs through a pure-serialization +pipeline (no native renderer). Reading is what the module mostly is; the one +thing it writes is **markup annotations**, appended without disturbing anything +already in the file (see [`docs/design/pdf-annotation.md`](../../../../docs/design/pdf-annotation.md)). +The file-format, text-extraction, font and graphics foundations are in place; +what remains is interaction & navigation plus a tail of known gaps (see +*Roadmap*). **Scope in one line.** Parse the PDF object/file structure (xref tables, xref streams, object streams, hybrid files, forward-scan recovery), decrypt (RC4, @@ -15,6 +18,9 @@ AES-128, AES-256), build the page tree, tokenize content streams, and emit HTML: absolutely-positioned text spans placed by the full text transform, with vector graphics / images / shadings / patterns / transparency as inline SVG per page, embedded fonts via `@font-face` and non-embedded fonts substituted to CSS stacks. +On the write side: append an incremental update (7.5.6) carrying highlight, +underline, strike-out, squiggly and ink annotations, each with the appearance +stream it paints through. --- @@ -226,7 +232,9 @@ Things the code won't shout at you: | `pdf_graphics_operator*.{hpp,cpp}` | Operator enum + `GraphicsOperator`; content-stream tokenizer | | `pdf_graphics_state.{hpp,cpp}` | `GraphicsState`: state stack, `execute(op)` for the modelled subset; CTM/`Tm`/`Tlm`; `text_placement_matrix()`, `advance_text()`; `save`/`restore`/`concat_matrix` reused by `q`/`Q`/`cm` and form invocation | | `pdf_page_text.{hpp,cpp}` | `extract_text`: content → `TextElement` per shown segment; `Do` recursion; marked-content/`ActualText`; pen-based space inference | -| `pdf_file.{hpp,cpp}` | `abstract::PdfFile`; probes encryption at construction, carries the authenticated `Decryptor` forward | +| `pdf_file.{hpp,cpp}` | `abstract::PdfFile`; probes encryption at construction, carries the authenticated `Decryptor` forward; `annotate` parses the json wire format and drives the two below | +| `pdf_writer.{hpp,cpp}` | `IncrementalWriter`: copies the source through and appends changed objects under a cross-reference section of their own, matching the file's own xref flavor. Refuses a recovered or encrypted file | +| `pdf_annotation.{hpp,cpp}` | The markup and ink annotations, and the appearance streams they paint through. Only the highlight blends Multiply (11.6.4.1) | Consumers outside the module: `open_strategy.cpp` (detection/engine selection) and `html/pdf_file.cpp` (`create_pdf_service`; the per-font PUA re-encode + OTF wrap + @@ -270,8 +278,10 @@ fixtures are verified manually but not pinned. # Roadmap -The next feature cluster is **interaction & navigation**; the rest is a tail of -known gaps. Each remaining item gets its own detailed design before +Markup annotations have landed — see +[`docs/design/pdf-annotation.md`](../../../../docs/design/pdf-annotation.md) for +the decisions and what the writer unlocks next. The next feature cluster is +**interaction & navigation**; the rest is a tail of known gaps. Each remaining item gets its own detailed design before implementation. Grow the corpus alongside (odr-public fixtures + the PDF101 "nasty files" collection linked in `README.md`; assertion tests per feature). @@ -286,7 +296,8 @@ Link annotations (`/URI` + internal `/GoTo`) already land. Remaining: `pointer-events:none`, a click handler re-hit-tests via `elementFromPoint`) was built then **reverted as too involved** (commit `5cfa8a09`, reachable for later). Options: (a) reinstate it; (b) a CSS-only route if one exists; - (c) accept clickable-only. + (c) accept clickable-only. The markup annotator made this user-visible: text + under a link cannot be highlighted by selecting it either. - **Document outline** (`/Outlines`) → nav anchors/sidebar. - **Optional content groups** (layers): honor default visibility, no toggle UI. - **Output scaling**: monolithic HTML vs. per-page lazy loading (check what odr's diff --git a/src/odr/internal/pdf/pdf_file.cpp b/src/odr/internal/pdf/pdf_file.cpp index 7ae2e0327..580727eef 100644 --- a/src/odr/internal/pdf/pdf_file.cpp +++ b/src/odr/internal/pdf/pdf_file.cpp @@ -111,6 +111,10 @@ PdfFile::PdfFile(std::shared_ptr file) DocumentParser parser(m_file->stream()); m_file_meta.type = FileType::portable_document_format; + // Both conditions `IncrementalWriter` refuses on. An `/Encrypt` counts even + // where the empty password opens it, since the new objects would still have + // to be encrypted with a key the parser does not keep. + m_annotatable = !parser.is_encrypted() && !parser.is_recovered(); m_authenticator = parser.authenticator(); if (parser.is_encrypted()) { @@ -143,6 +147,8 @@ std::shared_ptr PdfFile::file() const noexcept { FileMeta PdfFile::file_meta() const noexcept { return m_file_meta; } +bool PdfFile::annotatable() const noexcept { return m_annotatable; } + bool PdfFile::password_encrypted() const noexcept { return m_encryption_state == EncryptionState::encrypted; } diff --git a/src/odr/internal/pdf/pdf_file.hpp b/src/odr/internal/pdf/pdf_file.hpp index 982f8b5c0..ddfbdcb1b 100644 --- a/src/odr/internal/pdf/pdf_file.hpp +++ b/src/odr/internal/pdf/pdf_file.hpp @@ -27,6 +27,7 @@ class PdfFile final : public abstract::PdfFile { decrypt(const std::string &password) const override; [[nodiscard]] bool is_decodable() const noexcept override; + [[nodiscard]] bool annotatable() const noexcept override; void annotate(std::string_view annotations, std::ostream &out, const Logger &logger) const override; @@ -41,6 +42,9 @@ class PdfFile final : public abstract::PdfFile { FileMeta m_file_meta; EncryptionState m_encryption_state{EncryptionState::not_encrypted}; + /// What `IncrementalWriter`'s constructor would accept, settled by the + /// probe rather than by re-parsing when someone asks. + bool m_annotatable{false}; }; } // namespace odr::internal::pdf diff --git a/test/data.cmake b/test/data.cmake index 5dacfb5f6..e00c71919 100644 --- a/test/data.cmake +++ b/test/data.cmake @@ -17,9 +17,9 @@ odr_test_data( odr_test_data( PATH "reference-output/odr-public" URL "https://github.com/opendocument-app/OpenDocument.test.output.git" - REVISION "29fc1949468ed7aeba800a74db5025a7f8b61105") + REVISION "19392dfc4e3f4c5507dae231fe96a92fe0ca29f4") odr_test_data( PATH "reference-output/odr-private" URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git" - REVISION "d7aa0dc9c9e54450df9822322bff5d517ff520ba") + REVISION "ee3077eaf35b36e084f14b130c4e9236fc9be0f1") diff --git a/test/src/pdf_annotate_test.cpp b/test/src/pdf_annotate_test.cpp index 10f5d669e..d496407db 100644 --- a/test/src/pdf_annotate_test.cpp +++ b/test/src/pdf_annotate_test.cpp @@ -6,9 +6,12 @@ #include #include +#include + #include #include +#include #include #include #include @@ -17,6 +20,7 @@ #include using namespace odr; +using namespace odr::internal; using namespace odr::test; using odr::internal::pdf::DocumentParser; using odr::internal::pdf::Page; @@ -61,6 +65,45 @@ TEST(PdfAnnotate, capability_is_declared) { capabilities_by_file_type(FileType::portable_document_format).annotate); } +// The static table is an upper bound for the format; the file narrows it to +// what this file can actually take. +TEST(PdfAnnotate, capability_narrows_to_the_file) { + const PdfFile plain = open_fixture("odr-public/pdf/style-various-1.pdf"); + EXPECT_TRUE(plain.is_annotatable()); + EXPECT_TRUE(plain.capabilities().annotate); + + // Owner-locked: the empty password opens it, so it reports itself + // unencrypted — but the `/Encrypt` is still there and we cannot append. + const PdfFile locked = open_fixture("odr-public/pdf/Casio_WVA-M650-7AJF.pdf"); + EXPECT_EQ(locked.encryption_state(), EncryptionState::not_encrypted); + EXPECT_FALSE(locked.is_annotatable()); + EXPECT_FALSE(locked.capabilities().annotate); + + std::ostringstream out; + EXPECT_ANY_THROW(locked.annotate(std::string(one_highlight), out)); +} + +// A file whose cross-reference table had to be rebuilt cannot take an +// incremental update either, and says so before it is asked. +TEST(PdfAnnotate, capability_is_false_for_a_recovered_file) { + // the header still claims a pdf, so it is detected; the `startxref` points + // nowhere, so the table has to be rebuilt by scanning + std::string source = util::file::read( + TestData::test_file_path("odr-public/pdf/style-various-1.pdf")); + const std::size_t position = + source.rfind("startxref\n") + std::strlen("startxref\n"); + source.replace(position, source.find('\n', position) - position, "999999"); + + const PdfFile file = + open(File::from_memory(source), DecodeOptions{}, Logger::null()) + .as_pdf_file(); + EXPECT_FALSE(file.is_annotatable()); + EXPECT_FALSE(file.capabilities().annotate); + + std::ostringstream out; + EXPECT_ANY_THROW(file.annotate(std::string(one_highlight), out)); +} + TEST(PdfAnnotate, writes_a_highlight) { const std::string result = annotate(std::string(one_highlight)); diff --git a/wasm/js/index.d.ts b/wasm/js/index.d.ts index a348b1873..c24299195 100644 --- a/wasm/js/index.d.ts +++ b/wasm/js/index.d.ts @@ -157,6 +157,12 @@ export declare class Document { * @throws OdrError `NoDocumentFile` */ edit(diff: string): this; + /** + * Whether this pdf can take annotations — false for one declaring an + * `/Encrypt`, or whose cross-reference table had to be rebuilt. + */ + isAnnotatable(): boolean; + /** * Applies markup annotations to a pdf and returns the annotated bytes. * diff --git a/wasm/js/index.js b/wasm/js/index.js index a3e012444..6c68fcdca 100644 --- a/wasm/js/index.js +++ b/wasm/js/index.js @@ -91,6 +91,11 @@ export class Document { return this; } + // Whether this pdf can take annotations. + isAnnotatable() { + return unwrap(this.#core.isAnnotatable(this.#handle)); + } + // The annotated pdf's bytes; what `odr.annotation.getAnnotations()` collected // goes in. annotate(annotations) { diff --git a/wasm/src/wasm_file.cpp b/wasm/src/wasm_file.cpp index 6e5e3beaf..12c0dc88d 100644 --- a/wasm/src/wasm_file.cpp +++ b/wasm/src/wasm_file.cpp @@ -115,6 +115,16 @@ emscripten::val file_type(const Handle handle) { }); } +/// Whether this file can take annotations, the counterpart of `isEditable`. +emscripten::val is_annotatable(const Handle handle) { + return guarded([&] { + const Session &s = session(handle); + return ok(emscripten::val(s.file.file_type() == + FileType::portable_document_format && + s.file.as_pdf_file().is_annotatable())); + }); +} + /// The annotated pdf's bytes; there is no filesystem to write to. `payload` is /// what the rendered page's `odr.annotation.getAnnotations()` collected. emscripten::val annotate(const Handle handle, const std::string &payload) { @@ -152,6 +162,7 @@ EMSCRIPTEN_BINDINGS(odr_file) { emscripten::function("decrypt", &odr::wasm::decrypt); emscripten::function("fileType", &odr::wasm::file_type); emscripten::function("fileName", &odr::wasm::file_name); + emscripten::function("isAnnotatable", &odr::wasm::is_annotatable); emscripten::function("annotate", &odr::wasm::annotate); emscripten::function("close", &odr::wasm::close); emscripten::function("closeAll", &odr::wasm::close_all);