diff --git a/CHANGELOG.md b/CHANGELOG.md index dce85b746..159ad3249 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 +- A selection reaching over a picture is taken, and the picture goes with the + text: a frame carries `data-odr-id`, so an operation can name it. A frame + holding text of its own is still refused. + +- **Fix**: undoing a paragraph split threw, taking every later edit in the + session with it — the node it restored each moved child before could be the + line box, which the split had replaced. + - **Fix**: deleting across several lines of a `.txt` left stray line numbers in the gutter. The renderer writes whitespace between the number cells, so `lastChild` there was a text node and the removal took that instead of a cell. diff --git a/docs/design/document-editing.md b/docs/design/document-editing.md index 9f5d84f0f..914b65ff3 100644 --- a/docs/design/document-editing.md +++ b/docs/design/document-editing.md @@ -346,6 +346,18 @@ Two details the checks pin down: paragraph holds nothing, `` where it holds something. An edited page then looks like a re-rendered one, which is what makes the two comparable. +## A range that reaches over a picture + +`replaceRange` takes away the runs between its ends. A frame carries an +address too, so it takes away **a frame that holds no run** — a picture, a +plain shape. One that holds runs is a text box, and the text inside it is text +the reader meant to keep, so it refuses. + +What it cannot reach is a picture **alone in its paragraph**, where both ends +of the range land in a paragraph with no run: there is no run to anchor the +edit to, so the edit is taken and changes nothing. Deleting one needs a gesture +that names the frame rather than a range across text. + ## Open questions - **A list item** is a paragraph in a list. Enter at the end of one should make diff --git a/docs/design/editing.md b/docs/design/editing.md index e09736a1c..5cda776e6 100644 --- a/docs/design/editing.md +++ b/docs/design/editing.md @@ -346,7 +346,8 @@ then intercepts `beforeinput` and takes the edits it can express as operations: | a paste of plain text, over as many lines as it holds | taken: each line after the first opens a paragraph | | a composition (CJK, autocorrect, dictation) | let through and reconciled on `compositionend` | | a soft line break (`insertLineBreak`) | refused, reason `newLine` - no operation carries one | -| a range reaching over a picture or a table | refused, reason `range` - `replaceRange` takes runs and whole paragraphs away, so anything else caught between the ends would survive while the text around it went | +| a range reaching over a picture | taken: the frame carries an address, so the picture goes with the text | +| a range reaching over a text box or a table | refused, reason `range` - it holds text of its own, which the reader did not mean to lose | | anything else the browser offers (a mark, a list, a drop) | refused, reason `unsupportedEdit` | | an edit landing outside every run | refused, reason `range` | diff --git a/src/odr/internal/html/document_element.cpp b/src/odr/internal/html/document_element.cpp index eacf3811f..24e32d0ac 100644 --- a/src/odr/internal/html/document_element.cpp +++ b/src/odr/internal/html/document_element.cpp @@ -250,6 +250,15 @@ void write_edit_address(const Element &element, const html::WritingState &state, } } +/// The attributes a drawing's box takes: its address, so an operation names +/// the drawing rather than the runs inside it. +html::HtmlAttributeCallback frame_attributes(const Frame &frame, + const html::WritingState &state) { + return [&frame, &state](const html::HtmlAttributeWriterCallback &clb) { + write_edit_address(frame, state, clb); + }; +} + /// 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, @@ -1045,9 +1054,10 @@ void translate_plain_frame(const Frame &frame, const GraphicStyle &style, background = "background-color:" + color(*style.fill_color) + ";"; } state.out().write_element_begin( - "div", HtmlElementOptions().set_style(translate_frame_properties(frame) + - translate_drawing_style(style) + - background)); + "div", HtmlElementOptions() + .set_attributes(frame_attributes(frame, state)) + .set_style(translate_frame_properties(frame) + + translate_drawing_style(style) + background)); translate_children(frame.children(), state); state.out().write_element_end("div"); } @@ -1055,8 +1065,10 @@ void translate_plain_frame(const Frame &frame, const GraphicStyle &style, void translate_rect(const Frame &frame, const GraphicStyle &style, const WritingState &state) { state.out().write_element_begin( - "div", HtmlElementOptions().set_style(translate_shape_properties(frame) + - translate_drawing_style(style))); + "div", HtmlElementOptions() + .set_attributes(frame_attributes(frame, state)) + .set_style(translate_shape_properties(frame) + + translate_drawing_style(style))); translate_children(frame.children(), state); state.out().write_new_line(); state.out().write_raw( @@ -1067,8 +1079,10 @@ void translate_rect(const Frame &frame, const GraphicStyle &style, void translate_ellipse(const Frame &frame, const GraphicStyle &style, const WritingState &state) { state.out().write_element_begin( - "div", HtmlElementOptions().set_style(translate_shape_properties(frame) + - translate_drawing_style(style))); + "div", HtmlElementOptions() + .set_attributes(frame_attributes(frame, state)) + .set_style(translate_shape_properties(frame) + + translate_drawing_style(style))); state.out().write_new_line(); translate_children(frame.children(), state); state.out().write_raw( @@ -1082,10 +1096,12 @@ void translate_line(const Frame &frame, const GraphicStyle &style, state.out().write_element_begin( "svg", HtmlElementOptions() - .set_attributes(HtmlAttributesVector{ - {"xmlns", "http://www.w3.org/2000/svg"}, - {"version", "1.1"}, - {"overflow", "visible"}}) + .set_attributes([&](const HtmlAttributeWriterCallback &clb) { + clb("xmlns", "http://www.w3.org/2000/svg"); + clb("version", "1.1"); + clb("overflow", "visible"); + write_edit_address(frame, state, clb); + }) .set_style("z-index:-1;position:absolute;top:0;left:0;" + translate_drawing_style(style) + translate_drawing_transform(frame.transform()))); @@ -1120,8 +1136,10 @@ void translate_line(const Frame &frame, const GraphicStyle &style, void translate_custom_shape(const Frame &frame, const GraphicStyle &style, const WritingState &state) { state.out().write_element_begin( - "div", HtmlElementOptions().set_style(translate_shape_properties(frame) + - translate_drawing_style(style))); + "div", HtmlElementOptions() + .set_attributes(frame_attributes(frame, state)) + .set_style(translate_shape_properties(frame) + + translate_drawing_style(style))); translate_children(frame.children(), state); if (const std::optional path = frame.path(); path.has_value()) { diff --git a/src/odr/internal/html/frontend/document.js b/src/odr/internal/html/frontend/document.js index 1d19f4ca8..410dac885 100644 --- a/src/odr/internal/html/frontend/document.js +++ b/src/odr/internal/html/frontend/document.js @@ -56,14 +56,22 @@ // ------------------------------------------------------------------- page + /// The `
` or `` @p paragraph ends with, or null. A line break in + /// the middle of one is a `
` too, so only the last child counts. + function lineBoxOf(paragraph) { + var last = paragraph.lastChild; + return last !== null && (last.nodeName === "BR" || last.nodeName === "WBR") + ? last + : null; + } + /// The renderer ends a paragraph with `
` where it holds nothing and /// `` where it holds something; keeping to that makes an edited page /// look like a freshly rendered one. function refreshLineBox(paragraph) { - var last = paragraph.lastChild; - while (last !== null && (last.nodeName === "BR" || last.nodeName === "WBR")) { - paragraph.removeChild(last); - last = paragraph.lastChild; + var box; + while ((box = lineBoxOf(paragraph)) !== null) { + paragraph.removeChild(box); } // a picture is content the same way text is, as the renderer has it var holds = @@ -233,19 +241,24 @@ copy.parentNode.removeChild(copy); }); + // What moved, in order, rather than a sibling captured per node: the one + // after the last of them is the line box, which `refreshLineBox` replaces. + var moved = []; var node = stays === null ? element.firstChild : stays.nextSibling; while (node !== null) { var next = node.nextSibling; if (node.nodeName !== "BR" && node.nodeName !== "WBR") { - (function (moved, from, at) { - undoLog.push(function () { - from.insertBefore(moved, at); - }); - })(node, element, next); + moved.push(node); copy.appendChild(node); } node = next; } + undoLog.push(function () { + var box = lineBoxOf(element); + for (var i = 0; i < moved.length; ++i) { + element.insertBefore(moved[i], box); + } + }); return copy; } @@ -522,39 +535,76 @@ return caret; } - /// Removes the runs of @p paragraph that lie strictly between @p after and - /// @p before; a null end means from the first run, or to the last. + /// What @p paragraph holds that an operation can name, in document order: + /// its runs, and anything a range takes away whole. + function addressedIn(paragraph) { + return Array.prototype.filter.call( + paragraph.querySelectorAll("[data-odr-id]"), + function (element) { + return element.tagName === "X-S" || removableWhole(element); + } + ); + } + + /// Removes what @p paragraph holds strictly between @p after and @p before; + /// a null end means from the first, or to the last. function removeRunsBetween(paragraph, after, before) { - var runs = runsOf(paragraph); - var from = after === null ? 0 : runs.indexOf(after) + 1; - var to = before === null ? runs.length : runs.indexOf(before); + var held = addressedIn(paragraph); + var from = after === null ? 0 : held.indexOf(after) + 1; + var to = before === null ? held.length : held.indexOf(before); for (var i = from; i < to; ++i) { - perform(removeElement(runs[i])); + perform(removeElement(held[i])); } } - // What a range may reach over: a run, a wrapper around one, a paragraph of - // them, and the line box. A picture or a table is not, and no sequence of - // operations takes one away. + // The text a range reaches over: a run, a wrapper around one, a paragraph of + // them, and the line box. var reachable = { "X-S": 1, A: 1, "X-P": 1, BR: 1, WBR: 1 }; - /// Whether everything between @p from and @p to is text. + /// Whether a range can take @p element away whole: it carries an address, so + /// `removeElement` can name it, and holds no run to orphan. A picture is one; + /// a text box is not. + function removableWhole(element) { + return ( + element.getAttribute("data-odr-id") !== null && + element.tagName !== "X-S" && + element.tagName !== "X-P" && + element.querySelector("x-s[data-odr-id]") === null + ); + } + + /// Whether everything between @p from and @p to is text, or sits in + /// something the range takes away whole. function coversOnlyText(from, to) { var probe = document.createRange(); probe.setStartAfter(from); probe.setEndBefore(to); - var nodes = probe.cloneContents().querySelectorAll("*"); + var fragment = probe.cloneContents(); + var nodes = fragment.querySelectorAll("*"); for (var i = 0; i < nodes.length; ++i) { - if (reachable[nodes[i].tagName] !== 1) { + if (!reaches(nodes[i], fragment)) { return false; } } return true; } + /// Whether @p element is text, or sits in something taken away whole. + function reaches(element, fragment) { + if (reachable[element.tagName] === 1) { + return true; + } + for (var at = element; at !== null && at !== fragment; at = at.parentNode) { + if (removableWhole(at)) { + return true; + } + } + return false; + } + /// The paragraphs strictly between @p first and @p last, or null where - /// something that is not a paragraph lies between them - a table, a picture - /// - which is a range no sequence of operations can express. + /// something that is not a paragraph lies between them - a table, or a + /// drawing anchored beside them rather than inside one. function paragraphsBetween(first, last) { if (first.parentNode !== last.parentNode) { return null; diff --git a/test/browser/plaintext/tests.html b/test/browser/plaintext/tests.html index 3ac3dd1fa..0a0c718c0 100644 --- a/test/browser/plaintext/tests.html +++ b/test/browser/plaintext/tests.html @@ -27,6 +27,13 @@ @@ -44,6 +69,13 @@