diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5b2a7a9d8..d81ba4afa 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,14 @@ The release run heads these entries with the version and opens a fresh
## Unreleased
+- **Breaking** (wire only): an edit operation names its element by the id the
+ render writes into the page, `data-odr-id`, not by a document path. The
+ envelope is `{"version": 2, ...}`; version 1 is refused. An editable render
+ addresses its paragraphs as well as its runs.
+
+- `Element::identifier()` and `Document::element_by_id()` are the two ends of
+ that address, in C++ and in the python, jni and apple bindings.
+
- `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.
@@ -108,11 +116,11 @@ The release run heads these entries with the version and opens a fresh
beside it.
- A sheet rendered with `HtmlConfig::editable` carries no `contenteditable`
- and no `data-odr-path`: its editing is an overlay, so the markup states
+ and no `data-odr-id`: its editing is an overlay, so the markup states
none. A cell's runs fold into the `td` as they do read-only.
- **Breaking** (wire only) `Document::edit` takes an op envelope,
- `{"version": 1, "ops": [...]}`, with `setCell` writing a sheet cell by
+ `{"version": 2, "ops": [...]}`, with `setCell` writing a sheet cell by
position and `setText` carrying what the `modifiedText` map carried.
- `Sheet::set_cell` writes a repeated `.ods` cell: the run is cut into the
diff --git a/apple/include/OdrCoreObjC/ODRDocumentElement.h b/apple/include/OdrCoreObjC/ODRDocumentElement.h
index 0299eba03..bf8ddc572 100644
--- a/apple/include/OdrCoreObjC/ODRDocumentElement.h
+++ b/apple/include/OdrCoreObjC/ODRDocumentElement.h
@@ -82,6 +82,8 @@ NS_SWIFT_NAME(Element)
@interface ODRElement : NSObject
@property(nonatomic, readonly) ODRElementType type;
+/// The address an edit operation names this element by.
+@property(nonatomic, readonly) uint64_t identifier;
/// `NO` for the element you get past the end of the tree.
@property(nonatomic, readonly) BOOL exists;
diff --git a/apple/src/ODRDocumentElement.mm b/apple/src/ODRDocumentElement.mm
index aab40313a..d90e4d1bc 100644
--- a/apple/src/ODRDocumentElement.mm
+++ b/apple/src/ODRDocumentElement.mm
@@ -237,6 +237,10 @@ - (BOOL)isEditable {
return guarded_value([&] { return _handle.is_editable() ? YES : NO; }, NO);
}
+- (uint64_t)identifier {
+ return guarded_value([&] { return _handle.identifier(); }, 0);
+}
+
@end
#pragma mark - typed views
diff --git a/docs/design/document-editing.md b/docs/design/document-editing.md
new file mode 100644
index 000000000..b33c2a35a
--- /dev/null
+++ b/docs/design/document-editing.md
@@ -0,0 +1,278 @@
+# Document editing design
+
+The editor of the **document view** — `frontend/document.js` — built on the
+mode frame in [`editing.md`](editing.md), and the decisions that are its own.
+[`spreadsheet-editing.md`](spreadsheet-editing.md) is the sibling document for
+the sheet view. Text documents, presentations and drawings share this one:
+what it edits is runs and paragraphs, wherever the format puts them.
+
+Status: **the schema and the replay are landing; the browser editor follows.**
+This document is written ahead of the code, and each section says what is in
+and what is not.
+
+Scope of this work: an edit that spans several runs, a new paragraph, and a
+delete or a replace that reaches across both. Inline formatting (bold, italic,
+highlight) is *not* in it — decision 5 below says why the schema takes it later
+without changing.
+
+## What the editor has to express
+
+A reader does four things the old `setText` could not say:
+
+1. **Type over a selection that spans two runs.** The selection starts in one
+ run and ends in another, so one `setText` cannot name it.
+2. **Press Enter.** The paragraph splits, and everything after the caret moves
+ into a new paragraph.
+3. **Press Backspace at the start of a paragraph.** The paragraph merges into
+ the one before it.
+4. **Delete a selection that spans paragraphs.** Runs disappear, paragraphs
+ disappear, and the two ends become one paragraph.
+
+Every one of them creates or destroys elements, which is why the schema and the
+adapters both change.
+
+## Decisions
+
+### 1. An op names an element by its id, not by its path
+
+`data-odr-path` is replaced by `data-odr-id`, carrying the
+`ElementIdentifier` the registry already assigns. `Document::element_by_id`
+turns it back into an `Element` at replay. This is decision 4 of
+[`editing.md`](editing.md), now that there is something that needs it.
+
+**Why:** a `DocumentPath` is positional. The moment an op inserts a paragraph,
+every path recorded after it in the same log names a different element, so a
+log of more than one structural op cannot be replayed. Ids do not move.
+
+**Why it is safe:** an id has to hold for one `translate → edit → save`, and a
+registry id is the index of a `std::deque` that only grows. The ops below
+append and unlink; none of them renumbers. The sheet write side already creates
+elements after the parse and leaves the old ones unreachable
+([`odf/AGENTS.md`](../../src/odr/internal/odf/AGENTS.md)), so the discipline is
+one the engines keep already.
+
+**The cost:** an id is meaningless outside the render that wrote it. A path was
+readable and could be written by hand; an id cannot. `back_translate` replays
+against a fresh decode of the same file, and a fresh decode of the same bytes
+assigns the same ids, because parsing is deterministic. Nothing else read the
+attribute.
+
+### 2. The wire carries no character offsets
+
+An op names whole elements and whole strings. There is no `(id, start,
+length)`. A reader typing in the middle of a run produces `setText` with the
+run's new text, not an insertion at an offset.
+
+**Why:** JavaScript counts a string in UTF-16 code units and `std::string`
+counts bytes, so an offset on the wire needs a conversion on one side and a
+rule about which side that is. An emoji, a combining accent and a `text:s`
+run-of-spaces each make the two disagree. Nothing in the feature needs the
+offset: the browser owns the model (decision 8 of [`editing.md`](editing.md)),
+so it already knows the text each run ends up with, and handing that text over
+is both shorter to write and impossible to misread.
+
+**What it costs:** the log is longer. Typing one character in the middle of a
+long run sends the whole run. The log is coalesced before it is emitted
+(decision 6 of [`editing.md`](editing.md)), so the length is per run and per
+save, not per keystroke.
+
+**Why it does not paint us into a corner:** see decision 5.
+
+### 3. A split point is a run boundary, so `splitParagraph` needs no offset
+
+Enter in the middle of a run is three ops, not one:
+
+1. `setText` — the run keeps the text before the caret.
+2. `insertText` — a new run after it holds the text after the caret.
+3. `splitParagraph` — the paragraph splits after the first run.
+
+**Why:** it keeps decision 2, and it is what the file formats do anyway. ODF
+and OOXML both represent a styled stretch of text as its own run, so a split
+inside one *is* a split of the run followed by a split of the paragraph. Doing
+it in that order makes the second step a pure move of whole children.
+
+### 4. A created element is addressed by a negative id
+
+An op that creates an element carries `"id": -1`, `-2`, … . A later op in the
+same log names the created element by the same negative number. A positive id
+is one the render wrote into the page.
+
+**Why an explicit number rather than a returned one:** replay stays a pure
+function of the log. Returning minted ids to the browser is the round trip
+architecture A exists to avoid (decision 1 of [`editing.md`](editing.md)).
+
+**Why negative rather than a reserved high range:** `ElementIdentifier` is 64
+bits and odf already spends the top bit on a positional cell id
+([`odf/AGENTS.md`](../../src/odr/internal/odf/odf_element_registry.hpp)), so a
+reserved range means an engine-by-engine collision argument. A sign has no such
+argument to make, and the address stays one integer.
+
+Replay keeps a per-log map from the negative number to the id it minted. A
+number used before it was created, or created twice, throws.
+
+### 5. Formatting fits this schema unchanged, which is why it is not in it yet
+
+Toggling bold on part of a run is, in both formats, "split the run, restyle the
+middle one". The split is decision 3's first two ops, and what is left is one
+op naming whole runs — `setMark {ids, mark, on}`. No offsets, no new
+addressing.
+
+**Why it is not in this work:** the split is the same machinery either way, and
+ODF reaches a mark through a named automatic style it may have to create, which
+is a style-registry change with nothing to do with the ops. Landing it here
+would double the size of the change for a feature nobody asked for yet.
+
+### 6. The browser applies the edit itself, so undo becomes ours
+
+The editor cancels `beforeinput` and mutates the DOM itself, rather than
+letting the browser apply the edit and reading the run back afterwards
+(decision 13 of [`editing.md`](editing.md) as it was first written).
+
+**Why it has to change:** reading the run back only works when the edit stayed
+inside one run. Contenteditable's answer to a selection spanning two paragraphs
+is browser-specific — a `
` wrapper here, a merged `
` there — and none
+of it maps onto the element tree. What we could read back afterwards would not
+be what we would have to replay.
+
+**The consequence: the browser's undo stack goes empty**, because we cancel
+every edit it was going to apply. So this work has to carry undo/redo, which
+until now was honestly refused (`canUndo` answered false and a host's button
+stayed grey). That is phase 3 item 2 of [`editing.md`](editing.md), and it
+arrives here because it is no longer optional.
+
+### 7. Read-only engines say nothing
+
+The new adapter hooks default to throwing `UnsupportedOperation`, rather than
+being pure virtual like `text_set_content`.
+
+**Why:** ten engines have a `TextAdapter` and two of them can write. A pure
+virtual costs eight identical throwing bodies and grows every time the surface
+does. `FrameAdapter` already defaults its three shape readers for the same
+reason.
+
+## The op envelope
+
+Version **2**. Version 1 is refused rather than read: it addressed by path, and
+a path in a version-2 world names the wrong element rather than none.
+
+```json
+{"version": 2, "ops": [{"op": "setText", "id": 41, "text": "typed"}]}
+```
+
+| op | fields | what it does |
+|---|---|---|
+| `setText` | `id`, `text` | replaces the whole text of one run |
+| `insertText` | `after` or `before`, `text`, `id` | a new run beside the named one, in the same parent, so it takes the same style |
+| `removeElement` | `id` | unlinks the element and removes its nodes |
+| `splitParagraph` | `paragraph`, `after` (optional), `id` | the children after `after` move into a new paragraph that copies the style; no `after` moves all of them |
+| `mergeParagraph` | `paragraph` | takes the children of the next sibling paragraph and removes it |
+| `insertParagraph` | `after`, `id` | a fresh empty paragraph after the named one, copying its style |
+| `setCell` | `sheet`, `column`, `row`, `value` | unchanged; see [`spreadsheet-editing.md`](spreadsheet-editing.md) |
+
+Every `id` field on an op that creates an element is negative (decision 4).
+Every other id is one the page wrote.
+
+### The four reader gestures, as ops
+
+**Type over a selection spanning two runs** — `a[bc` … `de]f` becoming `aXf`:
+
+```json
+[{"op": "setText", "id": 10, "text": "aX"},
+ {"op": "setText", "id": 11, "text": "f"}]
+```
+
+**Enter in the middle of a run** — decision 3:
+
+```json
+[{"op": "setText", "id": 10, "text": "head"},
+ {"op": "insertText", "after": 10, "text": "tail", "id": -1},
+ {"op": "splitParagraph", "paragraph": 9, "after": 10, "id": -2}]
+```
+
+**Backspace at the start of a paragraph:**
+
+```json
+[{"op": "mergeParagraph", "paragraph": 9}]
+```
+
+**Delete a selection spanning three paragraphs:**
+
+```json
+[{"op": "setText", "id": 10, "text": "head"},
+ {"op": "removeElement", "id": 11},
+ {"op": "removeElement", "id": 20},
+ {"op": "setText", "id": 31, "text": "tail"},
+ {"op": "mergeParagraph", "paragraph": 9},
+ {"op": "mergeParagraph", "paragraph": 9}]
+```
+
+## The adapter surface
+
+Alongside `TextAdapter::text_set_content`, all defaulting to
+`UnsupportedOperation` (decision 7):
+
+```cpp
+// ElementAdapter
+virtual void element_remove(ElementIdentifier id) const;
+
+// TextAdapter
+virtual ElementIdentifier text_insert(ElementIdentifier anchor_id,
+ Placement where,
+ const std::string &text) const;
+
+// ParagraphAdapter
+virtual ElementIdentifier paragraph_split(ElementIdentifier id,
+ ElementIdentifier after_id) const;
+virtual void paragraph_merge_next(ElementIdentifier id) const;
+virtual ElementIdentifier paragraph_insert_after(ElementIdentifier id) const;
+```
+
+Each engine does the same three things it already does for a text edit:
+**resolve the id to its registry entry, splice the pugixml subtree, fix up the
+registry links.** Only the tag names differ — `text:p` / `text:span` against
+`w:p` / `w:r` / `a:p` / `a:r`.
+
+The shared `internal::ElementRegistry` grows the links the structural ops need:
+`unlink_child`, `insert_child_after` and `insert_child_before`. It has only
+`append_child` today, because until now nothing built a tree except a parser
+reading forward.
+
+## Which formats
+
+| Format | Engine | State |
+|---|---|---|
+| `.odt`, `.odp`, `.ods`, `.odg` | `odf` | edits and saves today; the new ops land here |
+| `.docx` | `ooxml/text` | edits and saves today; the new ops land here |
+| `.pptx` | `ooxml/presentation` | **read-only today.** It already has `text_set_content` and keeps its slide DOMs resident; what it lacks is `save`, the two flags and the capability row |
+| everything else | — | read-only, and says so by decision 7 |
+
+`.odp` needs nothing of its own: a presentation is the same odf `Document` as a
+text document, and a run inside a slide's frame is the same `text` element.
+
+## Order of work
+
+Each step is a pull request that builds and tests on its own.
+
+1. **Address by id.** `data-odr-id` on runs and paragraphs,
+ `Document::element_by_id`, `setText` by id, envelope version 2.
+2. **Runs come and go.** `insertText` and `removeElement`, the registry links
+ they need, odf and ooxml text. A selection spanning runs is replayable.
+3. **Paragraphs split and merge.** `splitParagraph`, `mergeParagraph`,
+ `insertParagraph`.
+4. **The browser editor.** Model-first, owns the DOM mutation, records the ops,
+ and carries undo/redo (decision 6).
+5. **pptx writes.** `save`, `is_editable`, `is_savable`, the capability row and
+ the new hooks over `a:p` / `a:r`.
+
+## Open questions
+
+- A run inside a **link** or a **bookmark** splits differently: splitting the
+ paragraph has to decide whether the link follows the tail. Today it would,
+ because the link is a child that moves whole. Whether that is right is a
+ question for step 3.
+- **A list item** is a paragraph in a list. Enter at the end of one should make
+ a new list item, not a bare paragraph. Step 3 splits what the element tree
+ says is a paragraph; the list case is not covered.
+- The **plain-text view** (`html/text_file.cpp`) is still its own editor and
+ still answers to nobody. Unchanged by this work, and still the open question
+ at the end of [`editing.md`](editing.md).
diff --git a/docs/design/editing.md b/docs/design/editing.md
index 1aa3e3340..c21c21a59 100644
--- a/docs/design/editing.md
+++ b/docs/design/editing.md
@@ -1,15 +1,16 @@
# Editing design
-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.
+Status: **the mode frame is landed for every format, and the text editor behind
+it is being built.** 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.
[`spreadsheet-editing.md`](spreadsheet-editing.md) is the first editor built on
the frame, and it is where a sheet's own decisions live.
+[`document-editing.md`](document-editing.md) is the second, and it is where the phases
+below are being carried out — an edit across runs, a new paragraph, and the
+delete and replace that reach across both.
This builds on the existing principle in [`README.md`](README.md):
@@ -26,8 +27,8 @@ edits back into the original ODF/OOXML file.
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`,
+ page-level state on ``, `data-odr-id` on every editable run and
+ paragraph, 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
@@ -236,7 +237,7 @@ editing:
| `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
+Per element the page states only the exceptions: `data-odr-id` addresses an
editable run, and `odr-locked` plus `data-odr-lock=""` marks what
refuses. Everything unmarked is editable.
@@ -272,7 +273,7 @@ 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
+`data-odr-id` 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.
@@ -285,7 +286,7 @@ 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,
+nothing. `data-odr-id` 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.
@@ -372,7 +373,7 @@ 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
+allowed because it lands inside a `x-s[data-odr-id]` 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.
diff --git a/docs/design/spreadsheet-editing.md b/docs/design/spreadsheet-editing.md
index 7d9059f62..3c6540347 100644
--- a/docs/design/spreadsheet-editing.md
+++ b/docs/design/spreadsheet-editing.md
@@ -50,7 +50,7 @@ results go stale the moment an input changes.
| 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: 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: text editor | `html/frontend/document.js` | The skeleton, attached to the mode: the whole view editable, runs keyed by `data-odr-id`, 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/...` |
@@ -78,7 +78,7 @@ value beside the op. The whole log is idempotent, which decision 5 leans on.
```json
{
- "version": 1,
+ "version": 2,
"ops": [
{"op": "setCell", "sheet": 0, "column": 1, "row": 2,
"value": {"type": "number", "number": 12.5, "text": "12.5"}},
@@ -94,10 +94,10 @@ value beside the op. The whole log is idempotent, which decision 5 leans on.
op it cannot apply, leaving the ones before it applied — a document is decoded
fresh by `DocumentFile::document()`, so the host replays onto a copy by
construction; the wasm session, which holds one document, has to replay onto a
-fresh decode too. `setText {path, text}` carries what `modifiedText` carried
-and is what `generateDiff()` now emits; it gains the id form when
-[`editing.md`](editing.md) phase 1 lands. The bindings pass a string through
-and did not change.
+fresh decode too. `setText {id, text}` carries what `modifiedText` carried and
+is what `generateDiff()` now emits, addressing its run by the id the page
+states ([`document-editing.md`](document-editing.md) decision 1). The bindings pass a
+string through and did not change.
`version` is the wire version. A document stamp (decision 7 in `editing.md`)
is deferred: a sheet op names a position, and a position is meaningful against
diff --git a/jni/java/app/opendocument/core/Element.java b/jni/java/app/opendocument/core/Element.java
index dc6d13c8f..ec540de32 100644
--- a/jni/java/app/opendocument/core/Element.java
+++ b/jni/java/app/opendocument/core/Element.java
@@ -56,6 +56,11 @@ public boolean isSame(Element other) {
}
}
+ /** The address an edit operation names this element by. */
+ public long identifier() {
+ return identifierNative(handle());
+ }
+
public DocumentPath documentPath() {
return new DocumentPath(documentPathNative(handle()));
}
@@ -208,6 +213,8 @@ final List wrapAll(long[] handles) {
private native boolean isSameNative(long handle, long otherHandle);
+ private native long identifierNative(long handle);
+
private native long documentPathNative(long handle);
private native long navigatePathNative(long handle, long pathHandle);
diff --git a/jni/src/jni_document.cpp b/jni/src/jni_document.cpp
index 780061058..2a7a99b9b 100644
--- a/jni/src/jni_document.cpp
+++ b/jni/src/jni_document.cpp
@@ -222,6 +222,13 @@ extern "C" JNIEXPORT jint JNICALL Java_app_opendocument_core_Element_typeNative(
[&] { return static_cast(element(handle).type()); });
}
+extern "C" JNIEXPORT jlong JNICALL
+Java_app_opendocument_core_Element_identifierNative(JNIEnv *env, jobject,
+ jlong handle) {
+ return guarded(
+ env, [&] { return static_cast(element(handle).identifier()); });
+}
+
extern "C" JNIEXPORT jlong JNICALL
Java_app_opendocument_core_Element_parentNative(JNIEnv *env, jobject,
jlong handle) {
diff --git a/jni/tests/app/opendocument/core/DocumentTest.java b/jni/tests/app/opendocument/core/DocumentTest.java
index 61f843ba0..f17f0715e 100644
--- a/jni/tests/app/opendocument/core/DocumentTest.java
+++ b/jni/tests/app/opendocument/core/DocumentTest.java
@@ -98,10 +98,10 @@ void editAppliesADiff() throws IOException {
assertTrue(document.isEditable());
Element paragraph = document.rootElement().firstChild();
- DocumentPath text = paragraph.firstChild().documentPath();
+ long text = paragraph.firstChild().identifier();
- document.edit("{\"version\":1,\"ops\":[{\"op\":\"setText\",\"path\":\""
- + text + "\",\"text\":\"edited by the diff\"}]}");
+ document.edit("{\"version\":2,\"ops\":[{\"op\":\"setText\",\"id\":"
+ + text + ",\"text\":\"edited by the diff\"}]}");
assertTrue(walkText(document.rootElement()).contains("edited by the diff"));
}
@@ -111,9 +111,9 @@ void saveToMemoryRoundTripsAnEdit() throws IOException {
Document document = openDocument();
Element paragraph = document.rootElement().firstChild();
- DocumentPath text = paragraph.firstChild().documentPath();
- document.edit("{\"version\":1,\"ops\":[{\"op\":\"setText\",\"path\":\""
- + text + "\",\"text\":\"saved to memory\"}]}");
+ long text = paragraph.firstChild().identifier();
+ document.edit("{\"version\":2,\"ops\":[{\"op\":\"setText\",\"id\":"
+ + text + ",\"text\":\"saved to memory\"}]}");
byte[] saved = document.saveToMemory();
assertTrue(saved.length > 0);
diff --git a/python/src/bind_document.cpp b/python/src/bind_document.cpp
index 38ab64741..dd7839e0f 100644
--- a/python/src/bind_document.cpp
+++ b/python/src/bind_document.cpp
@@ -146,6 +146,7 @@ void odr_python::bind_document(py::module_ &m) {
},
py::is_operator())
.def("type", &odr::Element::type)
+ .def("identifier", &odr::Element::identifier)
.def("parent", &odr::Element::parent, keep_self_alive)
.def("first_child", &odr::Element::first_child, keep_self_alive)
.def("previous_sibling", &odr::Element::previous_sibling, keep_self_alive)
@@ -322,6 +323,8 @@ void odr_python::bind_document(py::module_ &m) {
},
py::arg("operations"),
"Apply the operations our browser-side editor produces.")
+ .def("element_by_id", &odr::Document::element_by_id,
+ py::arg("identifier"), keep_self_alive)
.def("is_savable", &odr::Document::is_savable,
py::arg("encrypted") = false)
// saving serialises the whole document; holding the GIL for it blocks
diff --git a/python/tests/test_document.py b/python/tests/test_document.py
index 093bcd448..6b2359d02 100644
--- a/python/tests/test_document.py
+++ b/python/tests/test_document.py
@@ -136,9 +136,10 @@ def test_save_to_memory_round_trips(odt_path, tmp_path):
def test_save_to_memory_carries_an_edit(odt_path, tmp_path):
document = pyodr.open(str(odt_path)).as_document_file().document()
+ run = document.root_element().first_child().first_child()
diff = (
- '{"version":1,"ops":[{"op":"setText","path":"/child:0/child:0",'
- '"text":"edited in python"}]}'
+ '{"version":2,"ops":[{"op":"setText","id":%d,'
+ '"text":"edited in python"}]}' % run.identifier()
)
document.edit(diff)
diff --git a/src/odr/document.cpp b/src/odr/document.cpp
index c876903de..302214867 100644
--- a/src/odr/document.cpp
+++ b/src/odr/document.cpp
@@ -1,7 +1,6 @@
#include
#include
-#include
#include
#include
#include
@@ -124,10 +123,22 @@ Sheet sheet_at(const Element root, const std::uint32_t ordinal) {
void Document::edit(const std::string_view operations,
const Logger & /*logger*/) const {
const nlohmann::json json = nlohmann::json::parse(operations);
- if (json.value("version", 0) != 1) {
+ if (json.value("version", 0) != 2) {
throw std::invalid_argument("unsupported edit version");
}
+ // the element @p field names, checked to be one this document holds
+ const auto element_of = [&](const nlohmann::json &operation,
+ const char *field) {
+ const auto identifier = operation.at(field).get();
+ const Element element = element_by_id(identifier);
+ if (!element) {
+ throw std::invalid_argument("element " + std::to_string(identifier) +
+ " not found");
+ }
+ return element;
+ };
+
for (const nlohmann::json &operation : json.at("ops")) {
const auto name = operation.at("op").get();
@@ -140,16 +151,14 @@ void Document::edit(const std::string_view operations,
}
if (name == "setText") {
- const auto path = operation.at("path").get();
- const Element element = root_element().navigate_path(DocumentPath(path));
- if (!element) {
- throw std::invalid_argument("element with path " + path + " not found");
- }
- if (!element.as_text()) {
- throw std::invalid_argument("element with path " + path +
+ const Element element = element_of(operation, "id");
+ const Text text = element.as_text();
+ if (!text) {
+ throw std::invalid_argument("element " +
+ std::to_string(element.identifier()) +
" is not a text element");
}
- element.as_text().set_content(operation.at("text").get());
+ text.set_content(operation.at("text").get());
continue;
}
@@ -161,6 +170,17 @@ Element Document::root_element() const {
return {m_impl->element_adapter(), m_impl->root_element()};
}
+Element Document::element_by_id(const ElementIdentifier identifier) const {
+ const internal::abstract::ElementAdapter *adapter = m_impl->element_adapter();
+ try {
+ // throwing is how a registry answers an id it does not hold
+ static_cast(adapter->element_type(identifier));
+ } catch (const std::out_of_range &) {
+ return {};
+ }
+ return {adapter, identifier};
+}
+
Filesystem Document::as_filesystem() const {
if (std::shared_ptr files =
m_impl->as_filesystem()) {
diff --git a/src/odr/document.hpp b/src/odr/document.hpp
index 69e942096..c286896e6 100644
--- a/src/odr/document.hpp
+++ b/src/odr/document.hpp
@@ -1,5 +1,6 @@
#pragma once
+#include
#include
#include
@@ -46,11 +47,12 @@ class Document final {
/// @brief Applies @p operations to the document, in order.
///
/// The wire format our browser-side editor produces:
- /// `{"version": 1, "ops": [{"op": "setCell", "sheet": 0, "column": 1,
+ /// `{"version": 2, "ops": [{"op": "setCell", "sheet": 0, "column": 1,
/// "row": 2, "value": {"type": "number", "number": 12.5, "text": "12.5"}}]}`.
/// A value is typed `number`, `string` or `empty`; `setText` names a text
- /// element by `path` instead. Editing a single element in process is
- /// @ref Text::set_content and needs none of this.
+ /// element by the `id` the render wrote into the page instead
+ /// (`docs/design/document-editing.md`). Editing a single element in process
+ /// is @ref Text::set_content and needs none of this.
/// @throws std::invalid_argument on the first operation it cannot apply,
/// leaving the ones before it applied - a host replays onto a fresh
/// decode.
@@ -59,6 +61,10 @@ class Document final {
[[nodiscard]] Element root_element() const;
+ /// The element @ref Element::identifier handed out, or one that does not
+ /// exist where this document holds no such id.
+ [[nodiscard]] Element element_by_id(ElementIdentifier identifier) const;
+
/// The files the document is packaged from; empty for a document that is
/// one file.
[[nodiscard]] Filesystem as_filesystem() const;
diff --git a/src/odr/document_element.cpp b/src/odr/document_element.cpp
index 362fd7d68..6ac8129ef 100644
--- a/src/odr/document_element.cpp
+++ b/src/odr/document_element.cpp
@@ -106,6 +106,8 @@ ElementType Element::type() const {
return exists_() ? m_adapter->element_type(m_identifier) : ElementType::none;
}
+ElementIdentifier Element::identifier() const noexcept { return m_identifier; }
+
Element Element::parent() const {
return exists_() ? Element(m_adapter, m_adapter->element_parent(m_identifier))
: Element();
diff --git a/src/odr/document_element.hpp b/src/odr/document_element.hpp
index 0ee81e353..fdb55b0ee 100644
--- a/src/odr/document_element.hpp
+++ b/src/odr/document_element.hpp
@@ -205,6 +205,10 @@ class Element {
[[nodiscard]] ElementType type() const;
+ /// The address an edit operation names this element by; unique within one
+ /// document, and the same for every decode of the same bytes.
+ [[nodiscard]] ElementIdentifier identifier() const noexcept;
+
[[nodiscard]] Element parent() const;
[[nodiscard]] Element first_child() const;
[[nodiscard]] Element previous_sibling() const;
diff --git a/src/odr/internal/html/document_element.cpp b/src/odr/internal/html/document_element.cpp
index 2c95c39b0..eacf3811f 100644
--- a/src/odr/internal/html/document_element.cpp
+++ b/src/odr/internal/html/document_element.cpp
@@ -241,6 +241,15 @@ bool writes_edit_markup(const Element &element,
element.is_editable();
}
+/// The address an edit operation names @p element by, where the render offers
+/// editing at all.
+void write_edit_address(const Element &element, const html::WritingState &state,
+ const html::HtmlAttributeWriterCallback &clb) {
+ if (writes_edit_markup(element, state)) {
+ clb("data-odr-id", std::to_string(element.identifier()));
+ }
+}
+
/// 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,
@@ -778,9 +787,7 @@ void html::translate_text(const Element &element, const WritingState &state) {
HtmlElementOptions()
.set_inline(true)
.set_attributes([&](const HtmlAttributeWriterCallback &clb) {
- if (writes_edit_markup(element, state)) {
- clb("data-odr-path", element.document_path().to_string());
- }
+ write_edit_address(element, state, clb);
})
.set_style(translate_text_style(text.style()), state.styles()));
state.out().out() << escape_text(text.content());
@@ -817,11 +824,16 @@ void html::translate_paragraph(const Element &element,
state.out().write_element_begin(
"x-p",
- HtmlElementOptions().set_inline(true).set_style(
- "display:block;" +
- translate_paragraph_style(paragraph.style(), state.direction()) +
- translate_block_font_style(paragraph.text_style()),
- state.styles()));
+ HtmlElementOptions()
+ .set_inline(true)
+ .set_attributes([&](const HtmlAttributeWriterCallback &clb) {
+ write_edit_address(element, state, clb);
+ })
+ .set_style("display:block;" +
+ translate_paragraph_style(paragraph.style(),
+ state.direction()) +
+ translate_block_font_style(paragraph.text_style()),
+ state.styles()));
if (!marker.empty()) {
state.out().write_element_begin(
"x-s", HtmlElementOptions()
diff --git a/src/odr/internal/html/frontend/document.js b/src/odr/internal/html/frontend/document.js
index 422370c8a..2781a1f85 100644
--- a/src/odr/internal/html/frontend/document.js
+++ b/src/odr/internal/html/frontend/document.js
@@ -7,24 +7,28 @@
var odr = (window.odr = window.odr || {});
// 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) {
+ // is an overlay, and its cells state no id.
+ if (document.querySelector("x-s[data-odr-id]") === null) {
return;
}
var root = document.body;
var modified = {};
+ function idOf(run) {
+ return Number(run.getAttribute("data-odr-id"));
+ }
+
function operations() {
var ops = [];
- for (var path in modified) {
- if (Object.prototype.hasOwnProperty.call(modified, path)) {
+ for (var id in modified) {
+ if (Object.prototype.hasOwnProperty.call(modified, id)) {
ops.push({
op: "setText",
- path: path,
+ id: Number(id),
// Not `innerText`: that is the rendered text, and it drops the
// trailing space a reader just typed.
- text: modified[path].textContent,
+ text: modified[id].textContent,
});
}
}
@@ -37,7 +41,7 @@
return null;
}
var element = node.nodeType === 1 ? node : node.parentElement;
- return element === null ? null : element.closest("[data-odr-path]");
+ return element === null ? null : element.closest("x-s[data-odr-id]");
}
// The input types that only ever change the text of one run.
@@ -64,7 +68,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.
+ /// it spans two or lands outside every run. `id` is where it starts.
function target(event) {
var ranges =
typeof event.getTargetRanges === "function" ? event.getTargetRanges() : [];
@@ -78,12 +82,12 @@
}
}
if (range === undefined) {
- return { run: null, path: null };
+ return { run: null, id: 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"),
+ id: start === null ? null : idOf(start),
};
}
@@ -91,9 +95,9 @@
if (event.cancelable) {
event.preventDefault();
}
- // The path keeps two refusals apart, so Enter in one run and then in
+ // The id keeps two refusals apart, so Enter in one run and then in
// another is heard twice.
- odr.editing.refuse(reason, { path: at.path });
+ odr.editing.refuse(reason, { id: at.id });
}
// Where the edit the gate just allowed will land, for `input` to record.
@@ -164,7 +168,7 @@
odr.onError(9, "an edit landed where no operation can name it");
return;
}
- modified[run.getAttribute("data-odr-path")] = run;
+ modified[idOf(run)] = run;
odr.editing.changed();
});
diff --git a/test/browser/text/tests.html b/test/browser/text/tests.html
index 0d0141979..8d4060f15 100644
--- a/test/browser/text/tests.html
+++ b/test/browser/text/tests.html
@@ -15,18 +15,18 @@
a run under a style-only wrapper, and a paragraph holding a picture
and no run at all. -->
- first run first run a link and a taila link and a tail
- boldbold
-
+
+#include
#include
#include
#include
@@ -42,12 +43,17 @@ Sheet first_sheet(const Document &document) {
return (*document.root_element().children().begin()).as_sheet();
}
+/// The one run the @p column -th cell of the first row holds.
+Element run_of_cell(const Document &document, const std::uint32_t column) {
+ return first_sheet(document).cell(column, 0).first_child().first_child();
+}
+
} // namespace
TEST(DocumentEdit, the_ops_are_applied_in_order) {
const Document document = three_cell_sheet();
- document.edit(R"({"version":1,"ops":[)"
+ document.edit(R"({"version":2,"ops":[)"
R"({"op":"setCell","sheet":0,"column":0,"row":0,)"
R"("value":{"type":"string","text":"first"}},)"
R"({"op":"setCell","sheet":0,"column":0,"row":0,)"
@@ -59,7 +65,7 @@ TEST(DocumentEdit, the_ops_are_applied_in_order) {
TEST(DocumentEdit, a_number_op_states_the_number_and_the_text) {
const Document document = three_cell_sheet();
- document.edit(R"({"version":1,"ops":[)"
+ document.edit(R"({"version":2,"ops":[)"
R"({"op":"setCell","sheet":0,"column":1,"row":0,)"
R"("value":{"type":"number","number":12.5,"text":"12,50"}}]})");
@@ -74,7 +80,7 @@ TEST(DocumentEdit, a_number_op_states_the_number_and_the_text) {
TEST(DocumentEdit, a_number_op_without_text_spells_itself) {
const Document document = three_cell_sheet();
- document.edit(R"({"version":1,"ops":[)"
+ document.edit(R"({"version":2,"ops":[)"
R"({"op":"setCell","sheet":0,"column":1,"row":0,)"
R"("value":{"type":"number","number":12.5}}]})");
@@ -84,27 +90,49 @@ TEST(DocumentEdit, a_number_op_without_text_spells_itself) {
TEST(DocumentEdit, an_empty_op_clears_the_cell) {
const Document document = three_cell_sheet();
- document.edit(R"({"version":1,"ops":[)"
+ document.edit(R"({"version":2,"ops":[)"
R"({"op":"setCell","sheet":0,"column":2,"row":0,)"
R"("value":{"type":"empty"}}]})");
EXPECT_EQ(first_sheet(document).cell(2, 0).value().text(), "");
}
-TEST(DocumentEdit, a_text_op_names_its_element_by_path) {
+TEST(DocumentEdit, a_text_op_names_its_element_by_id) {
const Document document = three_cell_sheet();
+ const Element run = run_of_cell(document, 0);
- document.edit(
- R"({"version":1,"ops":[{"op":"setText",)"
- R"("path":"/child:0/cell:A1/child:0/child:0","text":"typed"}]})");
+ document.edit(R"({"version":2,"ops":[{"op":"setText","id":)" +
+ std::to_string(run.identifier()) + R"(,"text":"typed"}]})");
EXPECT_EQ(first_sheet(document).cell(0, 0).value().text(), "typed");
}
+TEST(DocumentEdit, a_text_op_naming_an_element_that_is_not_there_refuses) {
+ const Document document = three_cell_sheet();
+
+ EXPECT_THROW(document.edit(R"({"version":2,"ops":[)"
+ R"({"op":"setText","id":9999,"text":"typed"}]})"),
+ std::invalid_argument);
+ EXPECT_THROW(document.edit(R"({"version":2,"ops":[)"
+ R"({"op":"setText","id":0,"text":"typed"}]})"),
+ std::invalid_argument);
+}
+
+TEST(DocumentEdit, a_text_op_naming_something_that_is_not_a_run_refuses) {
+ const Document document = three_cell_sheet();
+ const ElementIdentifier cell = first_sheet(document).cell(0, 0).identifier();
+
+ EXPECT_THROW(document.edit(R"({"version":2,"ops":[{"op":"setText","id":)" +
+ std::to_string(cell) + R"(,"text":"typed"}]})"),
+ std::invalid_argument);
+}
+
TEST(DocumentEdit, an_unknown_version_refuses) {
const Document document = three_cell_sheet();
- EXPECT_THROW(document.edit(R"({"version":2,"ops":[]})"),
+ EXPECT_THROW(document.edit(R"({"version":1,"ops":[]})"),
+ std::invalid_argument);
+ EXPECT_THROW(document.edit(R"({"version":3,"ops":[]})"),
std::invalid_argument);
EXPECT_THROW(document.edit(R"({"ops":[]})"), std::invalid_argument);
}
@@ -112,14 +140,14 @@ TEST(DocumentEdit, an_unknown_version_refuses) {
TEST(DocumentEdit, an_unknown_op_refuses) {
const Document document = three_cell_sheet();
- EXPECT_THROW(document.edit(R"({"version":1,"ops":[{"op":"setStyle"}]})"),
+ EXPECT_THROW(document.edit(R"({"version":2,"ops":[{"op":"setStyle"}]})"),
std::invalid_argument);
}
TEST(DocumentEdit, an_op_naming_a_sheet_that_is_not_there_refuses) {
const Document document = three_cell_sheet();
- EXPECT_THROW(document.edit(R"({"version":1,"ops":[)"
+ EXPECT_THROW(document.edit(R"({"version":2,"ops":[)"
R"({"op":"setCell","sheet":3,"column":0,"row":0,)"
R"("value":{"type":"empty"}}]})"),
std::invalid_argument);
@@ -130,7 +158,7 @@ TEST(DocumentEdit, the_ops_before_a_refusal_are_applied) {
const Document document = three_cell_sheet();
EXPECT_ANY_THROW(
- document.edit(R"({"version":1,"ops":[)"
+ document.edit(R"({"version":2,"ops":[)"
R"({"op":"setCell","sheet":0,"column":0,"row":0,)"
R"("value":{"type":"string","text":"written"}},)"
R"({"op":"setCell","sheet":9,"column":0,"row":0,)"
diff --git a/test/src/document_test.cpp b/test/src/document_test.cpp
index 4ac62dc3e..b3c14a085 100644
--- a/test/src/document_test.cpp
+++ b/test/src/document_test.cpp
@@ -11,10 +11,14 @@
#include
+#include
+
#include
#include
#include
#include
+#include
+#include
using namespace odr;
using namespace odr::test;
@@ -66,9 +70,25 @@ void expect_text_at(const Document &document, const std::string &path,
.content());
}
-/// Applies `diff` to `path`'s document, saves to `output_name` in the working
+using TextEdits = std::vector>;
+
+/// One `setText` op per @p edits entry. A test states the path because that is
+/// what a reader can check; an op states the id the element carries.
+std::string set_text_ops(const Document &document, const TextEdits &edits) {
+ nlohmann::json ops = nlohmann::json::array();
+ for (const auto &[path, text] : edits) {
+ const Element element =
+ document.root_element().navigate_path(DocumentPath(path));
+ EXPECT_TRUE(element) << "no element at " << path;
+ ops.push_back(
+ {{"op", "setText"}, {"id", element.identifier()}, {"text", text}});
+ }
+ return nlohmann::json{{"version", 2}, {"ops", ops}}.dump();
+}
+
+/// Applies @p edits to `path`'s document, saves to `output_name` in the working
/// directory and reopens it, so the assertions see what was written.
-Document edit_and_reload(const std::string &path, const char *diff,
+Document edit_and_reload(const std::string &path, const TextEdits &edits,
const std::string &output_name) {
const Logger logger = Logger::create_stdio("odr-test", LogLevel::verbose);
@@ -76,7 +96,7 @@ Document edit_and_reload(const std::string &path, const char *diff,
open(TestData::test_file_path(path), {}, logger).as_document_file();
const Document document = document_file.document();
- document.edit(diff);
+ document.edit(set_text_ops(document, edits));
const std::string output_path =
(std::filesystem::current_path() / output_name).string();
@@ -349,10 +369,11 @@ TEST(Document, edit_docx) {
}
TEST(Document, edit_odt_diff) {
- const char *diff =
- R"({"version":1,"ops":[{"op":"setText","path":"/child:16/child:0","text":"Outasdfsdafdline"},{"op":"setText","path":"/child:24/child:0","text":"Colorasdfasdfasdfed Line"},{"op":"setText","path":"/child:6/child:0","text":"Text hello world!"}]})";
const Document document =
- edit_and_reload("odr-public/odt/style-various-1.odt", diff,
+ edit_and_reload("odr-public/odt/style-various-1.odt",
+ {{"/child:16/child:0", "Outasdfsdafdline"},
+ {"/child:24/child:0", "Colorasdfasdfasdfed Line"},
+ {"/child:6/child:0", "Text hello world!"}},
"style-various-1_edit_diff.odt");
expect_text_at(document, "/child:16/child:0", "Outasdfsdafdline");
@@ -363,11 +384,14 @@ TEST(Document, edit_odt_diff) {
// Asserted in memory: `pages.ods` is password-protected, and a decrypted
// package is not savable — see `a_decrypted_package_is_not_savable`.
TEST(Document, edit_ods_diff) {
- const char *diff =
- R"({"version":1,"ops":[{"op":"setText","path":"/child:0/cell:A1/child:0/child:0","text":"Page 1 hi"},{"op":"setText","path":"/child:1/cell:A1/child:0/child:0","text":"Page 2 hihi"},{"op":"setText","path":"/child:2/cell:A1/child:0/child:0","text":"Page 3 hihihi"},{"op":"setText","path":"/child:3/cell:A1/child:0/child:0","text":"Page 4 hihihihi"},{"op":"setText","path":"/child:4/cell:A1/child:0/child:0","text":"Page 5 hihihihihi"}]})";
const Document document = decrypted_pages_ods();
- document.edit(diff);
+ document.edit(set_text_ops(
+ document, {{"/child:0/cell:A1/child:0/child:0", "Page 1 hi"},
+ {"/child:1/cell:A1/child:0/child:0", "Page 2 hihi"},
+ {"/child:2/cell:A1/child:0/child:0", "Page 3 hihihi"},
+ {"/child:3/cell:A1/child:0/child:0", "Page 4 hihihihi"},
+ {"/child:4/cell:A1/child:0/child:0", "Page 5 hihihihihi"}}));
expect_text_at(document, "/child:0/cell:A1/child:0/child:0", "Page 1 hi");
expect_text_at(document, "/child:1/cell:A1/child:0/child:0", "Page 2 hihi");
@@ -397,11 +421,12 @@ TEST(Document, a_decrypted_package_is_not_savable) {
}
TEST(Document, edit_docx_diff) {
- const char *diff =
- R"({"version":1,"ops":[{"op":"setText","path":"/child:16/child:0/child:0","text":"Outasdfsdafdline"},{"op":"setText","path":"/child:24/child:0/child:0","text":"Colorasdfasdfasdfed Line"},{"op":"setText","path":"/child:6/child:0/child:0","text":"Text hello world!"}]})";
- const Document document =
- edit_and_reload("odr-public/docx/style-various-1.docx", diff,
- "style-various-1_edit_diff.docx");
+ const Document document = edit_and_reload(
+ "odr-public/docx/style-various-1.docx",
+ {{"/child:16/child:0/child:0", "Outasdfsdafdline"},
+ {"/child:24/child:0/child:0", "Colorasdfasdfasdfed Line"},
+ {"/child:6/child:0/child:0", "Text hello world!"}},
+ "style-various-1_edit_diff.docx");
expect_text_at(document, "/child:16/child:0/child:0", "Outasdfsdafdline");
expect_text_at(document, "/child:24/child:0/child:0",
diff --git a/test/src/html_test.cpp b/test/src/html_test.cpp
index 5839dee81..d56d46a98 100644
--- a/test/src/html_test.cpp
+++ b/test/src/html_test.cpp
@@ -208,6 +208,24 @@ std::string render_odt(const HtmlConfig &config) {
return render("odr-public/odt/about.odt", config);
}
+/// Whether any `<@p tag …>` in @p page states @p attribute. The writer puts
+/// class and style first, so the two are not adjacent.
+bool a_tag_states(const std::string &page, const std::string &tag,
+ const std::string &attribute) {
+ const std::string open = "<" + tag + " ";
+ for (std::size_t at = page.find(open); at != std::string::npos;
+ at = page.find(open, at + 1)) {
+ const std::size_t end = page.find('>', at);
+ if (end == std::string::npos) {
+ return false;
+ }
+ if (page.find(attribute, at) < end) {
+ return true;
+ }
+ }
+ return false;
+}
+
} // namespace
// Reflowed to the viewport there is no page box to inset the text.
@@ -976,20 +994,26 @@ TEST(html, a_plain_cell_carries_no_lock) {
EXPECT_EQ(page.find(R"(class="odr-locked")"), std::string::npos);
}
-// An op names a run, so the markup addresses one. The mode writes the
-// `contenteditable`, so one page serves both modes.
+// 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());
- EXPECT_NE(page.find("data-odr-path"), std::string::npos);
+ EXPECT_TRUE(a_tag_states(page, "x-s", "data-odr-id="));
EXPECT_EQ(page.find(R"(contenteditable="true")"), std::string::npos);
}
+// A paragraph is what a split or an insert anchors on.
+TEST(html, an_editable_text_document_addresses_its_paragraphs) {
+ const std::string page = render_odt(editing_config());
+
+ EXPECT_TRUE(a_tag_states(page, "x-p", "data-odr-id="));
+}
+
// 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);
+ EXPECT_EQ(page.find("data-odr-id"), std::string::npos);
}
// #822: a sheet cell does not break its text into lines unless the file says
diff --git a/wasm/tests/edit.test.mjs b/wasm/tests/edit.test.mjs
index ecd73b15b..b4415dc08 100644
--- a/wasm/tests/edit.test.mjs
+++ b/wasm/tests/edit.test.mjs
@@ -5,11 +5,12 @@ import { after, before, describe, it } from 'node:test';
import { Odr, OdrError, minimalOds, minimalOdt } from './helper.mjs';
-// Read out of the html rather than spelled, as the browser does.
-function firstEditablePath(html) {
- const match = html.match(/data-odr-path="([^"]+)"/);
- assert.ok(match, 'the editable render carries no data-odr-path');
- return match[1];
+// Read out of the html rather than spelled, as the browser does. The runs
+// carry the ids an op names; a paragraph carries one too, so the tag counts.
+function firstEditableRunId(html) {
+ const match = html.match(/]*data-odr-id="(\d+)"/);
+ assert.ok(match, 'the editable render carries no run id');
+ return Number(match[1]);
}
describe('edit', () => {
@@ -34,10 +35,10 @@ describe('edit', () => {
const doc = odr.open(minimalOdt('hello'), { editable: true });
try {
const { html } = doc.render(0);
- const path = firstEditablePath(html);
+ const id = firstEditableRunId(html);
doc.edit(JSON.stringify({
- version: 1,
- ops: [{ op: 'setText', path, text: 'edited in the browser' }],
+ version: 2,
+ ops: [{ op: 'setText', id, text: 'edited in the browser' }],
}));
// the edit is in the document, so the same service renders it
@@ -64,7 +65,7 @@ describe('edit', () => {
const doc = odr.open(minimalOds('hello'));
try {
doc.edit(JSON.stringify({
- version: 1,
+ version: 2,
ops: [{
op: 'setCell', sheet: 0, column: 0, row: 0,
value: { type: 'number', number: 12.5, text: '12.5' },
@@ -100,7 +101,7 @@ describe('edit', () => {
assert.equal(error.name, 'NoDocumentFile');
return true;
});
- assert.throws(() => doc.edit('{"version":1,"ops":[]}'), OdrError);
+ assert.throws(() => doc.edit('{"version":2,"ops":[]}'), OdrError);
} finally {
doc.close();
}
diff --git a/wasm/tests/render.test.mjs b/wasm/tests/render.test.mjs
index 04a52d74b..d891b074c 100644
--- a/wasm/tests/render.test.mjs
+++ b/wasm/tests/render.test.mjs
@@ -102,8 +102,8 @@ describe('render', () => {
try {
// `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'));
+ assert.ok(!plain.render(0).html.includes('data-odr-id'));
+ assert.ok(editable.render(0).html.includes('data-odr-id'));
} finally {
plain.close();
editable.close();
|