From ec257868dd65c888f220f51d92d6020c7db42080 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sat, 19 Sep 2026 09:23:30 +0200 Subject: [PATCH 1/2] feat(html): the pdf annotator reports its changes and takes a tool press A host had to count the pending marks after every pointer event, because odr.annotation had no callback. The count settles 50ms after the pointer lifts, so each host also waited on a timer. odr.onAnnotationChange({count}) now reports each change: a mark, a finished stroke, an undo, a removal and a clear. odr.annotation.press(tool, style) and recolor(tool, style) do what a tool button does. Over a selection, the tool marks it once and no tool stays armed. With no selection, press arms or disarms the tool, and recolor restyles the armed tool. The Android app, the iOS app and the website each had their own copy of this logic. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01H3sFjPT7zd8fvmpqhznzEP --- CHANGELOG.md | 10 ++ docs/design/editing.md | 4 +- .../internal/html/frontend/pdf-annotation.js | 106 +++++++++++++++--- test/browser/annotation/tests.html | 99 ++++++++++++++++ 4 files changed, 205 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d073a45b..f9002aacc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,16 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- The rendered pdf view reports each change to the pending annotations on + `odr.onAnnotationChange({count})`. A host no longer has to count them after + every pointer event. + +- `odr.annotation.press(tool, {color, width})` and `recolor(tool, style)` do + what a tool button does. Over a selection, the tool marks the selection once + and no tool stays armed. With no selection, a press arms the tool or disarms + it, and `recolor` restyles the armed tool. Both return the tool that stays + armed. + - The wasm `Document.edit`, `save`, `isEditable` and `isSavable` work for a plain text file, which saves as UTF-8. diff --git a/docs/design/editing.md b/docs/design/editing.md index b28ee3acd..664e02e49 100644 --- a/docs/design/editing.md +++ b/docs/design/editing.md @@ -483,4 +483,6 @@ version drifting until a new check page caught it. `odr.annotation` is its own API and `PdfFile::annotate` its own write path. It is a different gesture from editing text, so whether it should share the mode is a real question rather than an oversight - ([`txt-editing.md`](txt-editing.md) carries it too). + ([`txt-editing.md`](txt-editing.md) carries it too). It reports on a + callback of its own, `odr.onAnnotationChange`, and a pdf page does not + carry `editing.js`. diff --git a/src/odr/internal/html/frontend/pdf-annotation.js b/src/odr/internal/html/frontend/pdf-annotation.js index 555953b14..4f5e26e6c 100644 --- a/src/odr/internal/html/frontend/pdf-annotation.js +++ b/src/odr/internal/html/frontend/pdf-annotation.js @@ -18,6 +18,22 @@ overscrollBehavior: "contain", }; + /// Raised as what is pending changes: a mark, a finished stroke, an undo, a + /// removal, a clear. A stroke counts once it ends. + odr.onAnnotationChange = function () {}; + + var reported = 0; + + function changed() { + if (pending.length === reported) { + return; + } + reported = pending.length; + if (typeof odr.onAnnotationChange === "function") { + odr.onAnnotationChange({ count: pending.length }); + } + } + function pages() { return Array.prototype.slice.call( document.querySelectorAll("[data-odr-space]") @@ -269,8 +285,8 @@ /// One annotation per page the selection covers. `keep` holds the selection, /// which the automatic path cannot: the next `selectionchange` re-marks it. - function markSelection(keep) { - if (!tool || tool === "ink") { + function markSelection(type, rgb, keep) { + if (!type || type === "ink") { return false; } var byPage = selectionBoxes(); @@ -279,9 +295,9 @@ pending.push({ id: nextId++, page: +index, - type: tool, + type: type, boxes: byPage[index], - color: color.slice(), + color: rgb.slice(), }); added = true; }); @@ -290,10 +306,49 @@ window.getSelection().removeAllRanges(); } redraw(); + changed(); } return added; } + function hasSelection() { + var selection = window.getSelection(); + return ( + !!selection && !selection.isCollapsed && selection.toString().length > 0 + ); + } + + function arm(value) { + tool = value || null; + pages().forEach(function (page) { + page.classList.toggle("an-draw", tool === "ink"); + }); + document.documentElement.classList.toggle("an-drawing", tool === "ink"); + } + + function applyStyle(style) { + if (style && style.color) { + color = style.color.slice(0, 3).map(Number); + } + if (style && style.width !== undefined) { + width = Number(style.width); + } + } + + /// A tool button pressed over a standing selection: marks it once, and + /// leaves no tool armed. Disarmed first, so the cleared selection cannot + /// mark itself a second time. + function markOnce(type, style) { + var rgb = + style && style.color ? style.color.slice(0, 3).map(Number) : color; + arm(null); + markSelection(type, rgb, false); + } + + function marksSelection(type) { + return !!type && type !== "ink" && hasSelection(); + } + var stroke = null; var strokeNode = null; @@ -326,7 +381,7 @@ } window.clearTimeout(settle); settle = window.setTimeout(function () { - markSelection(false); + markSelection(tool, color, false); }, 50); } @@ -415,6 +470,7 @@ stroke = null; strokeNode = null; strokePointer = null; + changed(); } function applyOptions() { @@ -433,16 +489,37 @@ odr.annotation = { /// null, "highlight", "underline", "strikeOut", "squiggly" or "ink". - setTool: function (value) { - tool = value || null; - pages().forEach(function (page) { - page.classList.toggle("an-draw", tool === "ink"); - }); - document.documentElement.classList.toggle("an-drawing", tool === "ink"); - }, + setTool: arm, getTool: function () { return tool; }, + /// A tool button, pressed. Over a selection, @p type marks it once and no + /// tool stays armed; otherwise the press arms @p type, or disarms it if it + /// is armed already. Null disarms. @p style is `{color, width}`, either + /// optional. Answers the tool left armed. + press: function (type, style) { + type = type || null; + if (marksSelection(type)) { + markOnce(type, style); + } else if (type !== null && type === tool) { + arm(null); + } else { + applyStyle(style); + arm(type); + } + return tool; + }, + /// A new @p style chosen for @p type. Over a selection it marks it once, as + /// `press` does; else it restyles @p type if armed. Answers the tool left + /// armed. + recolor: function (type, style) { + if (marksSelection(type)) { + markOnce(type, style); + } else if (type && type === tool) { + applyStyle(style); + } + return tool; + }, /// DeviceRGB, each component in [0, 1]. setColor: function (value) { color = value.slice(0, 3).map(Number); @@ -470,7 +547,7 @@ /// Marks the selection with the armed tool, and answers whether anything /// was added. The selection is left standing. mark: function () { - return markSelection(true); + return markSelection(tool, color, true); }, /// What is pending, newest last. Geometry is in page-box points. list: function () { @@ -481,14 +558,17 @@ return a.id !== id; }); redraw(); + changed(); }, undo: function () { pending.pop(); redraw(); + changed(); }, clear: function () { pending = []; redraw(); + changed(); }, /// The payload `PdfFile::annotate` takes, in pdf user space. getAnnotations: function () { diff --git a/test/browser/annotation/tests.html b/test/browser/annotation/tests.html index 09493478b..54aaaf232 100644 --- a/test/browser/annotation/tests.html +++ b/test/browser/annotation/tests.html @@ -347,6 +347,105 @@ api.clear(); api.setTool(null); + + // --- the host hears of every change to what is pending ---------------- + const heard = []; + window.odr.onAnnotationChange = (event) => heard.push(event.count); + api.setTool("highlight"); + selectRun(0); + api.mark(); + check("a mark is reported", heard.join() === "1", heard); + api.undo(); + check("an undo is reported", heard.join() === "1,0", heard); + api.undo(); + api.clear(); + check("an undo or a clear that changes nothing is not", heard.length === 2, heard); + + heard.length = 0; + api.setTool("ink"); + document.dispatchEvent(new PointerEvent("pointerdown", at(100, 500))); + document.dispatchEvent(new PointerEvent("pointermove", at(140, 540))); + check("a stroke is not reported while it is drawn", heard.length === 0, heard); + document.dispatchEvent(new PointerEvent("pointerup", at(140, 540))); + check("but once it ends", heard.join() === "1", heard); + api.remove(api.list()[0].id); + check("a removal is reported", heard.join() === "1,0", heard); + api.setTool(null); + + // --- a tool button: mark a standing selection, or arm and disarm ------ + heard.length = 0; + selectRun(0); + check( + "a press over a selection arms nothing", + api.press("underline", { color: [0, 0, 1] }) === null + ); + const pressed = payload().annotations; + check( + "and marks it once, in the colour pressed", + pressed.length === 1 && + pressed[0].type === "underline" && + pressed[0].color.join() === "0,0,1", + pressed + ); + check("the selection goes with it", window.getSelection().isCollapsed); + check("the host hears of it", heard.join() === "1", heard); + + check("a press with nothing selected arms", api.press("highlight") === "highlight"); + check("a second press disarms", api.press("highlight") === null); + api.press("highlight", { color: [1, 0, 0] }); + check("a press on another tool swaps", api.press("squiggly") === "squiggly"); + check("null disarms", api.press(null) === null); + selectRun(0); + check("the pen arms over a selection too", api.press("ink", { width: 4 }) === "ink"); + check("it marks nothing", api.list().length === 1, api.list()); + api.press(null); + + // a press over a selection with the automatic path on marks it once: the + // selection it clears must not come back as a second mark + api.clear(); + api.setOptions({ markOnSelection: true }); + selectRun(0); + api.press("strikeOut"); + await new Promise((r) => setTimeout(r, 100)); + check("with markOnSelection, still once", api.list().length === 1, api.list()); + api.setOptions({ markOnSelection: false }); + + // --- a new colour marks a selection, or restyles the armed tool ------- + api.clear(); + selectRun(0); + check( + "recolor over a selection marks it once", + api.recolor("highlight", { color: [0, 1, 0] }) === null && + api.list().length === 1 && + api.list()[0].color.join() === "0,1,0", + api.list() + ); + api.clear(); + api.press("highlight", { color: [1, 1, 0] }); + api.recolor("highlight", { color: [0, 1, 1] }); + selectRun(0); + api.mark(); + check( + "recolor restyles the armed tool", + api.list()[0].color.join() === "0,1,1", + api.list()[0].color + ); + api.clear(); + // mark() left it standing + window.getSelection().removeAllRanges(); + api.recolor("underline", { color: [1, 0, 1] }); + check("and arms nothing that was not armed", api.getTool() === "highlight"); + selectRun(0); + api.mark(); + check( + "nor restyles the armed one", + api.list()[0].color.join() === "0,1,1", + api.list()[0].color + ); + + window.odr.onAnnotationChange = () => {}; + api.clear(); + api.setTool(null); } run(); From 91f8fad0584b78c1b098fc1cead9ec3fe0a547ab Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sat, 19 Sep 2026 09:27:50 +0200 Subject: [PATCH 2/2] fix(html): a press over a selection no page holds arms the tool `press` and `recolor` tested the selection for text and then disarmed the tool, even when no page held the selection and nothing was marked. Now they disarm only when the mark is made, so such a press arms the tool. The comments and the changelog entry are shorter. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01C725vurM2ZqBY6ddV2FMZ7 --- CHANGELOG.md | 12 +--- .../internal/html/frontend/pdf-annotation.js | 58 +++++++------------ test/browser/annotation/tests.html | 14 ++++- 3 files changed, 37 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9002aacc..dae2346c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,15 +16,9 @@ The release run heads these entries with the version and opens a fresh ## Unreleased -- The rendered pdf view reports each change to the pending annotations on - `odr.onAnnotationChange({count})`. A host no longer has to count them after - every pointer event. - -- `odr.annotation.press(tool, {color, width})` and `recolor(tool, style)` do - what a tool button does. Over a selection, the tool marks the selection once - and no tool stays armed. With no selection, a press arms the tool or disarms - it, and `recolor` restyles the armed tool. Both return the tool that stays - armed. +- The pdf view reports the pending annotation count on + `odr.onAnnotationChange({count})`, and `odr.annotation.press` and `recolor` + do what a tool button does. - The wasm `Document.edit`, `save`, `isEditable` and `isSavable` work for a plain text file, which saves as UTF-8. diff --git a/src/odr/internal/html/frontend/pdf-annotation.js b/src/odr/internal/html/frontend/pdf-annotation.js index 4f5e26e6c..1bb0a3143 100644 --- a/src/odr/internal/html/frontend/pdf-annotation.js +++ b/src/odr/internal/html/frontend/pdf-annotation.js @@ -18,8 +18,7 @@ overscrollBehavior: "contain", }; - /// Raised as what is pending changes: a mark, a finished stroke, an undo, a - /// removal, a clear. A stroke counts once it ends. + /// Raised as the pending count changes; a stroke counts once it ends. odr.onAnnotationChange = function () {}; var reported = 0; @@ -311,13 +310,6 @@ return added; } - function hasSelection() { - var selection = window.getSelection(); - return ( - !!selection && !selection.isCollapsed && selection.toString().length > 0 - ); - } - function arm(value) { tool = value || null; pages().forEach(function (page) { @@ -326,30 +318,29 @@ document.documentElement.classList.toggle("an-drawing", tool === "ink"); } + function rgbOf(value) { + return value.slice(0, 3).map(Number); + } + function applyStyle(style) { if (style && style.color) { - color = style.color.slice(0, 3).map(Number); + color = rgbOf(style.color); } if (style && style.width !== undefined) { width = Number(style.width); } } - /// A tool button pressed over a standing selection: marks it once, and - /// leaves no tool armed. Disarmed first, so the cleared selection cannot - /// mark itself a second time. + /// Marks the selection once and disarms, if there is anything to mark. function markOnce(type, style) { - var rgb = - style && style.color ? style.color.slice(0, 3).map(Number) : color; + var rgb = style && style.color ? rgbOf(style.color) : color; + if (!markSelection(type, rgb, false)) { + return false; + } arm(null); - markSelection(type, rgb, false); - } - - function marksSelection(type) { - return !!type && type !== "ink" && hasSelection(); + return true; } - var stroke = null; var strokeNode = null; var strokePointer = null; @@ -493,15 +484,13 @@ getTool: function () { return tool; }, - /// A tool button, pressed. Over a selection, @p type marks it once and no - /// tool stays armed; otherwise the press arms @p type, or disarms it if it - /// is armed already. Null disarms. @p style is `{color, width}`, either - /// optional. Answers the tool left armed. + /// A tool button: marks a selection once, else arms @p type or disarms + /// it. @p style is `{color, width}`. Answers the tool left armed. press: function (type, style) { - type = type || null; - if (marksSelection(type)) { - markOnce(type, style); - } else if (type !== null && type === tool) { + if (markOnce(type, style)) { + return tool; + } + if (type && type === tool) { arm(null); } else { applyStyle(style); @@ -509,20 +498,17 @@ } return tool; }, - /// A new @p style chosen for @p type. Over a selection it marks it once, as - /// `press` does; else it restyles @p type if armed. Answers the tool left - /// armed. + /// Marks a selection once, as `press` does, else restyles @p type if it is + /// armed. Answers the tool left armed. recolor: function (type, style) { - if (marksSelection(type)) { - markOnce(type, style); - } else if (type && type === tool) { + if (!markOnce(type, style) && type && type === tool) { applyStyle(style); } return tool; }, /// DeviceRGB, each component in [0, 1]. setColor: function (value) { - color = value.slice(0, 3).map(Number); + color = rgbOf(value); }, setWidth: function (value) { width = Number(value); diff --git a/test/browser/annotation/tests.html b/test/browser/annotation/tests.html index 54aaaf232..a194a3d53 100644 --- a/test/browser/annotation/tests.html +++ b/test/browser/annotation/tests.html @@ -59,6 +59,7 @@ Selectable text on page two +

Text on no page

@@ -395,13 +396,22 @@ api.press("highlight", { color: [1, 0, 0] }); check("a press on another tool swaps", api.press("squiggly") === "squiggly"); check("null disarms", api.press(null) === null); + const outside = document.createRange(); + outside.selectNodeContents(document.getElementById("outside")); + window.getSelection().removeAllRanges(); + window.getSelection().addRange(outside); + check( + "a press over a selection on no page arms", + api.press("underline") === "underline" && api.list().length === 1, + api.list() + ); + api.press(null); selectRun(0); check("the pen arms over a selection too", api.press("ink", { width: 4 }) === "ink"); check("it marks nothing", api.list().length === 1, api.list()); api.press(null); - // a press over a selection with the automatic path on marks it once: the - // selection it clears must not come back as a second mark + // the selection a press clears must not come back as a second mark api.clear(); api.setOptions({ markOnSelection: true }); selectRun(0);