From db4e23d1431afb230764f66fa22f4821ed85879f Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Thu, 10 Sep 2026 09:07:51 +0200 Subject: [PATCH 01/11] docs(design): make the editing mode a decision every format shares The mode, the refusal channel and the dirty flag are what a host wires, and it wires them once per document - so they cannot live in the sheet's script. `editing.md` gains decisions 9 to 12: one generic `odr.editing` an editor attaches to, the frame stated on ``, `HtmlConfig::editable` as the switch that writes the scaffolding, and a config for the keys a host may need back. `spreadsheet-editing.md` keeps the cell's own decisions and points at the frame. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KKFKbUVCYF2VhujdmjhhPW --- docs/design/editing.md | 207 ++++++++++++++++++++++++++--- docs/design/spreadsheet-editing.md | 45 +++++-- 2 files changed, 228 insertions(+), 24 deletions(-) diff --git a/docs/design/editing.md b/docs/design/editing.md index 6c8ea672e..5fc91d519 100644 --- a/docs/design/editing.md +++ b/docs/design/editing.md @@ -1,10 +1,15 @@ # Editing design -Status: **accepted direction; implementation deferred.** This records the -architecture we chose for in-browser editing of ODF and OOXML documents, the -alternatives we weighed, and *why* we took each decision. The implementation plan -and sketch at the end are **not scheduled yet** — they are captured here so the -decisions and the grounding survive until we pick the work up later. +Status: **the mode frame is landed for every format; the text editor behind it +is not.** This records the architecture we chose for in-browser editing of ODF +and OOXML documents, the alternatives we weighed, and *why* we took each +decision. Decisions 9 to 12 are the frame every format shares, and they are in +the code. The phases below them are the text editor, and they are **not +scheduled yet** — they are captured here so the decisions and the grounding +survive until we pick the work up later. + +[`spreadsheet-editing.md`](spreadsheet-editing.md) is the first editor built on +the frame, and it is where a sheet's own decisions live. This builds on the existing principle in [`README.md`](README.md): @@ -18,13 +23,20 @@ to let the user edit content — remove any element, add paragraphs, and toggle simple inline formatting (bold, italic, underline, highlight) — and persist those edits back into the original ODF/OOXML file. -Today this exists only in skeleton form: - -- `html::translate(..., config.editable)` stamps `contenteditable="true"` on - elements where `Element::is_editable()` (`internal/html/document_element.cpp`). -- `html::edit(document, diff)` (`src/odr/html.cpp`) parses a JSON blob and - understands a single key, `modifiedText`: a map of `DocumentPath → new string`. - It navigates to each text element and calls `set_content`. +Today the frame is there and the text editor is not: + +- `html::translate(..., config.editable)` writes the editing scaffolding: the + page-level state on ``, `data-odr-path` on every editable run, and the + scripts that carry the mode (`internal/html/document_element.cpp`, + `internal/html/document.cpp`). +- `frontend/editing.js` owns `odr.editing` — the mode, the refusals, the log a + save reads and the callbacks a host wires. Every format's editor attaches to + it; `frontend/sheet-editing.js` is the first one (decision 9). +- `frontend/document.js` holds the text editor, and it is still the skeleton: + `contenteditable` runs, a `MutationObserver` keyed by `data-odr-path`, and one + `setText` op per changed run. No selection model, no marks, no undo. +- `Document::edit(diff)` (`src/odr/document.cpp`) parses the op envelope and + dispatches `setCell` and `setText`. - `back_translate` CLI replays a diff file onto a source document and `save`s it. The goal is to generalise this from "replace text in a span" to full content and @@ -165,6 +177,150 @@ project, and hand-rolling gives full control over the model↔op mapping. merge-on-backspace-at-boundary, delete across paragraphs. This is where the real complexity lives, not inline marks. +### 9. Editing is one browser mode for every format; a format brings only its editor + +`odr.editing` is the mode, and it is generic. It is written by +`frontend/editing.js` for every document view and it knows nothing about cells, +runs or paragraphs. It owns: + +- the **mode** — `enable()`, `disable()`, `isEnabled()`, `isEditable()`; +- the **refusals** — the code table, the repeat suppression, the outline a + refused element gets, and `odr.onEditRefused`; +- the **log** — `getOperations()`, `undo()`, `redo()`, `committed()`, and the + `dirty` / `canUndo` / `canRedo` state `odr.onEditChange` reports; +- the **keyboard classes** the page may take (decision 12). + +A format's editor is a second script that **attaches** one editor to the mode: + +```js +odr.editing.attach({ + name: "sheet", // what the log's ops belong to + enable: function () {}, // the mode turned on + disable: function () {}, // the mode turned off + operations: function () {}, // the ops this editor would hand a save + undo: function () {}, // false where this editor has nothing to take back + redo: function () {}, +}); +``` + +`frontend/sheet-editing.js` attaches the cell overlay; `frontend/document.js` +attaches the text runs. Neither states a mode, a code table or a callback of its +own. + +**Why:** the mode, the refusal channel and the dirty flag are what a *host* +wires, and a host wires them once for every document it opens — it must not +learn a second API because the file turned out to be a sheet. Before this, all +of it lived in `sheet-editing.js`, so a `.docx` had no `odr.editing` at all and +an app could not grey its edit button without knowing the format first. + +**Why an editor per format, rather than one editor over the element tree:** a +cell is edited by an overlay and a paragraph by a caret in the flow. The two +share the log and share nothing else. Decision 8 already said the DOM is a +projection of a model; the projection is what differs per format. + +**The log is the editor's until an editor can invert its ops.** `getOperations()` +concatenates what the attached editors report, and `undo()` asks each in turn. +The sheet keeps an op with its inverse beside it, so it answers; the text +skeleton keeps a map of changed runs and answers `false`. The moment phase 1 +gives text a real op log, it moves onto the shared one and the concatenation +becomes a single array. + +### 10. The page states its editing frame at page level, on `` + +Three attributes, on the body element of every document view that offers +editing: + +| Attribute | Meaning | +|---|---| +| `data-odr-editable="true" \| "readOnly"` | whether `enable()` can succeed at all | +| `data-odr-keyboard="navigation shortcuts"` | which key classes the scripts may take (decision 12) | + +Per element the page states only the exceptions: `data-odr-path` addresses an +editable run, and `odr-locked` plus `data-odr-lock=""` marks what +refuses. Everything unmarked is editable. + +**Why page level:** the frame is a fact about the document, not about a table. +`data-odr-editable` sat on the `.odr-sheet` element first, which answered the +question for the one view that had an editor and for no other. A `.docx` view +has no sheet to hang it on. + +**Why on `` and not in ``:** `document.body` is one lookup, the body +writer already exists in `html/document.cpp`, and a `` block would need a +name space of its own for two attributes. The cost is two attributes on one +element per view. + +**Why `"readOnly"` rather than an absent attribute:** an absent attribute cannot +be told from a page written by an older library. The page says which of the two +it means. + +### 11. `HtmlConfig::editable` writes the scaffolding; only JavaScript turns the mode on + +`editable` steers one thing: whether the render **offers** editing. True writes +the editing scripts, the page-level state and the per-element addressing. False +writes none of it, and the page has no `odr.editing`. + +The mode itself always starts **off**. A host that opens a document to edit it +calls `odr.editing.enable()` at the point it wires its callbacks, which is after +the page has loaded either way. + +**Why not let it steer the default state of the mode:** it would be a second +meaning on one flag, and it buys a host nothing. A host assigns +`odr.onEditRefused` and friends on the load event (decision 7 in +[`spreadsheet-editing.md`](spreadsheet-editing.md)), so `enable()` costs it one +more line at a point it already has. An attribute that opens the view in edit +mode would be the only way to skip that line, and nothing needs it. + +**Why not drop the flag and always write the scaffolding:** a read-only host +would carry the editor's script bytes and an attribute on every editable run for +nothing. `data-odr-path` on the runs of a text document is the expensive half, +and it cannot be added later — the mode can only turn on if the addresses are +already in the page. + +**Why it may still change the markup, when decision 3 of +[`spreadsheet-editing.md`](spreadsheet-editing.md) said it must not:** that +decision is about *switching modes*, and it stands — a user toggling the edit +button must not cost a second `translate`. Whether editing is offered at all is +a host's decision, made once before it renders. The two look alike and are not: +one is a gesture, the other is a build-time choice. + +**A document that cannot be edited still gets the scaffolding** where the config +asks for it, and states `data-odr-editable="readOnly"`. That is what lets a host +grey its button rather than discover the refusal after a tap. + +### 12. A host keeps the keys it needs + +The scripts take three classes of key event, and two of them are configurable: + +| Class | Keys | Config | +|---|---|---| +| The open editor's | Escape, Enter, Tab while an editor holds focus | always taken | +| Navigation | the arrows, Tab, Escape, and the keys that open the editor over the selection (Enter, F2, Delete, a character) | `HtmlConfig::keyboard_navigation` | +| Shortcuts | the chords: ctrl/cmd+Z, ctrl/cmd+Y, ctrl/cmd+shift+Z | `HtmlConfig::keyboard_shortcuts` | + +Both default to **on**, and the page states what it may take in +`data-odr-keyboard`. + +**Why configurable at all:** the sheet's key handler is registered in the +capture phase and calls `preventDefault`, so an arrow key never reaches the +embedder. A host with its own bindings — an app whose arrow keys page through +the document, a site whose iframe sits inside a keyboard-driven shell — loses +them with no way to ask for them back. + +**Why the open editor's keys are not configurable:** the editor holds focus, and +Escape and Enter are the only way out of it. A host that took them would leave +the user in an overlay nothing closes. + +**Why two classes and not one:** they fail differently. Navigation collides with +a host that moves a selection of its own; the chords collide with a host that +owns undo for the whole app, which is the common case on desktop and the rarer +one on mobile. + +**Why the page states it rather than the script asking the config:** the scripts +are static files embedded at build time (`cmake/frontend_assets.cmake`), so a +config value reaches them only through the markup. One attribute carries both +classes as a token list, and a third class appends to it without a fourth +attribute. + ## Preliminary implementation plan (ODF / OOXML) Ordered to de-risk the linchpin (id stability) first and to keep every step @@ -211,11 +367,22 @@ Sequence within Phase 2: (a) delete element, (b) toggle mark on a range, ### Phase 3 — Browser editor +The frame is landed (decisions 9 to 12): the mode, the refusals, the callbacks +and the keyboard classes are in `frontend/editing.js`, and `document.js` +attaches the skeleton editor to it. What is left is the editor itself. + 1. Model keyed by `data-odr-id`; `beforeinput`-intercepting op recorder; - composition-aware reconciliation path. -2. Browser-side undo/redo over the in-memory log; coalescing before emit. + composition-aware reconciliation path. This replaces the `contenteditable` + plus `MutationObserver` path `document.js` still uses. +2. Browser-side undo/redo over the in-memory log; coalescing before emit. The + text editor then answers `undo()` rather than refusing it, and its ops move + onto the shared log (decision 9). 3. Emit coalesced JSON to the WebView bridge; wire the native side to - `html::edit` + `save`. + `html::edit` + `save`. `odr.editing.getOperations()` is already the envelope + a host hands over. +4. Refusal reasons for text: a field, a link target, a subtree a write would + take away unseen. The code table is shared and appended to, never renumbered + (decision 7 in [`spreadsheet-editing.md`](spreadsheet-editing.md)). ### Phase 4 — Formatting UI + polish @@ -309,3 +476,13 @@ session-scoped, decision 4). anchors), or only on editable leaves? - Highlight in ODF/OOXML: character background vs. a highlight-specific property — which maps cleanly to a single toggle? +- `element_is_editable` answers one bool, and ooxml text answers `true` for + everything. A refusal needs a reason, because the reason is what a host puts + on a snackbar (decision 7 in + [`spreadsheet-editing.md`](spreadsheet-editing.md)). Does the adapter hook + grow into `element_edit_lock(id) -> reason`, or does the renderer keep + deciding the reason from the element it is over? +- The plain-text view (`html/text_file.cpp`) writes `contenteditable` on its + whole body under `config.editable`, and `txt` declares no `edit` capability — + so nothing collects or replays those edits. Decision 11 drops the attribute. + Does the source view get a real editor later, or stay a reader? diff --git a/docs/design/spreadsheet-editing.md b/docs/design/spreadsheet-editing.md index 9bcb53fa7..7d9059f62 100644 --- a/docs/design/spreadsheet-editing.md +++ b/docs/design/spreadsheet-editing.md @@ -49,8 +49,9 @@ results go stale the moment an input changes. | Number formats | — | Not parsed in either engine. ODS shows the producer's cached `text:p`; XLSX shows the raw `` (a date is its serial) | | Formulas | `sheet_cell_value` | The expression is read and handed out as a string (step 0.1, landed); nothing parses or evaluates it. XLSX shows the cached ``, ODS the cached `text:p`. `xls` and `numbers` drop the expression at parse time | | Browser: sheet script | `html/frontend/spreadsheet.js` | Hover/pin, raise a clipped cell over its neighbours, sort rows in the DOM. Sorting reorders ``s, so a row's identity is its `` label, not its index. Publishes `odr.sheet` (step 1.1, landed), and the value and reflow half of it (steps 1.2/1.3, landed) | -| Browser: editing script | `html/frontend/document.js` | A `MutationObserver` over `contenteditable` runs keyed by `data-odr-path`; `odr.generateDiff()` emits the envelope | -| Browser: sheet editor | `html/frontend/sheet-editing.js` | `odr.editing` with the mode, the locks and the refusals (step 1.1, landed), and the overlay that types into a cell (steps 1.2/1.3, landed). Undo/redo and `committed()` are step 1.4 | +| Browser: the mode | `html/frontend/editing.js` | `odr.editing` — the mode, the refusal table, the log a save reads and the `odr.on*` callbacks, generic over every format. An editor attaches to it ([`editing.md`](editing.md) decision 9, step 1.6, landed) | +| Browser: text editor | `html/frontend/document.js` | The skeleton, attached to the mode: `contenteditable` runs keyed by `data-odr-path`, a `MutationObserver`, one `setText` op per changed run. No undo | +| Browser: sheet editor | `html/frontend/sheet-editing.js` | The cell overlay, the locks and the position map (steps 1.1 to 1.4, landed), attached to the mode as one editor | | Wire format | `document.cpp::Document::edit` | The op envelope, `setCell` and `setText` (step 0.4, landed) | | Addressing | `DocumentPath` | Already spells a cell by position: `/child:0/cell:A1/...` | | Capabilities | `file_type_table.cpp` | `ods` and `xlsx` declare `edit` and `save` (step 0.2, landed); `csv` declares neither. `odr_test` checks the declaration against `Document::is_editable` | @@ -104,15 +105,15 @@ any decode of the same file. ### 3. Editing is a browser mode, not markup -`odr.editing.enable()` / `disable()` turns the mode on; `HtmlConfig::editable` -stops changing what a sheet writes. The page carries only what the browser -cannot work out for itself: +`odr.editing.enable()` / `disable()` turns the mode on, and switching it changes +nothing a sheet writes — the user never translates twice for it. The page +carries only what the browser cannot work out for itself: - a **lock** on a cell that cannot be edited, as a class plus its reason — `formula`, `rich` (several paragraphs, a link, a line break), `shapes` only where the cell is nothing but its anchored drawings; -- whether the **document** can be edited at all, one attribute on the table, - so `enable()` can refuse with a reason before the user clicks anything. +- whether the **document** can be edited at all, so `enable()` can refuse with + a reason before the user clicks anything. Everything else — including every empty cell — is editable. The cost is a class on the locked cells only, nothing on the half million others. @@ -128,6 +129,24 @@ read-only document, outlines it briefly and calls `odr.onEditRefused` so the host can say why — a snackbar on mobile. A silent no-op is the frustrating outcome the mode exists to avoid. Decision 7 is the channel. +**The mode itself is generic, and it moved.** This decision was written when the +sheet was the only editor, so `sheet-editing.js` held the mode, the refusal +table and the callbacks. Every format wants those, so they are +[`editing.md`](editing.md) decisions 9 to 12 now, and `frontend/editing.js` +holds them: + +- the sheet script **attaches** an editor to `odr.editing` and states no mode of + its own; +- the document's editable state is one attribute on ``, not on the + `.odr-sheet` table (decision 10) — which answers the open question below; +- `HtmlConfig::editable` writes the scaffolding, the lock classes included, and + a read-only render carries none of it (decision 11); +- the arrow keys and the undo chord are configurable, because the sheet takes + them in the capture phase and a host may need them (decision 12). + +What stays here is the sheet's own: the cell overlay, the locks and their +reasons, the position map, and the `setCell` op. + ### 4. The type follows the content The typed string is parsed by a strict grammar: optional sign, digits, one `.`, @@ -379,6 +398,13 @@ Each step ships on its own. "Both" means `.ods` and `.xlsx`. wasm example is the host-wiring reference for droid/ios. A view holds its own log, so the example writes it into the document when the view goes away as well as on save. +6. **Landed.** The mode is generic. `frontend/editing.js` owns `odr.editing` + and every format's editor attaches to it; the document's editable state moved + to ``; `HtmlConfig::editable` writes the scaffolding and a read-only + render carries none of it; `keyboard_navigation` and `keyboard_shortcuts` + let a host keep the arrows and the undo chord. See + [`editing.md`](editing.md) decisions 9 to 12 — a `.docx` view has the same + `odr.editing` a sheet does, with the text skeleton behind it. ### Step 2 — Materialise the cells that are not there @@ -536,7 +562,8 @@ Ordered by value over cost; all in step 0 or 1. the compromise for step 1. - Where does the document locale come from for the decimal separator — `settings.xml`, the number format, the host? -- Does the read-only document attribute belong on the table or in a - page-level `data-odr-*` block the text editor will want too? +- **Answered** ([`editing.md`](editing.md) decision 10): the page-level block, + as `data-odr-editable` on ``. The frame is a fact about the document, + and a `.docx` view has no table to hang it on. - Should the refusal codes be generated from one C++ table so the bindings can hand a host the same list, rather than living only in the emitted script? From 68fd4a89965de5ded5b10dcde446187d2000c523 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Thu, 10 Sep 2026 09:25:39 +0200 Subject: [PATCH 02/11] feat(html): make editing one mode every format shares The mode, the refusal channel and the dirty flag are what a host wires, and it wires them once per document, so they cannot sit in the sheet's script. `frontend/editing.js` owns `odr.editing` for every document view and each format attaches its own editor: the cell overlay for a sheet, the runs for a text document. A `.docx` view answers `isEditable()` the way an `.ods` already did, so a host can grey its edit button before a tap. `HtmlConfig::editable` writes the scaffolding the mode needs rather than `contenteditable`: the addressing an op names, the lock on a locked cell, the state on ``, and the editor script. The mode writes `contenteditable` on the addressed runs when the host turns it on, so switching modes needs no second render, and a read-only render carries none of it. `keyboard_navigation` and `keyboard_shortcuts` let a host keep the arrow keys and the undo chord, which the sheet takes in the capture phase. The open editor's own keys are never taken away, because they are the way out of it. Also fixes two things the split exposed: a read-only text document took the Enter key from the reader and reported an error for it, and a sheet page swallowed Enter whenever no cell was pinned. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KKFKbUVCYF2VhujdmjhhPW --- CHANGELOG.md | 36 +++ CMakeLists.txt | 1 + apple/include/OdrCoreObjC/ODRHtml.h | 4 + apple/src/ODRHtml.mm | 4 + docs/design/editing.md | 30 ++- .../app/opendocument/core/HtmlConfig.java | 5 + jni/src/jni_style.cpp | 4 + python/src/bind_html.cpp | 3 + python/tests/test_html.py | 4 + src/odr/html.hpp | 11 +- src/odr/internal/html/document.cpp | 39 ++- src/odr/internal/html/document_element.cpp | 22 +- src/odr/internal/html/frontend.cpp | 16 +- src/odr/internal/html/frontend.hpp | 11 +- src/odr/internal/html/frontend/document.js | 68 ++++- src/odr/internal/html/frontend/editing.js | 228 ++++++++++++++++ .../internal/html/frontend/sheet-editing.js | 248 ++++++------------ src/odr/internal/html/frontend/spreadsheet.js | 14 +- src/odr/internal/html/text_file.cpp | 3 + test/browser/sheet/README.md | 19 +- test/browser/sheet/editing.html | 5 +- test/browser/sheet/keyboard.html | 93 +++++++ test/browser/sheet/positions.html | 5 +- test/browser/sheet/serve | 12 +- test/browser/sheet/sorting.html | 5 +- test/browser/sheet/tests.html | 3 +- test/src/html_test.cpp | 112 ++++++-- wasm/README.md | 19 +- wasm/example/index.html | 6 +- wasm/js/index.d.ts | 2 + wasm/src/wasm_html.cpp | 2 + wasm/tests/edit.test.mjs | 2 - wasm/tests/render.test.mjs | 6 +- 33 files changed, 780 insertions(+), 262 deletions(-) create mode 100644 src/odr/internal/html/frontend/editing.js create mode 100644 test/browser/sheet/keyboard.html diff --git a/CHANGELOG.md b/CHANGELOG.md index fd61f4184..7d02dd727 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,42 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- `odr.editing` is on every document view, not only on a sheet's. The mode, + the refusals, the log a save reads and the `odr.onEdit*` callbacks are one + generic surface a host wires once per document, and each format attaches its + own editor to it — the cell overlay for a sheet, the runs for a text + document. `isEditable()` is what greys a host's edit button: it answers for a + `.docx` the way it already answered for an `.ods`. + +- **Breaking**: `HtmlConfig::editable` no longer writes `contenteditable`. It + writes the scaffolding the mode needs — `data-odr-path` on every editable + run, the lock class on a locked cell, the page's editing state on ``, + and the editor script — and the mode writes `contenteditable` on those runs + when a host calls `odr.editing.enable()`. A host that rendered with + `editable` and expected the browser to edit the page has to turn the mode on. + Switching modes needs no second render. The plain-text source view is + unchanged: `text.js` is its own editor and keeps its `contenteditable`. + +- **Breaking**: a render with `editable` off carries no editing markup at all — + no `data-odr-editable`, no `data-odr-lock`, no `data-odr-path`. A read-only + view pays nothing for an editor it has no way to reach. + +- **Breaking**: a refused new line reaches `odr.onEditRefused` with reason + `newLine`, rather than `odr.onError`. It keeps code 1, and it now fires only + inside an editable run while the mode is on — a read-only page took the Enter + key from the reader and reported an error for it. + +- `HtmlConfig::keyboard_navigation` and `keyboard_shortcuts` decide whether the + page's scripts take the keys that move the selection (the arrows, Tab, + Escape, and the keys that open an editor over it) and the editing chords + (undo and redo). Both default to on. A host with its own bindings turns them + off and keeps the keys; the open editor's own keys — Escape, Enter, Tab — are + never taken away, because they are the only way out of it. + +- The document's editable state moved from the `.odr-sheet` table to ``, + as `data-odr-editable`, beside the new `data-odr-keyboard`. The table keeps + `data-odr-sheet`. + - A cell of several runs is written rather than locked: the write replaces what the cell shows with one run. A cell holding one run is written through it, so that run keeps its style. The `rich` lock stays on what a write would take diff --git a/CMakeLists.txt b/CMakeLists.txt index fbdfcefd5..a2e96342b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -104,6 +104,7 @@ set(ODR_FRONTEND_ASSETS "search.css" "search-dark.css" "document.js" + "editing.js" "search.js" "spreadsheet.js" "sheet-editing.js" diff --git a/apple/include/OdrCoreObjC/ODRHtml.h b/apple/include/OdrCoreObjC/ODRHtml.h index a904a1454..86386781f 100644 --- a/apple/include/OdrCoreObjC/ODRHtml.h +++ b/apple/include/OdrCoreObjC/ODRHtml.h @@ -83,6 +83,10 @@ NS_SWIFT_NAME(HtmlConfig) @property(nonatomic) BOOL relativeResourcePaths; @property(nonatomic) BOOL editable; +/// Whether the view's scripts take the keys that move the selection. +@property(nonatomic) BOOL keyboardNavigation; +/// Whether the view's scripts take the editing chords: undo and redo. +@property(nonatomic) BOOL keyboardShortcuts; @property(nonatomic) BOOL textDocumentMargin; @property(nonatomic) ODRHtmlColorScheme colorScheme; diff --git a/apple/src/ODRHtml.mm b/apple/src/ODRHtml.mm index 0c182b0e0..4a6bcac45 100644 --- a/apple/src/ODRHtml.mm +++ b/apple/src/ODRHtml.mm @@ -89,6 +89,8 @@ - (instancetype)initWithNativeConfig:(const odr::HtmlConfig &)config { _resourcePath = to_nsstring(config.resource_path); _relativeResourcePaths = config.relative_resource_paths ? YES : NO; _editable = config.editable ? YES : NO; + _keyboardNavigation = config.keyboard_navigation ? YES : NO; + _keyboardShortcuts = config.keyboard_shortcuts ? YES : NO; _textDocumentMargin = config.text_document_margin ? YES : NO; _colorScheme = static_cast(config.color_scheme); if (config.spreadsheet_limit.has_value()) { @@ -156,6 +158,8 @@ - (instancetype)initWithNativeConfig:(const odr::HtmlConfig &)config { } config.relative_resource_paths = _relativeResourcePaths == YES; config.editable = _editable == YES; + config.keyboard_navigation = _keyboardNavigation == YES; + config.keyboard_shortcuts = _keyboardShortcuts == YES; config.text_document_margin = _textDocumentMargin == YES; config.color_scheme = static_cast(_colorScheme); if (_spreadsheetLimit != nil) { diff --git a/docs/design/editing.md b/docs/design/editing.md index 5fc91d519..14eb67762 100644 --- a/docs/design/editing.md +++ b/docs/design/editing.md @@ -256,13 +256,25 @@ it means. ### 11. `HtmlConfig::editable` writes the scaffolding; only JavaScript turns the mode on `editable` steers one thing: whether the render **offers** editing. True writes -the editing scripts, the page-level state and the per-element addressing. False -writes none of it, and the page has no `odr.editing`. +the per-element addressing, the page-level editable state and the editor script +of the format at hand. False writes none of those. The mode itself always starts **off**. A host that opens a document to edit it calls `odr.editing.enable()` at the point it wires its callbacks, which is after the page has loaded either way. +**`odr.editing` is on every document view either way**, because `editing.js` is +written unconditionally. On a page with no scaffolding it answers +`isEditable() === false`, `enable()` refuses with `readOnly`, and +`getOperations()` hands out an empty envelope. So a host asks the page rather +than tracking what it rendered with, and `odr.generateDiff()` — the name the +apps and the wasm package already call — never goes missing. + +What the flag keeps out of a read-only render is what actually costs: the +`data-odr-path` attribute on every editable run, the lock class on every locked +cell, and the editor script (`document.js`, `sheet-editing.js`). The mode script +itself is small and buys the host one API for every format. + **Why not let it steer the default state of the mode:** it would be a second meaning on one flag, and it buys a host nothing. A host assigns `odr.onEditRefused` and friends on the load event (decision 7 in @@ -276,6 +288,10 @@ nothing. `data-odr-path` on the runs of a text document is the expensive half, and it cannot be added later — the mode can only turn on if the addresses are already in the page. +**Why the keyboard classes are stated either way:** they are not an editing +fact. A read-only sheet has a pinned cell, and Escape clears it +(`spreadsheet.js`), so `data-odr-keyboard` is written on every document view. + **Why it may still change the markup, when decision 3 of [`spreadsheet-editing.md`](spreadsheet-editing.md) said it must not:** that decision is about *switching modes*, and it stands — a user toggling the edit @@ -482,7 +498,9 @@ session-scoped, decision 4). [`spreadsheet-editing.md`](spreadsheet-editing.md)). Does the adapter hook grow into `element_edit_lock(id) -> reason`, or does the renderer keep deciding the reason from the element it is over? -- The plain-text view (`html/text_file.cpp`) writes `contenteditable` on its - whole body under `config.editable`, and `txt` declares no `edit` capability — - so nothing collects or replays those edits. Decision 11 drops the attribute. - Does the source view get a real editor later, or stay a reader? +- The plain-text view (`html/text_file.cpp`) is outside the mode: `text.js` is + its own editor, with its own `beforeinput` interception and its own undo, and + `config.editable` writes the `contenteditable` it needs. Nothing replays those + edits into a file, because `txt` declares no `edit` capability. Does that view + attach to `odr.editing` — which would need an editable-but-not-savable state — + or stay the one editor that answers to nobody? diff --git a/jni/java/app/opendocument/core/HtmlConfig.java b/jni/java/app/opendocument/core/HtmlConfig.java index 2c82a38c1..45e90b41e 100644 --- a/jni/java/app/opendocument/core/HtmlConfig.java +++ b/jni/java/app/opendocument/core/HtmlConfig.java @@ -21,6 +21,11 @@ public final class HtmlConfig { public boolean editable = false; + /** Whether the view's scripts take the keys that move the selection. */ + public boolean keyboardNavigation = true; + /** Whether the view's scripts take the editing chords: undo and redo. */ + public boolean keyboardShortcuts = true; + public boolean textDocumentMargin = false; /** The colors a document renders against. */ diff --git a/jni/src/jni_style.cpp b/jni/src/jni_style.cpp index 4e937c693..f5bfc18b1 100644 --- a/jni/src/jni_style.cpp +++ b/jni/src/jni_style.cpp @@ -453,6 +453,8 @@ jobject html_config_to_java(JNIEnv *env, const odr::HtmlConfig &config) { set_string("resourcePath", config.resource_path); set_boolean("relativeResourcePaths", config.relative_resource_paths); set_boolean("editable", config.editable); + set_boolean("keyboardNavigation", config.keyboard_navigation); + set_boolean("keyboardShortcuts", config.keyboard_shortcuts); set_boolean("textDocumentMargin", config.text_document_margin); set_object("colorScheme", "Lapp/opendocument/core/HtmlColorScheme;", enum_from_code(env, "app/opendocument/core/HtmlColorScheme", @@ -573,6 +575,8 @@ odr::HtmlConfig html_config_from_java(JNIEnv *env, jobject config) { } result.relative_resource_paths = get_boolean("relativeResourcePaths"); result.editable = get_boolean("editable"); + result.keyboard_navigation = get_boolean("keyboardNavigation"); + result.keyboard_shortcuts = get_boolean("keyboardShortcuts"); result.text_document_margin = get_boolean("textDocumentMargin"); { const jint code = enum_ordinal( diff --git a/python/src/bind_html.cpp b/python/src/bind_html.cpp index 9aea125cf..01b6b5444 100644 --- a/python/src/bind_html.cpp +++ b/python/src/bind_html.cpp @@ -79,6 +79,9 @@ void odr_python::bind_html(py::module_ &m) { .def_readwrite("relative_resource_paths", &odr::HtmlConfig::relative_resource_paths) .def_readwrite("editable", &odr::HtmlConfig::editable) + .def_readwrite("keyboard_navigation", + &odr::HtmlConfig::keyboard_navigation) + .def_readwrite("keyboard_shortcuts", &odr::HtmlConfig::keyboard_shortcuts) .def_readwrite("text_document_margin", &odr::HtmlConfig::text_document_margin) .def_readwrite("color_scheme", &odr::HtmlConfig::color_scheme) diff --git a/python/tests/test_html.py b/python/tests/test_html.py index 343350f4e..fe13ee004 100644 --- a/python/tests/test_html.py +++ b/python/tests/test_html.py @@ -15,12 +15,16 @@ def test_html_config_defaults(): config = pyodr.HtmlConfig() assert config.embed_images assert not config.editable + assert config.keyboard_navigation + assert config.keyboard_shortcuts assert config.spreadsheet_gridlines == pyodr.HtmlTableGridlines.soft config.editable = True + config.keyboard_navigation = False config.format_html = True config.spreadsheet_limit = pyodr.TableDimensions(100, 100) assert config.editable + assert not config.keyboard_navigation assert config.spreadsheet_limit.rows == 100 assert config.spreadsheet_cell_limit == 500000 diff --git a/src/odr/html.hpp b/src/odr/html.hpp index 4e2922632..e90439bce 100644 --- a/src/odr/html.hpp +++ b/src/odr/html.hpp @@ -129,9 +129,18 @@ struct HtmlConfig { /// output stays movable. bool relative_resource_paths{true}; - /// Write `contenteditable` output, which back-translation reads edits from. + /// Write the editing scaffolding: the page's editing state, the addressing + /// an edit operation names, and the scripts that carry `odr.editing`. The + /// mode itself always starts off - the host turns it on from JavaScript. bool editable{false}; + /// Whether the view's scripts take the keys that move the selection - the + /// arrows, Tab, Escape, and the keys that open an editor over it. Off leaves + /// every one of them to the host. + bool keyboard_navigation{true}; + /// Whether the view's scripts take the editing chords: undo and redo. + bool keyboard_shortcuts{true}; + /// Render a text document as fixed-size pages rather than reflowing text. bool text_document_margin{false}; diff --git a/src/odr/internal/html/document.cpp b/src/odr/internal/html/document.cpp index 4d817dd41..783b6e4cb 100644 --- a/src/odr/internal/html/document.cpp +++ b/src/odr/internal/html/document.cpp @@ -198,6 +198,22 @@ void write_head(const Document &document, const WritingState &state, out.write_header_end(); } +/// The key classes the view's scripts may take, as the page states them. +/// `editing.md` decision 12 names them. +std::string keyboard_classes(const HtmlConfig &config) { + std::string classes; + if (config.keyboard_navigation) { + classes += "navigation"; + } + if (config.keyboard_shortcuts) { + if (!classes.empty()) { + classes += " "; + } + classes += "shortcuts"; + } + return classes; +} + void write_body_begin(const Document &document, const WritingState &state) { HtmlWriter &out = state.out(); @@ -222,7 +238,20 @@ void write_body_begin(const Document &document, const WritingState &state) { } } - out.write_body_begin(HtmlElementOptions().set_class(body_clazz)); + out.write_body_begin( + HtmlElementOptions() + .set_class(body_clazz) + .set_attributes([&](const HtmlAttributeWriterCallback &clb) { + // what the mode answers before the user clicks anything; stated + // only where the render offers editing at all + if (state.config().editable) { + clb("data-odr-editable", + state.document_editable() ? "true" : "readOnly"); + } + // not an editing fact: a read-only sheet has a pin, and Escape + // clears it + clb("data-odr-keyboard", keyboard_classes(state.config())); + })); if (paged_content) { out.write_element_begin("div", HtmlElementOptions().set_class("odr-pages")); @@ -237,9 +266,15 @@ void write_body_end(const Document &document, const WritingState &state) { } write_search_script(state); - write_document_script(state); + write_editing_script(state); + if (state.config().editable) { + write_document_script(state); + } if (document.document_type() == DocumentType::spreadsheet) { write_spreadsheet_script(state); + if (state.config().editable) { + write_sheet_editing_script(state); + } } write_viewport_script(state); diff --git a/src/odr/internal/html/document_element.cpp b/src/odr/internal/html/document_element.cpp index 5cc9bde5b..330ab4f9a 100644 --- a/src/odr/internal/html/document_element.cpp +++ b/src/odr/internal/html/document_element.cpp @@ -233,11 +233,11 @@ std::optional sheet_print_fit(const Sheet &sheet, return printable / content; } -/// A run whose style the box around it can carry instead. Not a background, a -/// raised run or an editable one: each means something else on the box. -/// Whether @p element carries `contenteditable`: an editable run of a view -/// that writes its editing into the markup. -bool writes_editable(const Element &element, const html::WritingState &state) { +/// Whether @p element carries the addressing an edit operation names: an +/// editable run of a view whose editing is per run. `contenteditable` is not +/// written - the mode adds it to these runs when a host turns it on. +bool writes_edit_markup(const Element &element, + const html::WritingState &state) { return state.editable_markup() && state.config().editable && element.is_editable(); } @@ -247,7 +247,7 @@ std::optional plain_text(const Element &element, if (element.type() != ElementType::text) { return {}; } - if (writes_editable(element, state)) { + if (writes_edit_markup(element, state)) { return {}; } @@ -487,9 +487,6 @@ void html::translate_sheet(const Sheet &sheet, const WritingState &state) { HtmlElementOptions() .set_class("odr-sheet") .set_attributes([&](const HtmlAttributeWriterCallback &clb) { - // what the editor asks before the user clicks anything - clb("data-odr-editable", - state.document_editable() ? "true" : "readOnly"); // every op names its sheet, and a view holds only one clb("data-odr-sheet", std::to_string(sheet_ordinal(sheet))); }) @@ -669,7 +666,9 @@ void html::translate_sheet(const Sheet &sheet, const WritingState &state) { const std::optional folded = fold_cell( cell, sheet_state, wraps, anchors_shapes, table_row_style.height); - const char *lock = cell_lock(cell, anchors_shapes); + // scaffolding: a render that offers no editing states no lock + const char *lock = + state.config().editable ? cell_lock(cell, anchors_shapes) : nullptr; state.out().write_element_begin( "td", @@ -778,8 +777,7 @@ void html::translate_text(const Element &element, const WritingState &state) { HtmlElementOptions() .set_inline(true) .set_attributes([&](const HtmlAttributeWriterCallback &clb) { - if (writes_editable(element, state)) { - clb("contenteditable", "true"); + if (writes_edit_markup(element, state)) { clb("data-odr-path", element.document_path().to_string()); } }) diff --git a/src/odr/internal/html/frontend.cpp b/src/odr/internal/html/frontend.cpp index 2ad5777c8..ae41f46d0 100644 --- a/src/odr/internal/html/frontend.cpp +++ b/src/odr/internal/html/frontend.cpp @@ -87,6 +87,10 @@ constexpr Asset search_dark_css_asset{HtmlResourceType::css, "text/css", frontend_assets::search_dark_css}; constexpr Asset document_js_asset{HtmlResourceType::js, "text/javascript", "document.js", frontend_assets::document_js}; +/// `odr.editing`: the mode, the refusals and the log a save reads, generic +/// over every format. An editor script attaches itself to it. +constexpr Asset editing_js_asset{HtmlResourceType::js, "text/javascript", + "editing.js", frontend_assets::editing_js}; /// Text search over the rendered page, format-agnostic: it walks text nodes. constexpr Asset search_js_asset{HtmlResourceType::js, "text/javascript", "search.js", frontend_assets::search_js}; @@ -96,9 +100,8 @@ constexpr Asset search_js_asset{HtmlResourceType::js, "text/javascript", constexpr Asset spreadsheet_js_asset{HtmlResourceType::js, "text/javascript", "spreadsheet.js", frontend_assets::spreadsheet_js}; -/// `odr.editing`: the mode, and the refusals the page reports to its host. -/// A sheet's editing is an overlay, so the markup states only what the page -/// cannot work out - the document's editability and a locked cell's reason. +/// The cell overlay, and the locks the markup states because the page cannot +/// work them out: a sheet's editing is an overlay, not `contenteditable`. constexpr Asset sheet_editing_js_asset{HtmlResourceType::js, "text/javascript", "sheet-editing.js", frontend_assets::sheet_editing_js}; @@ -257,6 +260,10 @@ void html::write_search_dark_style(const WritingState &state) { write_dark_style(search_dark_css_asset, state); } +void html::write_editing_script(const WritingState &state) { + write_script(editing_js_asset, state); +} + void html::write_document_script(const WritingState &state) { write_script(document_js_asset, state); } @@ -267,6 +274,9 @@ void html::write_search_script(const WritingState &state) { void html::write_spreadsheet_script(const WritingState &state) { write_script(spreadsheet_js_asset, state); +} + +void html::write_sheet_editing_script(const WritingState &state) { write_script(sheet_editing_js_asset, state); } diff --git a/src/odr/internal/html/frontend.hpp b/src/odr/internal/html/frontend.hpp index 3ad2672cb..fb2064cb5 100644 --- a/src/odr/internal/html/frontend.hpp +++ b/src/odr/internal/html/frontend.hpp @@ -36,10 +36,19 @@ void write_search_dark_style(const WritingState &state); bool writes_dark_style(const HtmlConfig &config); std::string_view dark_style_media(const HtmlConfig &config); -/// The `odr` object a document view exposes to its host: `generateDiff()`. +/// `odr.editing`: the editing mode every format's editor attaches to, plus +/// `odr.onError`, `odr.takesKeys` and `odr.generateDiff`. Written by every +/// document view, and **before** the editor scripts and @ref +/// write_spreadsheet_script, which read `odr.takesKeys`. +void write_editing_script(const WritingState &state); +/// The text editor, attached to the mode: the runs the markup addresses with +/// `data-odr-path`. Written where the config offers editing. void write_document_script(const WritingState &state); /// Written in addition to the document script. void write_spreadsheet_script(const WritingState &state); +/// The sheet's editor, attached to the mode. Written in addition to @ref +/// write_spreadsheet_script, where the config offers editing. +void write_sheet_editing_script(const WritingState &state); void write_text_script(const WritingState &state); /// `odr.search()`, `searchNext()`, `searchPrevious()`, `resetSearch()` — the /// rest of that object, for every view rendering text, whatever the format. diff --git a/src/odr/internal/html/frontend/document.js b/src/odr/internal/html/frontend/document.js index eec61ab19..f16de0eaf 100644 --- a/src/odr/internal/html/frontend/document.js +++ b/src/odr/internal/html/frontend/document.js @@ -1,30 +1,46 @@ +// The text editor, attached to `odr.editing` as one editor among the formats. +// Still the skeleton `editing.md` phase 3 replaces: the browser edits the runs +// and a `MutationObserver` reads back what changed, so there is no selection +// model, no mark and no undo of our own. (function () { "use strict"; var odr = (window.odr = window.odr || {}); - odr.onError = function (code, message) { - console.error("error " + code + " message " + message); - }; - - var errorIllegalEditNewLine = { - code: 1, - message: "new line not supported by this document", - }; + var runs = document.querySelectorAll("[data-odr-path]"); + if (runs.length === 0) { + return; + } var modified = {}; - odr.generateDiff = function () { + function operations() { var ops = []; for (var path in modified) { if (Object.prototype.hasOwnProperty.call(modified, path)) { ops.push({ op: "setText", path: path, text: modified[path].innerText }); } } - return JSON.stringify({ version: 1, ops: ops }); - }; + return ops; + } + + /// The mode writes `contenteditable`; the markup carries the address alone, + /// so the same page serves both modes. + function editable(on) { + for (var i = 0; i < runs.length; ++i) { + if (on) { + runs[i].setAttribute("contenteditable", "true"); + } else { + runs[i].removeAttribute("contenteditable"); + } + } + } new MutationObserver(function (mutations) { + if (!odr.editing.isEnabled()) { + return; + } + var moved = false; for (var i = 0; i < mutations.length; ++i) { if (mutations[i].type !== "characterData") { continue; @@ -35,18 +51,42 @@ var owner = parent && parent.closest("[data-odr-path]"); if (owner) { modified[owner.getAttribute("data-odr-path")] = owner; + moved = true; } } + if (moved) { + odr.editing.changed(); + } }).observe(document.body, { childList: true, subtree: true, characterData: true, }); + // A run is one line, so a new line inside it has nowhere to go in the file. document.addEventListener("keydown", function (event) { - if (event.key === "Enter") { - event.preventDefault(); - odr.onError(errorIllegalEditNewLine.code, errorIllegalEditNewLine.message); + if (!odr.editing.isEnabled() || event.key !== "Enter") { + return; + } + var target = event.target; + var owner = target && target.closest && target.closest("[data-odr-path]"); + if (owner === null || owner === undefined) { + return; } + event.preventDefault(); + odr.editing.refuse("newLine", { path: owner.getAttribute("data-odr-path") }); + }); + + odr.editing.attach({ + enable: function () { + editable(true); + }, + disable: function () { + editable(false); + }, + operations: operations, + committed: function () { + modified = {}; + }, }); })(); diff --git a/src/odr/internal/html/frontend/editing.js b/src/odr/internal/html/frontend/editing.js new file mode 100644 index 000000000..c3bacfbfe --- /dev/null +++ b/src/odr/internal/html/frontend/editing.js @@ -0,0 +1,228 @@ +// `odr.editing`: the editing mode, and nothing about any one format. A host +// wires this once for every document it opens; a format's editor attaches to +// it. See `docs/design/editing.md` decisions 9 to 12. +(function () { + "use strict"; + + var odr = (window.odr = window.odr || {}); + var body = document.body; + + // The frame the page states. An absent `data-odr-editable` is a render that + // offers no editing, which is not the same as a document that refuses one - + // both answer `readOnly`, and neither can be told apart by a host that only + // ever asks whether it may edit. + var editable = body.getAttribute("data-odr-editable") === "true"; + var keyClasses = (body.getAttribute("data-odr-keyboard") || "").split(" "); + + var editing = false; + var editors = []; + var lastRefusal = null; + + // One space of codes, appended and never renumbered: a host maps the code to + // its own wording, and the message is for a developer who wires nothing. + var refusals = { + newLine: { code: 1, message: "new line not supported by this document" }, + formula: { code: 2, message: "cell holds a formula" }, + rich: { code: 3, message: "cell holds more than one plain run" }, + shapes: { code: 4, message: "cell holds a drawing" }, + readOnly: { code: 5, message: "document cannot be edited" }, + formulaInput: { code: 6, message: "typing a formula is not supported" }, + }; + + odr.onError = function (code, message) { + console.error("error " + code + " message " + message); + }; + odr.onEditRefused = function (event) { + console.warn("edit refused " + event.code + ": " + event.message); + }; + odr.onEditModeChange = function (event) { + console.log("editing " + (event.editing ? "on" : "off")); + }; + odr.onEditChange = function () {}; + + function fire(name, event) { + if (typeof odr[name] === "function") { + odr[name](event); + } + } + + /// Whether the view's scripts take a class of key event: `navigation` for + /// the keys that move the selection, `shortcuts` for the chords. A host that + /// owns the keyboard turns them off in `HtmlConfig`. + odr.takesKeys = function (name) { + return keyClasses.indexOf(name) !== -1; + }; + + /// Calls @p name on every attached editor, and answers whether one of them + /// said yes. `undo` and `redo` lean on the order: the editor attached last + /// is asked first, so the editor a page put on top answers for it. + function ask(name) { + for (var i = editors.length - 1; i >= 0; --i) { + var editor = editors[i]; + if (typeof editor[name] === "function" && editor[name]()) { + return true; + } + } + return false; + } + + function tell(name) { + for (var i = 0; i < editors.length; ++i) { + if (typeof editors[i][name] === "function") { + editors[i][name](); + } + } + } + + /// The ops every editor would hand a save, in the order they attached. + function operations() { + var ops = []; + for (var i = 0; i < editors.length; ++i) { + var mine = editors[i].operations(); + for (var j = 0; j < mine.length; ++j) { + ops.push(mine[j]); + } + } + return ops; + } + + function modeChange(reason) { + fire("onEditModeChange", { + editing: editing, + editable: editable, + reason: reason || null, + code: reason ? refusals[reason].code : 0, + message: reason ? refusals[reason].message : "", + }); + } + + odr.editing = { + /// Answers whether the mode is on. A render that offers no editing, and a + /// document that cannot be edited, both refuse and say why - so a host can + /// grey its button before a click. + enable: function () { + if (!editable) { + modeChange("readOnly"); + return false; + } + if (!editing) { + editing = true; + body.classList.add("odr-editing"); + tell("enable"); + modeChange(null); + } + return true; + }, + disable: function () { + if (editing) { + editing = false; + tell("disable"); + body.classList.remove("odr-editing"); + modeChange(null); + } + }, + isEnabled: function () { + return editing; + }, + /// Whether `enable` would succeed. + isEditable: function () { + return editable; + }, + + /// Adds one format's editor to the mode. Only `operations` is required; + /// `enable`, `disable`, `undo`, `redo`, `canUndo`, `canRedo` and + /// `committed` are answered for the editor where it states none. + attach: function (editor) { + editors.push(editor); + }, + + /// Reports a refused edit, and drops it where the page just reported the + /// same one: tapping a locked cell four times is one snackbar. @p detail + /// carries what the format addresses the refusal by - a sheet its + /// position. Painting the refusal is the editor's, because what an outline + /// goes around differs per format. + refuse: function (reason, detail) { + var refusal = refusals[reason] || refusals.readOnly; + var key = reason + ":" + JSON.stringify(detail || null); + var now = Date.now(); + if (lastRefusal !== null && lastRefusal.key === key) { + if (now - lastRefusal.at < 2000) { + return; + } + } + lastRefusal = { key: key, at: now }; + var event = { reason: reason, code: refusal.code, message: refusal.message }; + for (var field in detail) { + if (Object.prototype.hasOwnProperty.call(detail, field)) { + event[field] = detail[field]; + } + } + fire("onEditRefused", event); + }, + + /// What a host's save button and back-press warning read. An editor calls + /// this whenever its log moved. + changed: function () { + var count = operations().length; + fire("onEditChange", { + dirty: count > 0, + operations: count, + canUndo: ask("canUndo"), + canRedo: ask("canRedo"), + }); + }, + + /// The envelope a host hands to `Document::edit` before saving. + getOperations: function () { + return JSON.stringify({ version: 1, ops: operations() }); + }, + + /// Takes the last edit back; false where no editor has one. + undo: function () { + return ask("undo"); + }, + redo: function () { + return ask("redo"); + }, + + /// The host saved the log: the page and the file agree, and undo starts + /// over. + committed: function () { + tell("committed"); + odr.editing.changed(); + }, + }; + + /// The name the apps and the wasm package already call. Same envelope. + odr.generateDiff = function () { + return odr.editing.getOperations(); + }; + + /// The undo chord, for whichever editor answers it. A form field keeps its + /// own text undo, and a key no editor took is left alone - so a run the + /// browser edits keeps the undo the browser gives it until the text editor + /// has a log of its own. + function chordKey(event) { + if (!editing || event.altKey || !(event.ctrlKey || event.metaKey)) { + return; + } + var target = event.target; + if (target && /^(INPUT|TEXTAREA|SELECT)$/.test(target.tagName)) { + return; + } + var chord = event.key.toLowerCase(); + if (chord !== "z" && chord !== "y") { + return; + } + var taken = + chord === "y" || event.shiftKey ? odr.editing.redo() : odr.editing.undo(); + if (taken) { + event.stopPropagation(); + event.preventDefault(); + } + } + + if (odr.takesKeys("shortcuts")) { + document.addEventListener("keydown", chordKey, true); + } +})(); diff --git a/src/odr/internal/html/frontend/sheet-editing.js b/src/odr/internal/html/frontend/sheet-editing.js index 02ee00b7e..ec3ce6c4a 100644 --- a/src/odr/internal/html/frontend/sheet-editing.js +++ b/src/odr/internal/html/frontend/sheet-editing.js @@ -1,3 +1,6 @@ +// The sheet's editor, attached to `odr.editing` as one editor among the +// formats: the cell overlay, the locks and the `setCell` op. The mode itself, +// the refusal channel and the log a save reads are `editing.js`. (function () { "use strict"; @@ -9,34 +12,6 @@ var odr = (window.odr = window.odr || {}); var sheet = Number(table.getAttribute("data-odr-sheet") || 0); - var editable = table.getAttribute("data-odr-editable") === "true"; - var editing = false; - var lastRefusal = null; - - // One space with `odr.onError`'s codes, appended and never renumbered - 1 is - // `errorIllegalEditNewLine`. The host maps the code to its own wording; the - // message is for a developer who wires nothing. - var refusals = { - formula: { code: 2, message: "cell holds a formula" }, - rich: { code: 3, message: "cell holds more than one plain run" }, - shapes: { code: 4, message: "cell holds a drawing" }, - readOnly: { code: 5, message: "document cannot be edited" }, - formulaInput: { code: 6, message: "typing a formula is not supported" }, - }; - - odr.onEditRefused = function (event) { - console.warn("edit refused " + event.code + ": " + event.message); - }; - odr.onEditModeChange = function (event) { - console.log("editing " + (event.editing ? "on" : "off")); - }; - odr.onEditChange = function () {}; - - function fire(name, event) { - if (typeof odr[name] === "function") { - odr[name](event); - } - } var outlined = null; var outlinedTimer = 0; @@ -58,89 +33,13 @@ }, 700); } - /// Four taps on a locked cell are one snackbar: the same refusal within two - /// seconds of the last is the page's to drop. The outline answers each. + /// Four taps on a locked cell are one snackbar, which `odr.editing` decides. + /// The outline answers each. function refuse(reason, column, row) { - var refusal = refusals[reason] || refusals.readOnly; - var key = reason + ":" + column + ":" + row; - var now = Date.now(); outline(odr.sheet.cellAt(column, row)); - if (lastRefusal && lastRefusal.key === key && now - lastRefusal.at < 2000) { - return; - } - lastRefusal = { key: key, at: now }; - fire("onEditRefused", { - sheet: sheet, - column: column, - row: row, - reason: reason, - code: refusal.code, - message: refusal.message, - }); - } - - function modeChange(reason) { - fire("onEditModeChange", { - editing: editing, - editable: editable, - reason: reason || null, - code: reason ? refusals[reason].code : 0, - message: reason ? refusals[reason].message : "", - }); + odr.editing.refuse(reason, { sheet: sheet, column: column, row: row }); } - odr.editing = { - /// Answers whether the mode is on. A document that cannot be edited - /// refuses and says why, so a host can grey its button before a click. - enable: function () { - if (!editable) { - modeChange("readOnly"); - return false; - } - if (!editing) { - editing = true; - table.classList.add("odr-editing"); - modeChange(null); - } - return true; - }, - disable: function () { - if (editing) { - editing = false; - close(); - table.classList.remove("odr-editing"); - modeChange(null); - } - }, - isEnabled: function () { - return editing; - }, - /// Whether `enable` would succeed. - isEditable: function () { - return editable; - }, - /// The lock on the cell at (@p column, @p row), or null where it has none. - lockAt: function (column, row) { - var cell = odr.sheet.cellAt(column, row); - return cell === null ? null : cell.getAttribute("data-odr-lock"); - }, - }; - - /// Whether the cell at (@p column, @p row) refuses a write, which is also - /// what tells the host. - odr.editing.refuseAt = function (column, row) { - if (!editable) { - refuse("readOnly", column, row); - return true; - } - var lock = odr.editing.lockAt(column, row); - if (lock !== null) { - refuse(lock, column, row); - return true; - } - return false; - }; - var overlay = null; var editingAt = null; var history = []; @@ -156,16 +55,6 @@ return Array.from(byPosition.values()); } - /// What a host's save button and back-press warning read. - function changed() { - fire("onEditChange", { - dirty: history.length > 0, - operations: coalesced().length, - canUndo: history.length > 0, - canRedo: undone.length > 0, - }); - } - var NUMBER = /^[+-]?([0-9]+(\.[0-9]*)?|\.[0-9]+)([eE][+-]?[0-9]+)?$/; /// The type follows the string the user typed: a number where the grammar @@ -210,7 +99,7 @@ before: before, }); undone = []; - changed(); + odr.editing.changed(); return true; } @@ -219,7 +108,7 @@ function replay(entry, value) { close(); odr.sheet.showValue(entry.op.column, entry.op.row, value); - changed(); + odr.editing.changed(); } // Offsets, not rects: blink scales a rect by the body zoom `viewport_js` @@ -248,7 +137,7 @@ /// The raise is put back down: the overlay shows what it would have. function edit(column, row, typed) { finish(); - if (!editing || odr.editing.refuseAt(column, row)) { + if (!odr.editing.isEnabled() || odr.editing.refuseAt(column, row)) { return false; } var cell = odr.sheet.cellAt(column, row); @@ -313,6 +202,8 @@ return true; } + /// The open editor's own keys, which no config takes away: Escape and Enter + /// are the only way out of an overlay. function overlayKey(event) { // Typing is the overlay's, not the sheet's underneath it. event.stopPropagation(); @@ -336,12 +227,16 @@ }; /// What a pinned cell does with a key when no editor is open. Captured, so - /// the keys taken here never reach the pin and the sort beneath. + /// the keys taken here never reach the pin and the sort beneath. The undo + /// chord is `editing.js`'s, for every format. function pinnedKey(event) { var target = event.target; if ( - !editing || + !odr.editing.isEnabled() || overlay !== null || + event.ctrlKey || + event.metaKey || + event.altKey || (target && (target.isContentEditable || /^(INPUT|TEXTAREA|SELECT)$/.test(target.tagName))) @@ -349,21 +244,6 @@ return; } - // ctrl/cmd is the undo chord here and nothing else. - if (event.ctrlKey || event.metaKey || event.altKey) { - var chord = event.key.toLowerCase(); - if (!event.altKey && (chord === "z" || chord === "y")) { - if (chord === "y" || event.shiftKey) { - odr.editing.redo(); - } else { - odr.editing.undo(); - } - event.stopPropagation(); - event.preventDefault(); - } - return; - } - var at = odr.sheet.pinned(); if (at === null || at.column === null || at.row === null) { return; @@ -389,7 +269,9 @@ event.preventDefault(); } - document.addEventListener("keydown", pinnedKey, true); + if (odr.takesKeys("navigation")) { + document.addEventListener("keydown", pinnedKey, true); + } window.addEventListener("resize", function () { if (overlay !== null) { @@ -402,7 +284,7 @@ } table.addEventListener("dblclick", function (event) { - var at = editing ? targetPosition(event) : null; + var at = odr.editing.isEnabled() ? targetPosition(event) : null; if (at !== null) { edit(at.column, at.row, null); } @@ -410,48 +292,72 @@ // A locked cell says so on the click, not on the double click. table.addEventListener("click", function (event) { - var at = editing && overlay === null ? targetPosition(event) : null; + var at = + odr.editing.isEnabled() && overlay === null ? targetPosition(event) : null; if (at !== null && odr.editing.lockAt(at.column, at.row) !== null) { odr.editing.refuseAt(at.column, at.row); } }); - /// Opens the editor over a cell, as a double click does. - odr.editing.editAt = function (column, row) { - return edit(column, row, null); - }; - - /// The envelope a host hands to `Document::edit` before saving. - odr.editing.getOperations = function () { - return JSON.stringify({ version: 1, ops: coalesced() }); + /// The lock on the cell at (@p column, @p row), or null where it has none. + odr.editing.lockAt = function (column, row) { + var cell = odr.sheet.cellAt(column, row); + return cell === null ? null : cell.getAttribute("data-odr-lock"); }; - /// Takes the last write back; false where there is none. - odr.editing.undo = function () { - if (history.length === 0) { - return false; + /// Whether the cell at (@p column, @p row) refuses a write, which is also + /// what tells the host. + odr.editing.refuseAt = function (column, row) { + if (!odr.editing.isEditable()) { + refuse("readOnly", column, row); + return true; } - var entry = history.pop(); - undone.push(entry); - replay(entry, entry.before); - return true; - }; - - odr.editing.redo = function () { - if (undone.length === 0) { - return false; + var lock = odr.editing.lockAt(column, row); + if (lock !== null) { + refuse(lock, column, row); + return true; } - var entry = undone.pop(); - history.push(entry); - replay(entry, entry.op.value); - return true; + return false; }; - /// The host saved the log: the page and the file agree, and undo starts - /// over. - odr.editing.committed = function () { - history = []; - undone = []; - changed(); + /// Opens the editor over a cell, as a double click does. + odr.editing.editAt = function (column, row) { + return edit(column, row, null); }; + + odr.editing.attach({ + // The overlay goes with the mode: a cell nothing can commit must not keep + // one open over it. + disable: close, + operations: coalesced, + canUndo: function () { + return history.length > 0; + }, + canRedo: function () { + return undone.length > 0; + }, + /// Takes the last write back; false where there is none. + undo: function () { + if (history.length === 0) { + return false; + } + var entry = history.pop(); + undone.push(entry); + replay(entry, entry.before); + return true; + }, + redo: function () { + if (undone.length === 0) { + return false; + } + var entry = undone.pop(); + history.push(entry); + replay(entry, entry.op.value); + return true; + }, + committed: function () { + history = []; + undone = []; + }, + }); })(); diff --git a/src/odr/internal/html/frontend/spreadsheet.js b/src/odr/internal/html/frontend/spreadsheet.js index f59ad9c54..abea56b72 100644 --- a/src/odr/internal/html/frontend/spreadsheet.js +++ b/src/odr/internal/html/frontend/spreadsheet.js @@ -482,11 +482,15 @@ } }); - document.addEventListener("keydown", function (event) { - if (event.key === "Escape") { - pin(-1, null, null); - } - }); + // Navigation, not editing: a read-only sheet has a pin, and this is what + // clears it. + if (odr.takesKeys("navigation")) { + document.addEventListener("keydown", function (event) { + if (event.key === "Escape") { + pin(-1, null, null); + } + }); + } var body = table.tBodies[0]; diff --git a/src/odr/internal/html/text_file.cpp b/src/odr/internal/html/text_file.cpp index 2e2b43a5b..bc6b29a4d 100644 --- a/src/odr/internal/html/text_file.cpp +++ b/src/odr/internal/html/text_file.cpp @@ -128,6 +128,9 @@ class HtmlServiceImpl final : public HtmlService { } out.write_element_end("div"); + // `text_js` is this view's editor, with its own undo, and it needs the + // browser to edit the lines. Not the mode: nothing replays these edits + // into a file, because `txt` declares no `edit` capability. out.write_element_begin("div", HtmlElementOptions().set_attributes( [&](const HtmlAttributeWriterCallback &clb) { diff --git a/test/browser/sheet/README.md b/test/browser/sheet/README.md index c396db452..131baa902 100644 --- a/test/browser/sheet/README.md +++ b/test/browser/sheet/README.md @@ -10,13 +10,19 @@ open http://localhost:8732/tests.html open http://localhost:8732/positions.html open http://localhost:8732/sorting.html open http://localhost:8732/editing.html +open http://localhost:8732/keyboard.html ``` -`serve` serves `document.css`, `spreadsheet.css`, `spreadsheet.js` and -`sheet-editing.js` straight out of `src/odr/internal/html/frontend/`, so what -runs is the file the library embeds. Each page prints its own report and heads -it with a count; a page holds one `.odr-sheet`, because the script binds to the -first one it finds. +`serve` serves `document.css`, `spreadsheet.css`, `editing.js`, +`spreadsheet.js` and `sheet-editing.js` straight out of +`src/odr/internal/html/frontend/`, so what runs is the file the library embeds. +Each page prints its own report and heads it with a count; a page holds one +`.odr-sheet`, because the script binds to the first one it finds. + +`editing.js` goes first, as the library writes it: it owns `odr.editing` and +`odr.takesKeys`, and both of the other scripts read them. The page states the +frame on its `` — `data-odr-editable` and `data-odr-keyboard` — because +that is where `translate` writes it. - **`tests.html`** — raising a cell whose text is cut off. The markup is what `translate_sheet` writes, cut down to the shapes the script has to tell apart: @@ -35,6 +41,9 @@ first one it finds. where its neighbour shows something, a formula cell, a cell of several runs, and one whose single run carries a style a write must keep. Undo, redo and the log a save resets follow. +- **`keyboard.html`** — a page whose config took both key classes away. The + arrows, Escape, a printable key and the undo chord are all the host's, while + the commands (`editAt`, `undo`) and the open editor's own keys still work. - **`sorting.html`** — the same questions after the sort control has moved every row. Nothing here is merged, because a merged sheet is offered no sort control; a row is found by the label it carries, so where it now sits does not diff --git a/test/browser/sheet/editing.html b/test/browser/sheet/editing.html index 4906c8443..f8392f7d9 100644 --- a/test/browser/sheet/editing.html +++ b/test/browser/sheet/editing.html @@ -6,12 +6,12 @@ - + - +
@@ -66,6 +66,7 @@
+ diff --git a/test/browser/sheet/keyboard.html b/test/browser/sheet/keyboard.html new file mode 100644 index 000000000..621da2a54 --- /dev/null +++ b/test/browser/sheet/keyboard.html @@ -0,0 +1,93 @@ + + + + + sheet keyboard checks + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
AB
1one
2
+ +
+ + + + + + + diff --git a/test/browser/sheet/positions.html b/test/browser/sheet/positions.html index b39025dbe..305fad273 100644 --- a/test/browser/sheet/positions.html +++ b/test/browser/sheet/positions.html @@ -6,11 +6,11 @@ - + - +
@@ -62,6 +62,7 @@
+ diff --git a/test/browser/sheet/serve b/test/browser/sheet/serve index e8b5f586c..37b56b84c 100755 --- a/test/browser/sheet/serve +++ b/test/browser/sheet/serve @@ -9,4 +9,14 @@ sys.path.insert(0, str(HERE.parent)) from serve import serve # noqa: E402 -serve(8732, HERE, ("tests.html", "positions.html", "sorting.html", "editing.html")) +serve( + 8732, + HERE, + ( + "tests.html", + "positions.html", + "sorting.html", + "editing.html", + "keyboard.html", + ), +) diff --git a/test/browser/sheet/sorting.html b/test/browser/sheet/sorting.html index e3bb92f56..858c12cba 100644 --- a/test/browser/sheet/sorting.html +++ b/test/browser/sheet/sorting.html @@ -6,10 +6,10 @@ - + - +
@@ -57,6 +57,7 @@
+ + + + + + From 6dd631f6a1cd1f497ec2f645655e8987654ebe0f Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Thu, 10 Sep 2026 10:29:37 +0200 Subject: [PATCH 09/11] test: advance the reference output pins Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KKFKbUVCYF2VhujdmjhhPW --- test/data.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/data.cmake b/test/data.cmake index a1781b149..cc75e4595 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 "8306d524142f5fcc1af2766211e646766f459c5e") + REVISION "fcadf97427034ab742d4753253fc3e8edeee8dd2") odr_test_data( PATH "reference-output/odr-private" URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git" - REVISION "daaff778aa28beeb1525d43e63c3a77d81be75c8") + REVISION "3025d2dc210e9be8ebf484d10f043828bc5ece32") From 30c63fe782bcb242386b9626c97b661a0d99ad36 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Thu, 10 Sep 2026 10:46:21 +0200 Subject: [PATCH 10/11] fix(html): collect a text edit from `input`, not from every mutation A `MutationObserver` cannot tell a reader's edit from a script rewriting the page, and `search.js` rewrites plenty: highlighting nine matches put nine no-op `setText` ops in the log and lit the host's save button. `input` is the browser saying it applied an edit, which a script never raises, so the two are no longer confused. The run is the one the caret sits in, falling back to the one `beforeinput` named; where neither answers, code 9 says the gate has a hole rather than dropping the edit in silence. Also records the two limits a reader meets in `editing.md` decision 13 - undo belongs to the browser until phase 3 gives the editor an inverse, and backspace at the start of a run is refused because merging two runs is not something `setText` can express - and adds `test/browser/text`, whose README says why no check may use `execCommand`: Chrome's scripted editing raises no cancelable `beforeinput` and dissolves a run whose whole text it replaces, neither of which trusted input does. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KKFKbUVCYF2VhujdmjhhPW --- CHANGELOG.md | 5 ++ docs/design/editing.md | 31 ++++++++- src/odr/internal/html/frontend/document.js | 81 ++++++++++++---------- test/browser/text/README.md | 22 +++--- test/browser/text/tests.html | 72 ++++++++++--------- 5 files changed, 131 insertions(+), 80 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d3c77c592..5b2a7a9d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,11 @@ The release run heads these entries with the version and opens a fresh (`newLine`, 1), an edit spanning two runs or landing outside every run (`range`, 8), and anything else the browser offers (`unsupportedEdit`, 7). +- **Fix**: searching a text document while the mode is on no longer marks it + unsaved. The log is collected from `input`, which the browser raises for an + edit it applied, rather than from every text mutation - a search highlighting + nine matches was nine no-op `setText` operations. + - **Fix**: double-clicking a sheet cell no longer flashes its border off. A click on the pinned cell clears the pin, and the second click of a double click was taking it - so selecting a word left the border coming and going. diff --git a/docs/design/editing.md b/docs/design/editing.md index b105c22aa..1aa3e3340 100644 --- a/docs/design/editing.md +++ b/docs/design/editing.md @@ -33,9 +33,9 @@ Today the frame is there and the text editor is not: save reads and the callbacks a host wires. Every format's editor attaches to it; `frontend/sheet-editing.js` is the first one (decision 9). - `frontend/document.js` holds the text editor. The mode makes the **whole - view** editable and the editor refuses what it cannot replay (decision 13); - a `MutationObserver` reads the changed runs back and emits one `setText` op - each. No selection model of our own, no marks, no undo yet. + view** editable, the editor refuses what it cannot replay, and `input` is + what it collects on (decision 13). No selection model of our own, no marks, + no undo yet. - `Document::edit(diff)` (`src/odr/document.cpp`) parses the op envelope and dispatches `setCell` and `setText`. - `back_translate` CLI replays a diff file onto a source document and `save`s it. @@ -377,6 +377,15 @@ furniture, the gap between two paragraphs and the page box are all refused without a single attribute of their own. That is decision 10's rule — mark the exceptions, not the rest — applied to the caret instead of to a cell. +**`input` is what the log collects on, not a `MutationObserver`.** The browser +raises `input` when *it* applied an edit; a script rewriting the page raises +none. That is the whole difference: `search.js` wraps every match in a ``, +which an observer watching `characterData` reads as nine edits — measured, and +it lit the host's save button and put nine no-op `setText` ops in the log. The +run is the one `beforeinput` named, or the one the caret sits in where no +`beforeinput` arrived; a run the editor cannot name at all raises code 9 rather +than being dropped. + **Known holes, both narrow.** A scripted `document.execCommand` can bypass the gate, because Chrome does not fire a cancelable `beforeinput` for every command; trusted input, which is all a reader has, is refused correctly. And a @@ -386,6 +395,22 @@ changes where no op can name it, rather than dropping it in silence. Android WebView's incomplete `beforeinput` (decision 8) is the reason that report exists; verify it on a device before trusting the gate there. +**Two limits a reader meets, and phase 3 is where both go:** + +- **Undo belongs to the browser, not to us.** Every allowed edit is one the + browser applied, so its own stack is the one that replays — ctrl+Z works, and + `chordKey` leaves the key alone because no editor claims it (decision 9). But + we cannot read that stack's depth, so `canUndo` is honestly false and a host's + undo *button* stays grey. Phase 3 item 2 is what fixes it: once the editor + records an op with its inverse, it answers `undo()` and joins the shared log. + Until then the button and the chord disagree, which is worse than either. +- **Backspace at the start of a run is refused.** Its target range reaches back + into the run before it, so the edit spans two and `range` refuses it — + merging two runs is not something `setText` can express. It did nothing under + the per-run hosts either; the difference is that it now says why. The op that + would fix it is `deleteRange` across runs, which needs the write-side adapter + work in phase 2, not a browser change. + ## Preliminary implementation plan (ODF / OOXML) Ordered to de-risk the linchpin (id stability) first and to keep every step diff --git a/src/odr/internal/html/frontend/document.js b/src/odr/internal/html/frontend/document.js index 3b495c35a..422370c8a 100644 --- a/src/odr/internal/html/frontend/document.js +++ b/src/odr/internal/html/frontend/document.js @@ -1,7 +1,6 @@ // The text editor, attached to `odr.editing`. The mode makes the whole view -// editable and refuses what it cannot replay: only `setText` reaches the file, -// so an edit has to land inside one addressed run. `editing.md` phase 3 -// replaces the collection with a model of its own. +// editable and refuses what it cannot replay: `setText` is the only op the +// file takes, so an edit has to land inside one addressed run. (function () { "use strict"; @@ -32,15 +31,16 @@ return ops; } - /// The run @p node sits in, or null where it sits outside one - between two - /// paragraphs, beside a picture, in the gap under the last block. + /// The run @p node sits in, or null where it sits outside every run. function runOf(node) { - var element = node === null || node.nodeType === 1 ? node : node.parentElement; + if (node === null) { + return null; + } + var element = node.nodeType === 1 ? node : node.parentElement; return element === null ? null : element.closest("[data-odr-path]"); } - // The input types that only ever change the text of one run. Everything else - // is refused, because `setText` is the only op the file takes. + // The input types that only ever change the text of one run. var textual = { insertText: 1, insertReplacementText: 1, @@ -54,8 +54,8 @@ deleteWordForward: 1, deleteSoftLineBackward: 1, deleteSoftLineForward: 1, - // The browser's own stack holds the text edits above and nothing else, - // because the structural ones never happened. + // Its stack holds the edits above and nothing else: the structural ones + // never happened. historyUndo: 1, historyRedo: 1, }; @@ -64,8 +64,7 @@ var named = { insertParagraph: "newLine", insertLineBreak: "newLine" }; /// Where an edit lands: `run` is the one run it is confined to, null where - /// it spans two or lands outside every run. `path` is where it starts, which - /// is what a host needs to say *where* an edit was refused. + /// it spans two or lands outside every run. `path` is where it starts. function target(event) { var ranges = typeof event.getTargetRanges === "function" ? event.getTargetRanges() : []; @@ -92,12 +91,16 @@ if (event.cancelable) { event.preventDefault(); } - // The path is what keeps two refusals apart, so a reader pressing Enter in - // one run and then in another hears about both. + // The path keeps two refusals apart, so Enter in one run and then in + // another is heard twice. odr.editing.refuse(reason, { path: at.path }); } + // Where the edit the gate just allowed will land, for `input` to record. + var pending = null; + root.addEventListener("beforeinput", function (event) { + pending = null; var at = target(event); if (!odr.editing.isEnabled()) { refuse(event, "readOnly", at); @@ -115,6 +118,7 @@ refuse(event, "range", at); return; } + pending = at.run; if (event.inputType === "insertFromPaste") { // Whatever the clipboard holds, one run takes plain text on one line. var text = event.dataTransfer @@ -131,34 +135,37 @@ } }); - new MutationObserver(function (mutations) { + /// The run the caret sits in, for an edit no `beforeinput` announced. + function selectedRun() { + var selection = window.getSelection(); + return selection === null || selection.rangeCount === 0 + ? null + : runOf(selection.getRangeAt(0).startContainer); + } + + // `input` is the browser saying it applied an edit, which a script rewriting + // the page never raises - so a search highlighting nine matches leaves the + // log alone. A `MutationObserver` could not tell the two apart. + root.addEventListener("input", function () { if (!odr.editing.isEnabled()) { return; } - var moved = false; - for (var i = 0; i < mutations.length; ++i) { - if (mutations[i].type !== "characterData") { - continue; - } - // The nearest owner, not the direct parent: a search `` may sit - // between the edited text and the element carrying the path. - var owner = runOf(mutations[i].target); - if (owner !== null) { - modified[owner.getAttribute("data-odr-path")] = owner; - moved = true; - } else { - // The page changed where no op can name it. `beforeinput` should have - // refused this, so a host hearing it has found a hole. - odr.onError(9, "text changed outside an addressed run"); - } + // The caret first: the browser has just put it where the edit landed. A + // `beforeinput` whose edit changed nothing raises no `input`, so what it + // left in `pending` may be a run ago - it is the fallback, not the answer. + var run = selectedRun(); + if (run === null) { + run = pending; } - if (moved) { - odr.editing.changed(); + pending = null; + if (run === null) { + // The gate refuses an edit it cannot name, so this is a hole in it: a + // WebView that raised no `beforeinput`, or a composition it cannot stop. + odr.onError(9, "an edit landed where no operation can name it"); + return; } - }).observe(root, { - childList: true, - subtree: true, - characterData: true, + modified[run.getAttribute("data-odr-path")] = run; + odr.editing.changed(); }); odr.editing.attach({ diff --git a/test/browser/text/README.md b/test/browser/text/README.md index dc3ed9686..89ac6e404 100644 --- a/test/browser/text/README.md +++ b/test/browser/text/README.md @@ -31,13 +31,17 @@ Why the checks look the way they do: decision 9), and a refusal is keyed by its run — so the two `newLine` checks sit in different runs, and the one outside every run asserts the prevented edit rather than a second event. -- **The collection is a `MutationObserver`**, which reports on a microtask, so - the last checks wait a turn. `checks.js` retallies on every check for exactly - this reason. +- **The gate and the collection are driven apart.** `beforeinput` is what + refuses; `input` is what collects, because a script rewriting the page raises + none — which is how a search highlighting nine matches leaves the log alone. + No script can raise the pair the way a key does, so a check drives one or the + other. -**A scripted `document.execCommand` can still get past the guard.** Chrome does -not fire a cancelable `beforeinput` for every command, so -`execCommand("insertParagraph")` splits a paragraph the editor would have -refused — leaving two elements under one `data-odr-path`. Trusted input does -not: a real Enter is refused, which is what a reader can reach. Verified by -hand on 2026-09-10. +**Scripted editing is not the editing a reader does, which is why no check uses +`execCommand`.** Chrome's scripted path differs from its trusted-input path +twice over: it raises no cancelable `beforeinput`, so +`execCommand("insertParagraph")` splits a paragraph the gate would have +refused; and it dissolves a run whose whole text it replaces, leaving the new +text outside every address. Trusted input does neither — a real Enter is +refused, and typing over a whole run keeps the run, its address and its style. +Both were checked by hand in a browser, and neither is reachable by a reader. diff --git a/test/browser/text/tests.html b/test/browser/text/tests.html index cd2515d08..0d0141979 100644 --- a/test/browser/text/tests.html +++ b/test/browser/text/tests.html @@ -40,6 +40,7 @@
+ From 0fc53599ae643786dacacc431facdd6fe02abbb5 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Thu, 10 Sep 2026 10:46:21 +0200 Subject: [PATCH 11/11] test: advance the reference output pins Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KKFKbUVCYF2VhujdmjhhPW --- test/data.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/data.cmake b/test/data.cmake index cc75e4595..16e1370fa 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 "fcadf97427034ab742d4753253fc3e8edeee8dd2") + REVISION "3420c71a5bec0548cc50db14a1a7f99f99abd4f2") odr_test_data( PATH "reference-output/odr-private" URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git" - REVISION "3025d2dc210e9be8ebf484d10f043828bc5ece32") + REVISION "bafe69ecd4c237f682284f09fc0633b8ada6c661")