diff --git a/CHANGELOG.md b/CHANGELOG.md index fd61f4184..5b2a7a9d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,45 @@ 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 and the `odr.onEdit*` callbacks are one surface a host + wires per document, and each format attaches its own editor to it. + +- **Breaking**: `HtmlConfig::editable` writes the editing scaffolding rather + than `contenteditable` - the address an op names, the lock on a locked cell, + the state on ``, the editor script. `odr.editing.enable()` writes + `contenteditable`, so switching modes needs no second render. + +- **Breaking**: a render with `editable` off carries no editing markup at all, + and the document's editable state moved off the `.odr-sheet` table onto + `` as `data-odr-editable`. The table keeps `data-odr-sheet`. + +- **Breaking**: a refused new line reaches `odr.onEditRefused` with reason + `newLine` rather than `odr.onError`, keeps code 1, and now fires only inside + an editable run while the mode is on. + +- `HtmlConfig::keyboard_navigation` and `keyboard_shortcuts`, both on by + default, decide whether the page takes the keys that move the selection and + the undo chord. An open editor's own keys are never taken away. + +- A text document is edited as a document: the mode makes the whole view + editable rather than each run, so the caret, a selection and a double click + cross runs and paragraphs the way a reader expects. + +- Every edit a text document cannot replay is refused through + `odr.onEditRefused` rather than silently impossible: a new line + (`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. + - 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 6c8ea672e..1aa3e3340 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,21 @@ 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. The mode makes the **whole + 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. The goal is to generalise this from "replace text in a span" to full content and @@ -165,6 +178,239 @@ 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 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 +[`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 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 +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. + +### 13. One editable view, and every edit it cannot replay is refused + +`enable()` puts `contenteditable` on the **body**, not on each run. The editor +then intercepts `beforeinput` and refuses everything that is not the text of one +addressed run: + +| The edit | What happens | +|---|---| +| text typed, replaced, pasted plain, deleted, composed — inside one run | allowed, and the run joins the log | +| a new line (`insertParagraph`, `insertLineBreak`) | refused, reason `newLine` | +| anything else the browser offers (a mark, a list, a drop) | refused, reason `unsupportedEdit` | +| an edit spanning two runs, or landing outside every run | refused, reason `range` | + +**Why the whole view rather than a run at a time:** `contenteditable` per run +makes every run its own editing host, and a host is a wall. The caret cannot +cross it, a selection cannot span two of them, and a reader who selects a +sentence gets nothing — silently, with no way to say why. One host gives the +document the caret, selection and word-double-click a reader expects, and the +refusal channel (decision 9) is what says no where we cannot follow. That is +also far less markup to write and one attribute to toggle rather than a walk +over every run. + +**Why `beforeinput` is the gate:** it fires before the browser changes anything, +it says *what* the edit is (`inputType`), it says *where* (`getTargetRanges()`), +and it is cancelable. `text.js` already edits the plain-text view this way. + +**The whitelist is closed, not open.** Only the input types that change the text +of one run are allowed; anything unrecognised is refused. An open list would let +a browser-specific `inputType` through to a `MutationObserver` that only watches +`characterData`, and a structural change would then be invisible to the log and +saved wrong. Refusing something we could have allowed costs a reader one +gesture; allowing something we cannot replay costs them their document. + +**The address is the whole guard.** No element is marked non-editable: an edit is +allowed because it lands inside a `[data-odr-path]` run, so a picture, a table's +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 +composition cannot be cancelled at all — `insertCompositionText` is allowed and +reconciled afterwards, which is why the observer reports code 9 when text +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 @@ -211,11 +457,22 @@ Sequence within Phase 2: (a) delete element, (b) toggle mark on a range, ### Phase 3 — Browser editor -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. +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`; op recorder; composition-aware reconciliation. + `beforeinput` is already the gate (decision 13); what is left is owning the + edit rather than letting the browser apply it and reading the run back. +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 +566,15 @@ 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`) 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/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? 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..ff85c9070 100644 --- a/src/odr/html.hpp +++ b/src/odr/html.hpp @@ -129,9 +129,17 @@ 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 address an + /// edit operation names, and the editor script. The mode itself starts off - + /// the host turns it on with `odr.editing.enable()`. 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. + 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..7baf195d4 100644 --- a/src/odr/internal/html/document.cpp +++ b/src/odr/internal/html/document.cpp @@ -198,6 +198,21 @@ void write_head(const Document &document, const WritingState &state, out.write_header_end(); } +/// The key classes the view's scripts may take; `editing.md` decision 12. +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 +237,18 @@ 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 `enable()` answers, stated only by a render that edits + if (state.config().editable) { + clb("data-odr-editable", + state.document_editable() ? "true" : "readOnly"); + } + // not an editing fact: a read-only sheet has a pin to clear + clb("data-odr-keyboard", keyboard_classes(state.config())); + })); if (paged_content) { out.write_element_begin("div", HtmlElementOptions().set_class("odr-pages")); @@ -237,9 +263,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..2c95c39b0 100644 --- a/src/odr/internal/html/document_element.cpp +++ b/src/odr/internal/html/document_element.cpp @@ -233,21 +233,22 @@ 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 address an edit operation names. +/// `contenteditable` is not written: the mode adds it to these runs. +bool writes_edit_markup(const Element &element, + const html::WritingState &state) { return state.editable_markup() && state.config().editable && element.is_editable(); } +/// A run whose style the box around it can carry instead. Not a background, a +/// raised run or an addressed one: each means something else on the box. std::optional plain_text(const Element &element, const html::WritingState &state) { if (element.type() != ElementType::text) { return {}; } - if (writes_editable(element, state)) { + if (writes_edit_markup(element, state)) { return {}; } @@ -487,9 +488,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 +667,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 read-only render states no lock + const char *lock = + state.config().editable ? cell_lock(cell, anchors_shapes) : nullptr; state.out().write_element_begin( "td", @@ -778,8 +778,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..608d6ea64 100644 --- a/src/odr/internal/html/frontend.cpp +++ b/src/odr/internal/html/frontend.cpp @@ -87,6 +87,9 @@ 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}; +/// The mode, the refusals and the log a save reads, generic over the formats. +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 +99,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 +259,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 +273,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..0c01ad02c 100644 --- a/src/odr/internal/html/frontend.hpp +++ b/src/odr/internal/html/frontend.hpp @@ -36,10 +36,16 @@ 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`, `odr.onError`, `odr.takesKeys` and `odr.generateDiff`. +/// Written by every document view, and **first**: the scripts below read +/// `odr.takesKeys`. +void write_editing_script(const WritingState &state); +/// The text editor, attached to the mode. Written where the config edits. void write_document_script(const WritingState &state); -/// Written in addition to the document script. +/// Written in addition to the editing script. void write_spreadsheet_script(const WritingState &state); +/// The sheet's editor, attached to the mode. Written where the config edits. +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..422370c8a 100644 --- a/src/odr/internal/html/frontend/document.js +++ b/src/odr/internal/html/frontend/document.js @@ -1,52 +1,183 @@ +// The text editor, attached to `odr.editing`. The mode makes the whole view +// 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"; 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", - }; + // A view whose runs carry no address is not this editor's: a sheet's editing + // is an overlay, and its cells state no path. + if (document.querySelector("[data-odr-path]") === null) { + return; + } + var root = document.body; 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 }); + ops.push({ + op: "setText", + path: path, + // Not `innerText`: that is the rendered text, and it drops the + // trailing space a reader just typed. + text: modified[path].textContent, + }); } } - return JSON.stringify({ version: 1, ops: ops }); + return ops; + } + + /// The run @p node sits in, or null where it sits outside every run. + function runOf(node) { + 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. + var textual = { + insertText: 1, + insertReplacementText: 1, + insertFromPaste: 1, + insertCompositionText: 1, + deleteContent: 1, + deleteContentBackward: 1, + deleteContentForward: 1, + deleteByCut: 1, + deleteWordBackward: 1, + deleteWordForward: 1, + deleteSoftLineBackward: 1, + deleteSoftLineForward: 1, + // Its stack holds the edits above and nothing else: the structural ones + // never happened. + historyUndo: 1, + historyRedo: 1, }; - new MutationObserver(function (mutations) { - for (var i = 0; i < mutations.length; ++i) { - if (mutations[i].type !== "characterData") { - continue; + // A new line has nowhere to go in a run, and the reader knows the key. + 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. + function target(event) { + var ranges = + typeof event.getTargetRanges === "function" ? event.getTargetRanges() : []; + var range = ranges.length === 1 ? ranges[0] : undefined; + if (ranges.length === 0) { + // No target range: a composition, and some browsers for a paste. The + // caret is what the edit will land on. + var selection = window.getSelection(); + if (selection !== null && selection.rangeCount > 0) { + range = selection.getRangeAt(0); } - // The nearest owner, not the direct parent: a search `` may sit - // between the edited text and the element carrying the path. - var parent = mutations[i].target.parentElement; - var owner = parent && parent.closest("[data-odr-path]"); - if (owner) { - modified[owner.getAttribute("data-odr-path")] = owner; + } + if (range === undefined) { + return { run: null, path: null }; + } + var start = runOf(range.startContainer); + return { + run: start !== null && start === runOf(range.endContainer) ? start : null, + path: start === null ? null : start.getAttribute("data-odr-path"), + }; + } + + function refuse(event, reason, at) { + if (event.cancelable) { + event.preventDefault(); + } + // 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); + return; + } + if (!textual[event.inputType]) { + refuse(event, named[event.inputType] || "unsupportedEdit", at); + return; + } + // Its own stack is what the browser replays; there is nothing to address. + if (event.inputType === "historyUndo" || event.inputType === "historyRedo") { + return; + } + if (at.run === null) { + 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 + ? event.dataTransfer.getData("text/plain") + : null; + if (text === null) { + refuse(event, "unsupportedEdit", at); + } else if (/[\r\n]/.test(text)) { + refuse(event, "newLine", at); + } else { + event.preventDefault(); + document.execCommand("insertText", false, text); } } - }).observe(document.body, { - childList: true, - subtree: true, - characterData: true, }); - document.addEventListener("keydown", function (event) { - if (event.key === "Enter") { - event.preventDefault(); - odr.onError(errorIllegalEditNewLine.code, errorIllegalEditNewLine.message); + /// 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; + } + // 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; } + 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; + } + modified[run.getAttribute("data-odr-path")] = run; + odr.editing.changed(); + }); + + odr.editing.attach({ + enable: function () { + root.setAttribute("contenteditable", "true"); + }, + disable: function () { + root.removeAttribute("contenteditable"); + }, + 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..4d9b9e859 --- /dev/null +++ b/src/odr/internal/html/frontend/editing.js @@ -0,0 +1,215 @@ +// `odr.editing`: the editing mode, generic over the formats. 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; + + // An absent `data-odr-editable` is a render offering no editing, and answers + // `readOnly` like a document that refuses one. + 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 - `odr.onError` shares + // it, and holds 9. The host maps the code; the message is for a console. + 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" }, + unsupportedEdit: { code: 7, message: "this kind of edit is not supported" }, + range: { code: 8, message: "an edit has to lie inside one run of text" }, + }; + + 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 scripts take a class of key event: `navigation` or + /// `shortcuts`. `HtmlConfig` decides. + odr.takesKeys = function (name) { + return keyClasses.indexOf(name) !== -1; + }; + + /// Whether any editor answered @p name; the one attached last is asked first. + 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](); + } + } + } + + /// What 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 = { + /// False where nothing on this page can be edited, with the reason on + /// `onEditModeChange` - 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. Only `operations` is required; `enable`, + /// `disable`, `undo`, `redo`, `canUndo`, `canRedo` and `committed` default. + attach: function (editor) { + editors.push(editor); + }, + + /// Reports a refused edit, dropping a repeat of the same one within two + /// seconds: four taps on a locked cell are one snackbar. @p detail is how + /// the format addresses it. Painting it is the editor's. + 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 && 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); + }, + + /// The log a host's save button reads; an editor calls it when 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() }); + }, + + /// False where no editor has an edit to take back. + undo: function () { + return ask("undo"); + }, + redo: function () { + return ask("redo"); + }, + + /// The host saved: every editor's log resets and undo starts over. + committed: function () { + tell("committed"); + odr.editing.changed(); + }, + }; + + /// The name the apps and the wasm package call. Same envelope. + odr.generateDiff = function () { + return odr.editing.getOperations(); + }; + + /// The undo chord. A form field keeps its own text undo, and a key no editor + /// took is left to the browser. + 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..c9055e9d5 100644 --- a/src/odr/internal/html/frontend/sheet-editing.js +++ b/src/odr/internal/html/frontend/sheet-editing.js @@ -1,3 +1,5 @@ +// The sheet's editor, attached to `odr.editing`: the cell overlay, the locks +// and the `setCell` op. The mode itself is `editing.js`. (function () { "use strict"; @@ -9,34 +11,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 +32,12 @@ }, 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. + /// The outline answers every tap; `odr.editing` drops the repeated event. 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 +53,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 +97,7 @@ before: before, }); undone = []; - changed(); + odr.editing.changed(); return true; } @@ -219,7 +106,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 +135,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 +200,8 @@ return true; } + /// The open editor's own keys, which no config takes away: they are the way + /// out of the overlay. function overlayKey(event) { // Typing is the overlay's, not the sheet's underneath it. event.stopPropagation(); @@ -336,12 +225,15 @@ }; /// 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. + /// these never reach the pin and the sort beneath. The chord is `editing.js`'s. 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 +241,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 +266,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 +281,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 +289,71 @@ // 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({ + // A cell nothing can commit must keep no overlay 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..2e5c8dca7 100644 --- a/src/odr/internal/html/frontend/spreadsheet.js +++ b/src/odr/internal/html/frontend/spreadsheet.js @@ -458,9 +458,13 @@ return; } - // Clicking what is pinned clears it. + // Clicking what is pinned clears it - but `detail` counts the clicks, and + // the second of a double click is the reader selecting a word. Clearing + // the pin under that flickers the border off again. if (cell === pinnedCell) { - pin(-1, null, null); + if (event.detail <= 1) { + pin(-1, null, null); + } return; } @@ -482,11 +486,14 @@ } }); - document.addEventListener("keydown", function (event) { - if (event.key === "Escape") { - pin(-1, null, null); - } - }); + // Navigation, not editing: a read-only sheet has a pin to clear. + 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..04f52766b 100644 --- a/src/odr/internal/html/text_file.cpp +++ b/src/odr/internal/html/text_file.cpp @@ -128,6 +128,8 @@ class HtmlServiceImpl final : public HtmlService { } out.write_element_end("div"); + // `text.js` is this view's own editor and needs the browser to edit the + // lines. Not the mode: `txt` declares no `edit`, so nothing replays these. out.write_element_begin("div", HtmlElementOptions().set_attributes( [&](const HtmlAttributeWriterCallback &clb) { diff --git a/test/browser/sheet/checks.js b/test/browser/checks.js similarity index 58% rename from test/browser/sheet/checks.js rename to test/browser/checks.js index 401f46c2e..ca143157e 100644 --- a/test/browser/sheet/checks.js +++ b/test/browser/checks.js @@ -13,6 +13,19 @@ var report = document.getElementById("report"); var failed = 0; var total = 0; + var summary = null; + + // Written on every check, not once on load: a page whose last checks wait + // for a timer or an observer still gets counted. + function retally() { + if (summary === null) { + summary = document.createElement("div"); + summary.id = "summary"; + report.parentNode.insertBefore(summary, report); + } + summary.textContent = total + " checks, " + failed + " failed"; + summary.style.color = failed === 0 ? "#2a7" : "#c33"; + } window.check = function (name, condition) { var line = document.createElement("div"); @@ -23,6 +36,7 @@ if (!condition) { failed += 1; } + retally(); }; window.click = function (element) { @@ -30,11 +44,14 @@ document.body.offsetHeight; }; - window.addEventListener("load", function () { - var summary = document.createElement("div"); - summary.id = "summary"; - summary.textContent = total + " checks, " + failed + " failed"; - summary.style.color = failed === 0 ? "#2a7" : "#c33"; - report.parentNode.insertBefore(summary, report); - }); + // `detail` counts the clicks, as a browser counts them. + window.doubleClick = function (element) { + element.dispatchEvent(new MouseEvent("click", { bubbles: true, detail: 1 })); + element.dispatchEvent(new MouseEvent("click", { bubbles: true, detail: 2 })); + element.dispatchEvent( + new MouseEvent("dblclick", { bubbles: true, detail: 2 }) + ); + document.body.offsetHeight; + }; + })(); diff --git a/test/browser/serve.py b/test/browser/serve.py index 7ececcb51..5cc714a5f 100644 --- a/test/browser/serve.py +++ b/test/browser/serve.py @@ -2,7 +2,9 @@ A page links `viewport.js` or `spreadsheet.css` by name; those are not in the check directory but in `src/odr/internal/html/frontend/`, so what runs here is -the file the library embeds rather than a copy of it. +the file the library embeds rather than a copy of it. `checks.js` is shared by +every check directory and sits here, which is the second place a name is +looked up. """ import functools @@ -20,13 +22,17 @@ ) +SHARED = pathlib.Path(__file__).resolve().parent + + class Handler(http.server.SimpleHTTPRequestHandler): def translate_path(self, path: str) -> str: translated = pathlib.Path(super().translate_path(path)) if not translated.is_file(): - asset = ASSETS / translated.name - if asset.is_file(): - return str(asset) + for directory in (ASSETS, SHARED): + candidate = directory / translated.name + if candidate.is_file(): + return str(candidate) return str(translated) 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..96ec9f696 --- /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..9fd5b15a5 100644 --- a/test/browser/sheet/positions.html +++ b/test/browser/sheet/positions.html @@ -6,11 +6,11 @@ - + - +
@@ -62,6 +62,7 @@
+ @@ -148,6 +149,17 @@ document.querySelector(".odr-sheet-pinned-cell") === null ); + // A double click is the reader selecting a word, and the pin under it + // has to stand: clearing it on the second click flickers the border. + doubleClick(odr.sheet.cellAt(1, 2)); + var held = odr.sheet.pinned(); + check( + "a double click leaves the pin standing", + held !== null && held.column === 1 && held.row === 2 + ); + click(odr.sheet.cellAt(1, 2)); + check("a single click on it still clears the pin", odr.sheet.pinned() === null); + click(document.querySelectorAll(".odr-sheet-column-header")[2]); var column = odr.sheet.pinned(); check( 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 @@
+ + + + + + + diff --git a/test/data.cmake b/test/data.cmake index 4c0cbed89..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 "0ac2e32f40c7943327fe3fd4c455bf9d13ae7196") + REVISION "3420c71a5bec0548cc50db14a1a7f99f99abd4f2") odr_test_data( PATH "reference-output/odr-private" URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git" - REVISION "58e34ca977a9d46e290b895979872df982c51f00") + REVISION "bafe69ecd4c237f682284f09fc0633b8ada6c661") diff --git a/test/src/html_test.cpp b/test/src/html_test.cpp index 42b5fc82f..5839dee81 100644 --- a/test/src/html_test.cpp +++ b/test/src/html_test.cpp @@ -664,6 +664,13 @@ std::size_t count(const std::string &haystack, const std::string_view needle) { return result; } +/// The scaffolding the mode needs is written only under this config. +HtmlConfig editing_config() { + HtmlConfig config; + config.editable = true; + return config; +} + std::string render_sheet(const DecodedFile &file, const HtmlConfig &config) { std::ostringstream out; html::translate(file, config).list_views().at(0).write_html(out); @@ -839,30 +846,78 @@ TEST(html, a_cell_holding_one_plain_string_writes_no_box_of_its_own) { EXPECT_EQ(page.find(")" + R"(7)")), + HtmlConfig()); + + // The attribute, not the name: the stylesheet and the scripts name both. + EXPECT_EQ(page.find(R"(data-odr-editable=")"), std::string::npos); + EXPECT_EQ(page.find(R"(data-odr-lock=")"), std::string::npos); + // The mode itself stays, so a host asks the page rather than tracking what + // it rendered with. + EXPECT_NE(page.find("odr.takesKeys"), std::string::npos); +} + +// The classes the scripts may take, which a host with its own bindings keeps. +TEST(html, a_document_states_the_key_classes_its_scripts_take) { + const std::string page = + render_sheet(fods_file(fods_row(fods_cell("one"))), HtmlConfig()); + + EXPECT_NE(page.find(R"(data-odr-keyboard="navigation shortcuts")"), + std::string::npos); +} + +TEST(html, a_config_takes_the_key_classes_away) { + HtmlConfig config; + config.keyboard_navigation = false; + config.keyboard_shortcuts = false; + + const std::string page = + render_sheet(fods_file(fods_row(fods_cell("one"))), config); + + EXPECT_NE(page.find(R"(data-odr-keyboard="")"), std::string::npos); +} + +TEST(html, a_config_takes_one_key_class_away) { + HtmlConfig config; + config.keyboard_navigation = false; + + const std::string page = + render_sheet(fods_file(fods_row(fods_cell("one"))), config); + + EXPECT_NE(page.find(R"(data-odr-keyboard="shortcuts")"), std::string::npos); +} + // A formula cell is locked: overwriting it leaves its dependants stale. TEST(html, a_formula_cell_is_locked_with_its_reason) { const std::string page = render_sheet( @@ -870,7 +925,7 @@ TEST(html, a_formula_cell_is_locked_with_its_reason) { R"xml()" R"(7)")), - HtmlConfig()); + editing_config()); EXPECT_NE(page.find(R"(data-odr-lock="formula")"), std::string::npos); EXPECT_NE(page.find("odr-locked"), std::string::npos); @@ -882,7 +937,7 @@ TEST(html, a_cell_of_several_runs_carries_no_lock) { fods_file(fods_row(R"()" R"(two runs)" R"()")), - HtmlConfig()); + editing_config()); EXPECT_EQ(page.find(R"(data-odr-lock=")"), std::string::npos); } @@ -894,7 +949,7 @@ TEST(html, a_cell_holding_a_link_is_locked_rich) { fods_file(fods_row(R"()" R"()" R"(x)")), - HtmlConfig()); + editing_config()); EXPECT_NE(page.find(R"(data-odr-lock="rich")"), std::string::npos); } @@ -906,7 +961,7 @@ TEST(html, a_cell_of_several_paragraphs_is_locked_rich) { fods_file(fods_row(R"()" R"(ab)" R"()")), - HtmlConfig()); + editing_config()); EXPECT_NE(page.find(R"(data-odr-lock="rich")"), std::string::npos); } @@ -914,22 +969,27 @@ TEST(html, a_cell_of_several_paragraphs_is_locked_rich) { // The cost is a class on the locked cells only, nothing on the others. TEST(html, a_plain_cell_carries_no_lock) { const std::string page = - render_sheet(fods_file(fods_row(fods_cell("one"))), HtmlConfig()); + render_sheet(fods_file(fods_row(fods_cell("one"))), editing_config()); EXPECT_EQ(page.find(R"(data-odr-lock=")"), std::string::npos); // the stylesheet names the class either way EXPECT_EQ(page.find(R"(class="odr-locked")"), std::string::npos); } -// A text document still says so in the markup: it has no overlay. -TEST(html, an_editable_text_document_marks_its_runs) { - HtmlConfig config; - config.editable = true; +// An op names a run, so the markup addresses one. The mode writes the +// `contenteditable`, so one page serves both modes. +TEST(html, an_editable_text_document_addresses_its_runs) { + const std::string page = render_odt(editing_config()); - const std::string page = render_odt(config); - - EXPECT_NE(page.find(R"(contenteditable="true")"), std::string::npos); EXPECT_NE(page.find("data-odr-path"), std::string::npos); + EXPECT_EQ(page.find(R"(contenteditable="true")"), std::string::npos); +} + +// The address is the expensive half, and a read-only render pays none of it. +TEST(html, a_read_only_text_document_addresses_no_run) { + const std::string page = render_odt(HtmlConfig()); + + EXPECT_EQ(page.find("data-odr-path"), std::string::npos); } // #822: a sheet cell does not break its text into lines unless the file says diff --git a/wasm/README.md b/wasm/README.md index d29fa994b..f8d052ed6 100644 --- a/wasm/README.md +++ b/wasm/README.md @@ -67,15 +67,28 @@ Editing is a round trip through the rendered page: ```js const doc = odr.open(bytes, { editable: true }); const { html } = doc.render(0); +const page = iframe.contentWindow.odr; +// The mode starts off, whatever the config: turning it on is the host's, and +// `enable()` refuses where the document cannot be edited. +page.editing.enable(); // ... the reader edits the page in the iframe ... -// `generateDiff()` returns the json already; stringifying it again is a string -// where `edit` wants an object, and it throws. -doc.edit(iframe.contentWindow.odr.generateDiff()); +// `getOperations()` returns the json already; stringifying it again is a string +// where `edit` wants an object, and it throws. `generateDiff()` is the older +// name for the same envelope. +doc.edit(page.editing.getOperations()); const saved = doc.save(); // the document, not the html download(new Blob([saved])); +// The page and the file agree now, so its log resets and undo starts over. +page.editing.committed(); ``` +`odr.editing` is on every document view, editable or not: `isEditable()` is what +greys a host's edit button, and `onEditRefused` says why an edit was refused. +`keyboardNavigation` and `keyboardShortcuts` in the config decide whether the +page takes the arrow keys and the undo chord, for a host that has its own. +`example/index.html` wires the whole surface. + `isEditable()` and `isSavable()` answer for this document, where `capabilities()` answers for the format. Only ODF and docx can be saved so far; anything else throws `UnsupportedOperation`. diff --git a/wasm/example/index.html b/wasm/example/index.html index 1de0e0ef4..d6b54a660 100644 --- a/wasm/example/index.html +++ b/wasm/example/index.html @@ -93,8 +93,10 @@ // `evaluateJavascript` once the WebView has finished loading. function wire() { const page = editing(); - edit.hidden = page === null; - save.hidden = page === null || !doc.isSavable(); + // `odr.editing` is on every document view, so the page's own answer + // greys the button rather than the absence of the object. + edit.hidden = page === null || !page.isEditable(); + save.hidden = edit.hidden || !doc.isSavable(); if (page === null) return; edit.textContent = 'edit'; frame.contentWindow.odr.onEditRefused = (event) => { diff --git a/wasm/js/index.d.ts b/wasm/js/index.d.ts index c24299195..57e54aca2 100644 --- a/wasm/js/index.d.ts +++ b/wasm/js/index.d.ts @@ -87,6 +87,8 @@ export interface Content { export interface HtmlConfig { embedImages?: boolean; editable?: boolean; + keyboardNavigation?: boolean; + keyboardShortcuts?: boolean; textDocumentMargin?: boolean; formatHtml?: boolean; pageRangeBegin?: number; diff --git a/wasm/src/wasm_html.cpp b/wasm/src/wasm_html.cpp index ac20a8725..bfed564e5 100644 --- a/wasm/src/wasm_html.cpp +++ b/wasm/src/wasm_html.cpp @@ -157,6 +157,8 @@ HtmlConfig to_html_config(const emscripten::val &value) { read(value, "embedImages", config.embed_images); read(value, "editable", config.editable); + read(value, "keyboardNavigation", config.keyboard_navigation); + read(value, "keyboardShortcuts", config.keyboard_shortcuts); read(value, "textDocumentMargin", config.text_document_margin); read(value, "formatHtml", config.format_html); diff --git a/wasm/tests/edit.test.mjs b/wasm/tests/edit.test.mjs index 1270fe1e4..ecd73b15b 100644 --- a/wasm/tests/edit.test.mjs +++ b/wasm/tests/edit.test.mjs @@ -34,8 +34,6 @@ describe('edit', () => { const doc = odr.open(minimalOdt('hello'), { editable: true }); try { const { html } = doc.render(0); - assert.match(html, /contenteditable/); - const path = firstEditablePath(html); doc.edit(JSON.stringify({ version: 1, diff --git a/wasm/tests/render.test.mjs b/wasm/tests/render.test.mjs index 978ab2031..04a52d74b 100644 --- a/wasm/tests/render.test.mjs +++ b/wasm/tests/render.test.mjs @@ -100,8 +100,10 @@ describe('render', () => { const plain = odr.open(fixture('mixed-layout.odt')); const editable = odr.open(fixture('mixed-layout.odt'), { editable: true }); try { - assert.ok(!plain.render(0).html.includes('contenteditable')); - assert.ok(editable.render(0).html.includes('contenteditable')); + // `editable` writes the address the mode needs; the mode writes the + // `contenteditable`. + assert.ok(!plain.render(0).html.includes('data-odr-path')); + assert.ok(editable.render(0).html.includes('data-odr-path')); } finally { plain.close(); editable.close();