From 524e1e56e609dbd7b2b81c0927cef723f1f2709a Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 29 Sep 2026 07:47:33 +0200 Subject: [PATCH 1/5] feat(document): align a paragraph of an odf, docx or pptx file Paragraph::set_style and the setParagraphStyle op write text_align. Any other field of ParagraphStyle refuses with UnsupportedOperation. The op takes `align` as left, center, right or justify. - odf points the paragraph at a fresh automatic style P that carries fo:text-align. It is made as a cell style is, so one base and one delta make one style. create_cell_style and create_paragraph_style now share the code that copies or inherits the base style. - docx writes w:jc at its rank in the CT_PPr sequence. Justified is `both`. - pptx writes a:pPr/@algn, and refuses start and end, because ST_TextAlignType has no value for either. The docx reader did not read w:jc="both", which is how Word writes a justified paragraph. It now reads it, so twelve docx pages of the reference output change. In each page, the only change is the alignment. The design doc also describes how the editor aligns, which the next commit adds. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Tigcf9he3w1aHfNPJoEGsd --- CHANGELOG.md | 10 + docs/design/document-editing.md | 56 +++- docs/design/editing.md | 8 +- src/odr/document.cpp | 30 +++ src/odr/document.hpp | 9 +- src/odr/document_element.cpp | 14 + src/odr/document_element.hpp | 4 + src/odr/internal/abstract/document.hpp | 6 + src/odr/internal/odf/AGENTS.md | 4 + src/odr/internal/odf/odf_document.cpp | 15 ++ src/odr/internal/odf/odf_style.cpp | 110 ++++++-- src/odr/internal/odf/odf_style.hpp | 14 + src/odr/internal/ooxml/ooxml_util.cpp | 5 +- src/odr/internal/ooxml/presentation/AGENTS.md | 4 +- .../ooxml_presentation_document.cpp | 34 +++ src/odr/internal/ooxml/text/AGENTS.md | 4 + .../ooxml/text/ooxml_text_document.cpp | 74 ++++++ test/src/document_edit_test.cpp | 242 ++++++++++++++++++ 18 files changed, 606 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 357b7bcee..4fd77a436 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,16 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- A paragraph of an odt, odp, odg, docx or pptx file takes a horizontal + alignment: `Paragraph::set_style` and the `setParagraphStyle` op write + `text_align`. The document editor aligns every paragraph that the selection + reaches through `odr.editing.format({align})`, with `left`, `center`, + `right` or `justify`. `onSelectionChange` reports `align`. The python, + Java, Objective-C and npm bindings expose it, and in Java and Objective-C a + `ParagraphStyle` can now be built and written. + +- **Fix**: a docx paragraph with `w:jc="both"` was not justified. It now is. + - The python, Java, Objective-C and npm bindings style a sheet cell (`Sheet::set_cell_style`). In Java and Objective-C, a `TableCellStyle` can now be built and written. diff --git a/docs/design/document-editing.md b/docs/design/document-editing.md index 63af8a52e..7d8d65309 100644 --- a/docs/design/document-editing.md +++ b/docs/design/document-editing.md @@ -104,6 +104,7 @@ Version 2. Version 1 addressed by path, and replay refuses it. | `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 | | `setTextStyle` | `id`, `style` | states the listed properties on one run; see [Inline formatting](#inline-formatting) | +| `setParagraphStyle` | `id`, `style` | states the alignment of one paragraph; see [Paragraph alignment](#paragraph-alignment) | | `setCell` | `sheet`, `column`, `row`, `value` | see [`spreadsheet-editing.md`](spreadsheet-editing.md) | Every `id` on an op that creates an element is negative (decision 4). Every @@ -154,7 +155,7 @@ call refuses an element of another document. The adapter hooks, all defaulting to `UnsupportedOperation` (decision 7): `element_remove`, `text_insert`, `text_set_style`, `paragraph_split`, -`paragraph_merge_next` and `paragraph_insert_after`. Each engine resolves the +`paragraph_merge_next`, `paragraph_insert_after` and `paragraph_set_style`. Each engine resolves the id to its registry entry, splices the pugixml subtree and fixes the registry links. Only the tag names differ: `text:p` and `text:span` against `w:p`, `w:r`, `a:p` and `a:r`. `internal::ElementRegistry` has `unlink_child`, @@ -333,6 +334,59 @@ run stops the fold. `background_color` with alpha 0. The bindings expose it in python, Java, Objective-C and the npm package. +## Paragraph alignment + +```json +{"op": "setParagraphStyle", "id": 9, "style": {"align": "center"}} +``` + +`align` is `left`, `center`, `right` or `justify`. It is the only key. + +| On the wire | `ParagraphStyle` | ODF `style:paragraph-properties` | docx `w:pPr` | pptx `a:pPr` | +|---|---|---|---|---| +| `left` | `TextAlign::left` | `fo:text-align="left"` | `` | `algn="l"` | +| `center` | `TextAlign::center` | `fo:text-align="center"` | `` | `algn="ctr"` | +| `right` | `TextAlign::right` | `fo:text-align="right"` | `` | `algn="r"` | +| `justify` | `TextAlign::justify` | `fo:text-align="justify"` | `` | `algn="just"` | + +`Paragraph::set_style(delta)` reaches `ParagraphAdapter::paragraph_set_style`. +It writes `text_align` only, and any other field of the delta refuses with +`UnsupportedOperation`. The C++ call also takes `start` and `end`. ODF and +docx write them as they are, and pptx refuses them, because +`ST_TextAlignType` has no value for either. + +### 17. ODF aligns through a fresh automatic paragraph style + +This is decision 12 for a paragraph. The writer copies the automatic style +that the paragraph shows, or makes a child of a named one, under a fresh +`P`. It then sets `fo:text-align` in the copy. The same base and the same +delta give one style for the length of a replay, so a selection over twenty +paragraphs of one style adds one style. + +### 18. docx and pptx write into the paragraph's own properties + +`w:pPr` and `a:pPr` belong to one paragraph, so no cut is needed. The writer +makes the element as the first child where it is missing. `CT_PPr` is a +sequence, so `w:jc` goes to its rank, as the run properties of decision 13 +do. + +### 19. The editor aligns every paragraph that the selection reaches + +`odr.editing.format({align: "center"})` sends one `setParagraphStyle` per +paragraph from the start of the selection to its end. A collapsed caret +aligns its own paragraph. A style can hold `align` and run keys together. The +paragraphs are checked first and aligned last, so a refused mark leaves no +paragraph aligned. The editor writes `text-align` on the `x-p`, which is the +declaration that `translate_paragraph_style` writes. `onSelectionChange` +reports `align` where the paragraphs agree, and it resolves `start` and `end` +against the direction of the paragraph. Two alignments of one paragraph fold +into one op, unless an op that names the paragraph lies between them. + +`formatJustifyLeft`, `formatJustifyCenter`, `formatJustifyRight` and +`formatJustifyFull` are chords, as `formatBold` is. Under scope `paragraph` +the host's `format` aligns, because an alignment moves no range, but a chord +refuses, as every chord does. + ## Open items - A list item is a paragraph in a list. Enter at the end of one makes a bare diff --git a/docs/design/editing.md b/docs/design/editing.md index d107dac87..3cfc52673 100644 --- a/docs/design/editing.md +++ b/docs/design/editing.md @@ -33,8 +33,9 @@ no live connection between the browser and C++. - The `back_translate` CLI replays an envelope onto a source document and saves it. - Inline formatting is `setTextStyle` on the wire and `odr.editing.format` in - the page. [`document-editing.md`](document-editing.md#inline-formatting) - holds its decisions. + the page, and a paragraph alignment is `setParagraphStyle`. + [`document-editing.md`](document-editing.md#inline-formatting) holds their + decisions. ## Decisions @@ -189,6 +190,7 @@ the markup. | Backspace at the start of a paragraph | taken: the paragraph merges into the one before | | a paste of plain text | taken: each line after the first opens a paragraph | | a mark (ctrl/cmd+B, I, U, or `odr.editing.format`) under scope `document` | taken: a run covered in part is cut, and the covered runs are restyled | +| an alignment (`formatJustify*`, or `odr.editing.format({align})`) | taken: every paragraph the selection reaches is aligned | | a composition (CJK, autocorrect, dictation) | let through, and each change recorded after its `input` | | a soft line break (`insertLineBreak`) | refused, reason `newLine` | | a range over a picture | taken: the frame carries an address | @@ -217,7 +219,7 @@ each `input`. A run the browser took out of the page raises `unnameableEdit`. | Scope | What the document editor takes | |---|---| | `document` (default) | everything in decision 13 | -| `paragraph` | an edit that starts and ends in one paragraph, and no formatting | +| `paragraph` | an edit or a host's `format` that starts and ends in one paragraph, and no formatting chord | Why a paragraph and not a run: Word splits runs by revision session, so a wall at a run would stand in the middle of uniform text. The editor reads the diff --git a/src/odr/document.cpp b/src/odr/document.cpp index c3ec462ba..981b01601 100644 --- a/src/odr/document.cpp +++ b/src/odr/document.cpp @@ -204,6 +204,30 @@ parse_cell_style(const nlohmann::json &json) { return {cell_style, parse_text_style(text_keys)}; } +/// The `style` of a `setParagraphStyle` op: `align` as `left`, `center`, +/// `right` or `justify`. +ParagraphStyle parse_paragraph_style(const nlohmann::json &json) { + ParagraphStyle style; + for (const auto &[key, value] : json.items()) { + if (key != "align") { + throw std::invalid_argument("unknown paragraph style property " + key); + } + const auto align = value.get(); + if (align == "left") { + style.text_align = TextAlign::left; + } else if (align == "center") { + style.text_align = TextAlign::center; + } else if (align == "right") { + style.text_align = TextAlign::right; + } else if (align == "justify") { + style.text_align = TextAlign::justify; + } else { + throw std::invalid_argument("unknown alignment " + align); + } + } + return style; +} + /// The @p ordinal -th sheet in document order, which is how an op names one. Sheet sheet_at(const Element root, const std::uint32_t ordinal) { std::uint32_t seen = 0; @@ -324,6 +348,12 @@ void Document::edit(const std::string_view operations, continue; } + if (name == "setParagraphStyle") { + paragraph_of(operation, "id") + .set_style(parse_paragraph_style(operation.at("style"))); + continue; + } + if (name == "insertText") { const auto text = operation.at("text").get(); const std::int64_t address = reserve(operation); diff --git a/src/odr/document.hpp b/src/odr/document.hpp index a76065ec2..e8896a34f 100644 --- a/src/odr/document.hpp +++ b/src/odr/document.hpp @@ -56,10 +56,11 @@ class Document final { /// `{"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` and - /// `setTextStyle` name a text 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 or @ref Text::set_style and - /// needs none of this. + /// `setTextStyle` name a text element, and `setParagraphStyle` a paragraph, + /// 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, @ref Text::set_style or + /// @ref Paragraph::set_style 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. diff --git a/src/odr/document_element.cpp b/src/odr/document_element.cpp index c700dc9d8..3bb9f1ca3 100644 --- a/src/odr/document_element.cpp +++ b/src/odr/document_element.cpp @@ -524,6 +524,20 @@ ParagraphStyle Paragraph::style() const { : ParagraphStyle(); } +void Paragraph::set_style(const ParagraphStyle &style) const { + if (!exists_()) { + return; + } + if (style.direction.has_value() || style.margin.right.has_value() || + style.margin.top.has_value() || style.margin.left.has_value() || + style.margin.bottom.has_value() || style.line_height.has_value() || + style.text_indent.has_value() || style.break_before.has_value() || + style.break_after.has_value()) { + throw UnsupportedOperation(); + } + m_adapter2->paragraph_set_style(m_identifier, style); +} + TextStyle Paragraph::text_style() const { return exists_() ? m_adapter2->paragraph_text_style(m_identifier) : TextStyle(); diff --git a/src/odr/document_element.hpp b/src/odr/document_element.hpp index 987eaa3d7..e54ad8530 100644 --- a/src/odr/document_element.hpp +++ b/src/odr/document_element.hpp @@ -454,6 +454,10 @@ class Paragraph final using ElementBase::ElementBase; [[nodiscard]] ParagraphStyle style() const; + /// States the set fields of @p style on the paragraph and leaves the rest. + /// Only `text_align` is written; any other field refuses with + /// `UnsupportedOperation`. + void set_style(const ParagraphStyle &style) const; [[nodiscard]] TextStyle text_style() const; }; diff --git a/src/odr/internal/abstract/document.hpp b/src/odr/internal/abstract/document.hpp index 7932fc163..8150ba10c 100644 --- a/src/odr/internal/abstract/document.hpp +++ b/src/odr/internal/abstract/document.hpp @@ -355,6 +355,12 @@ class ParagraphAdapter { paragraph_style(ElementIdentifier element_id) const = 0; [[nodiscard]] virtual TextStyle paragraph_text_style(ElementIdentifier element_id) const = 0; + /// States the set fields of @p style on the paragraph and leaves the rest. + virtual void + paragraph_set_style([[maybe_unused]] const ElementIdentifier element_id, + [[maybe_unused]] const ParagraphStyle &style) const { + throw UnsupportedOperation(); + } /// Splits @p element_id after @p after_id - one of its descendants, or null /// to move every child - into a new paragraph of the same style. diff --git a/src/odr/internal/odf/AGENTS.md b/src/odr/internal/odf/AGENTS.md index 6f7ec48d5..ea46044cc 100644 --- a/src/odr/internal/odf/AGENTS.md +++ b/src/odr/internal/odf/AGENTS.md @@ -133,6 +133,10 @@ unknown mimetype are tolerated. and points it at a fresh automatic style `T` (`StyleRegistry::create_text_style`): a copy of a shared automatic style plus the delta, or a child of a named style. +- **A paragraph alignment.** `paragraph_set_style` points `text:style-name` + at a fresh automatic style `P` (`StyleRegistry::create_paragraph_style`) + that carries `fo:text-align`. It is made as a cell style is, and one base + and one delta make one style. - **Cells.** `sheet_set_cell` writes `office:value-type`, `office:value` and the `text:p` under the cell, because the file states the value and shows a rendering of it. It writes through the run the cell holds, so the run keeps diff --git a/src/odr/internal/odf/odf_document.cpp b/src/odr/internal/odf/odf_document.cpp index d40a771fc..a7b322964 100644 --- a/src/odr/internal/odf/odf_document.cpp +++ b/src/odr/internal/odf/odf_document.cpp @@ -786,6 +786,21 @@ class ElementAdapter final : public AdapterBase { paragraph_text_style(const ElementIdentifier element_id) const override { return get_intermediate_style(element_id).text_style; } + /// Points the paragraph at a fresh automatic style: a copy of the one it + /// shows plus the delta, or a child of a named one. + void paragraph_set_style(const ElementIdentifier element_id, + const ParagraphStyle &style) const override { + pugi::xml_node node = get_node(element_id); + const std::string name = + m_document->style_registry().create_paragraph_style( + automatic_styles_of(node), + node.attribute("text:style-name").value(), style); + pugi::xml_attribute attribute = node.attribute("text:style-name"); + if (!attribute) { + attribute = node.prepend_attribute("text:style-name"); + } + attribute.set_value(name.c_str()); + } [[nodiscard]] TextStyle span_style(const ElementIdentifier element_id) const override { diff --git a/src/odr/internal/odf/odf_style.cpp b/src/odr/internal/odf/odf_style.cpp index 25ef8ba73..a38021e0e 100644 --- a/src/odr/internal/odf/odf_style.cpp +++ b/src/odr/internal/odf/odf_style.cpp @@ -803,6 +803,24 @@ pugi::xml_node properties_of(pugi::xml_node style, const char *name) { return xml::insert_in_sequence(style, name, order); } +const char *text_align_value(const TextAlign align) { + switch (align) { + case TextAlign::left: + return "left"; + case TextAlign::right: + return "right"; + case TextAlign::center: + return "center"; + case TextAlign::justify: + return "justify"; + case TextAlign::start: + return "start"; + case TextAlign::end: + return "end"; + } + return "start"; +} + const char *text_align_value(const HorizontalAlign align) { switch (align) { case HorizontalAlign::left: @@ -841,32 +859,9 @@ std::string StyleRegistry::create_cell_style(pugi::xml_node automatic_styles, return it->second; } - std::string name; - for (;; ++m_next_cell_style) { - name = "ce" + std::to_string(m_next_cell_style); - if (!m_index_style.contains(name)) { - break; - } - } - - const auto base_it = m_index_style.find(base); - const pugi::xml_node base_node = - base_it != std::end(m_index_style) ? base_it->second : pugi::xml_node(); - pugi::xml_node node; - // an automatic style may be shared, so it is copied; a named one is - // inherited from - if (base_node && - std::strcmp(base_node.parent().name(), "office:automatic-styles") == 0) { - node = automatic_styles.append_copy(base_node); - xml::set_attribute(node, "style:name", name.c_str()); - } else { - node = automatic_styles.append_child("style:style"); - node.append_attribute("style:name").set_value(name.c_str()); - node.append_attribute("style:family").set_value("table-cell"); - if (!base.empty()) { - node.append_attribute("style:parent-style-name").set_value(base.c_str()); - } - } + const pugi::xml_node node = create_style_( + automatic_styles, base, "table-cell", "ce", m_next_cell_style); + const std::string name = node.attribute("style:name").value(); if (cell.background_color.has_value()) { xml::set_attribute(properties_of(node, "style:table-cell-properties"), @@ -895,6 +890,69 @@ std::string StyleRegistry::create_cell_style(pugi::xml_node automatic_styles, return name; } +std::string +StyleRegistry::create_paragraph_style(pugi::xml_node automatic_styles, + const char *base_name, + const ParagraphStyle &style) { + const std::string base = base_name != nullptr ? base_name : ""; + const std::string key = fmt::format( + "{}|{}", base, + style.text_align ? std::to_string(static_cast(*style.text_align)) + : "-"); + if (const auto it = m_created_paragraph_styles.find(key); + it != std::end(m_created_paragraph_styles)) { + return it->second; + } + + const pugi::xml_node node = create_style_(automatic_styles, base, "paragraph", + "P", m_next_paragraph_style); + const std::string name = node.attribute("style:name").value(); + + if (style.text_align.has_value()) { + xml::set_attribute(properties_of(node, "style:paragraph-properties"), + "fo:text-align", text_align_value(*style.text_align)); + } + + m_index_style[name] = node; + generate_style_(name, node); + m_created_paragraph_styles.emplace(key, name); + return name; +} + +pugi::xml_node StyleRegistry::create_style_(pugi::xml_node automatic_styles, + const std::string &base_name, + const char *family, + const char *prefix, + std::uint32_t &next) { + std::string name; + for (;; ++next) { + name = prefix + std::to_string(next); + if (!m_index_style.contains(name)) { + break; + } + } + + const auto base_it = m_index_style.find(base_name); + const pugi::xml_node base_node = + base_it != std::end(m_index_style) ? base_it->second : pugi::xml_node(); + // an automatic style may be shared, so it is copied; a named one is + // inherited from + if (base_node && + std::strcmp(base_node.parent().name(), "office:automatic-styles") == 0) { + pugi::xml_node node = automatic_styles.append_copy(base_node); + xml::set_attribute(node, "style:name", name.c_str()); + return node; + } + pugi::xml_node node = automatic_styles.append_child("style:style"); + node.append_attribute("style:name").set_value(name.c_str()); + node.append_attribute("style:family").set_value(family); + if (!base_name.empty()) { + node.append_attribute("style:parent-style-name") + .set_value(base_name.c_str()); + } + return node; +} + std::string StyleRegistry::create_text_style(pugi::xml_node automatic_styles, const char *base_name, const TextStyle &style) { diff --git a/src/odr/internal/odf/odf_style.hpp b/src/odr/internal/odf/odf_style.hpp index 88b3e6065..02ea9e2e0 100644 --- a/src/odr/internal/odf/odf_style.hpp +++ b/src/odr/internal/odf/odf_style.hpp @@ -94,12 +94,19 @@ class StyleRegistry final { const char *base_name, const TableCellStyle &cell, const TextStyle &text); + /// A paragraph style carrying the `text_align` of @p style, made as a cell + /// style is. + std::string create_paragraph_style(pugi::xml_node automatic_styles, + const char *base_name, + const ParagraphStyle &style); private: /// Where the search for a free `T` name starts. std::uint32_t m_next_text_style{1}; std::uint32_t m_next_cell_style{1}; + std::uint32_t m_next_paragraph_style{1}; std::unordered_map m_created_cell_styles; + std::unordered_map m_created_paragraph_styles; std::unordered_map m_index_font_face; std::unordered_map m_index_default_style; @@ -125,6 +132,13 @@ class StyleRegistry final { Style *generate_style_(const std::string &name, pugi::xml_node node); void generate_master_pages_(Document &); + + /// A style of @p family under @p automatic_styles named @p prefix and the + /// first free number from @p next: a copy of the automatic style + /// @p base_name names, a child of a named one. + pugi::xml_node create_style_(pugi::xml_node automatic_styles, + const std::string &base_name, const char *family, + const char *prefix, std::uint32_t &next); }; } // namespace odr::internal::odf diff --git a/src/odr/internal/ooxml/ooxml_util.cpp b/src/odr/internal/ooxml/ooxml_util.cpp index e5f7c3318..91cca7f5e 100644 --- a/src/odr/internal/ooxml/ooxml_util.cpp +++ b/src/odr/internal/ooxml/ooxml_util.cpp @@ -379,7 +379,8 @@ ooxml::read_font_style_attribute(const pugi::xml_attribute attribute) { return font_style_from_value(attribute.value()); } -/// [ECMA-376] 17.18.44 ST_Jc. `start`/`end` are relative to the direction. +/// [ECMA-376] 17.18.44 ST_Jc. `start`/`end` are relative to the direction, +/// and `both` is justified. std::optional ooxml::read_text_align_attribute(const pugi::xml_attribute attribute) { const char *val = attribute.value(); @@ -398,7 +399,7 @@ ooxml::read_text_align_attribute(const pugi::xml_attribute attribute) { if (std::strcmp("center", val) == 0) { return TextAlign::center; } - if (std::strcmp("justify", val) == 0) { + if (std::strcmp("both", val) == 0 || std::strcmp("justify", val) == 0) { return TextAlign::justify; } return {}; diff --git a/src/odr/internal/ooxml/presentation/AGENTS.md b/src/odr/internal/ooxml/presentation/AGENTS.md index 82b778b4f..553c322e8 100644 --- a/src/odr/internal/ooxml/presentation/AGENTS.md +++ b/src/odr/internal/ooxml/presentation/AGENTS.md @@ -61,7 +61,9 @@ default of 10in × 7.5in when absent. docx. `text_set_style` cuts the `a:r` around the run and writes the toggles and the size as `a:rPr` attributes, the colour as `a:solidFill` and the highlight as `a:highlight`, each at its place in the -`CT_TextCharacterProperties` sequence ([ECMA-376] 21.1.2.3.9). `save` +`CT_TextCharacterProperties` sequence ([ECMA-376] 21.1.2.3.9). +`paragraph_set_style` writes `algn` on the `a:pPr` of the paragraph and +refuses `start` and `end`, which `ST_TextAlignType` does not name. `save` re-serialises the slide parts and byte-copies the rest. The slides are held by `r:id`, so `save` keeps the path to `r:id` map to know which part it writes. diff --git a/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp b/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp index 8d84ece9b..2bb11684c 100644 --- a/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp +++ b/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp @@ -297,6 +297,40 @@ class ElementAdapter final : public AdapterBase { paragraph_text_style(const ElementIdentifier element_id) const override { return get_intermediate_style(element_id).text_style; } + /// [ECMA-376] 20.1.10.59 `ST_TextAlignType` names no `start` and no `end`, + /// so those refuse. + void paragraph_set_style(const ElementIdentifier element_id, + const ParagraphStyle &style) const override { + const char *algn = nullptr; + if (style.text_align.has_value()) { + switch (*style.text_align) { + case TextAlign::left: + algn = "l"; + break; + case TextAlign::right: + algn = "r"; + break; + case TextAlign::center: + algn = "ctr"; + break; + case TextAlign::justify: + algn = "just"; + break; + case TextAlign::start: + case TextAlign::end: + throw UnsupportedOperation(); + } + } + pugi::xml_node node = get_node(element_id); + pugi::xml_node properties = node.child("a:pPr"); + if (!properties) { + // the schema wants it ahead of the runs + properties = node.prepend_child("a:pPr"); + } + if (algn != nullptr) { + xml::set_attribute(properties, "algn", algn); + } + } [[nodiscard]] TextStyle span_style(const ElementIdentifier element_id) const override { diff --git a/src/odr/internal/ooxml/text/AGENTS.md b/src/odr/internal/ooxml/text/AGENTS.md index 3650acb59..e86cb317c 100644 --- a/src/odr/internal/ooxml/text/AGENTS.md +++ b/src/odr/internal/ooxml/text/AGENTS.md @@ -86,6 +86,10 @@ replaces an existing one whole. A highlight is `w:highlight` for one of the sixteen names and `w:shd` otherwise. The reader takes `w:shd` only where no highlight names a colour ([ECMA-376] 17.3.2.32). +`paragraph_set_style` writes `w:jc` into the `w:pPr` of the paragraph, at its +rank in `CT_PPr` (`paragraph_property_order`). Justified is `both` +([ECMA-376] 17.18.44), and the reader takes `both` and `justify`. + ## Module layout | File (`text/`) | Role | diff --git a/src/odr/internal/ooxml/text/ooxml_text_document.cpp b/src/odr/internal/ooxml/text/ooxml_text_document.cpp index 978f6e0b1..6e9da4020 100644 --- a/src/odr/internal/ooxml/text/ooxml_text_document.cpp +++ b/src/odr/internal/ooxml/text/ooxml_text_document.cpp @@ -211,6 +211,64 @@ constexpr std::array run_property_order{ "w:specVanish", "w:oMath"}; +/// [ECMA-376] 17.3.1.26 `CT_PPr`: a sequence, as `CT_RPr` is. +constexpr std::array paragraph_property_order{ + "w:pStyle", + "w:keepNext", + "w:keepLines", + "w:pageBreakBefore", + "w:framePr", + "w:widowControl", + "w:numPr", + "w:suppressLineNumbers", + "w:pBdr", + "w:shd", + "w:tabs", + "w:suppressAutoHyphens", + "w:kinsoku", + "w:wordWrap", + "w:overflowPunct", + "w:topLinePunct", + "w:autoSpaceDE", + "w:autoSpaceDN", + "w:bidi", + "w:adjustRightInd", + "w:snapToGrid", + "w:spacing", + "w:ind", + "w:contextualSpacing", + "w:mirrorIndents", + "w:suppressOverlap", + "w:jc", + "w:textDirection", + "w:textAlignment", + "w:textboxTightWrap", + "w:outlineLvl", + "w:divId", + "w:cnfStyle", + "w:rPr", + "w:sectPr", + "w:pPrChange"}; + +/// [ECMA-376] 17.18.44 `ST_Jc`; justified is `both`. +const char *jc_value(const TextAlign align) { + switch (align) { + case TextAlign::left: + return "left"; + case TextAlign::right: + return "right"; + case TextAlign::center: + return "center"; + case TextAlign::justify: + return "both"; + case TextAlign::start: + return "start"; + case TextAlign::end: + return "end"; + } + return "start"; +} + /// Replaces @p name whole at its place in the sequence, since a stale /// `w:themeColor` would win over a new `w:val`. pugi::xml_node set_run_property(pugi::xml_node properties, const char *name) { @@ -332,6 +390,22 @@ class ElementAdapter final : public AdapterBase { paragraph_text_style(const ElementIdentifier element_id) const override { return get_intermediate_style(element_id).text_style; } + void paragraph_set_style(const ElementIdentifier element_id, + const ParagraphStyle &style) const override { + pugi::xml_node node = get_node(element_id); + pugi::xml_node properties = node.child("w:pPr"); + if (!properties) { + properties = node.prepend_child("w:pPr"); + } + if (style.text_align.has_value()) { + pugi::xml_node jc = properties.child("w:jc"); + if (!jc) { + jc = xml::insert_in_sequence(properties, "w:jc", + paragraph_property_order); + } + xml::set_attribute(jc, "w:val", jc_value(*style.text_align)); + } + } [[nodiscard]] TextStyle span_style(const ElementIdentifier element_id) const override { diff --git a/test/src/document_edit_test.cpp b/test/src/document_edit_test.cpp index 160cd6ac7..8a131fa42 100644 --- a/test/src/document_edit_test.cpp +++ b/test/src/document_edit_test.cpp @@ -1183,3 +1183,245 @@ TEST(DocumentEdit, an_insert_naming_a_parent_and_a_run_to_sit_beside_refuses) { id_of(run_at(document, 0, 0)) + R"(,"text":"x","id":-1})")), std::invalid_argument); } + +namespace { + +/// Two paragraphs sharing an automatic style with a margin, one under a named +/// centered style, and a bare one. +Document aligned_text() { + const std::string source = + R"()" + R"()" + R"()" + R"()" + R"()" + R"()" + R"()" + R"()" + R"()" + R"()" + R"()" + R"(one)" + R"(two)" + R"(three)" + R"(four)" + R"()"; + return DecodedFile( + open_strategy::open_file(std::make_shared(source), {}, + Logger::null())) + .as_document_file() + .document(); +} + +std::string align_op(const Element paragraph, const std::string &align) { + return R"({"op":"setParagraphStyle","id":)" + id_of(paragraph) + + R"(,"style":{"align":")" + align + R"("}})"; +} + +std::optional align_of(const Element paragraph) { + return paragraph.as_paragraph().style().text_align; +} + +/// The @p ordinal -th paragraph anywhere in @p document. +Element nth_paragraph(const Document &document, const std::uint32_t ordinal) { + std::uint32_t seen = 0; + const auto walk = [&](this auto &&self, const Element element) -> Element { + if (element.type() == ElementType::paragraph && seen++ == ordinal) { + return element; + } + for (const Element child : element.children()) { + if (const Element found = self(child)) { + return found; + } + } + return {}; + }; + return walk(document.root_element()); +} + +} // namespace + +TEST(DocumentEdit, an_align_op_on_a_bare_paragraph_gives_it_a_style) { + const Document document = aligned_text(); + const Element paragraph = paragraph_at(document, 3); + ASSERT_EQ(align_of(paragraph), std::nullopt); + + document.edit(ops(align_op(paragraph, "right"))); + + EXPECT_EQ(align_of(paragraph), TextAlign::right); +} + +TEST(DocumentEdit, an_align_op_copies_a_shared_automatic_style) { + const Document document = aligned_text(); + const Element one = paragraph_at(document, 0); + const Element two = paragraph_at(document, 1); + + document.edit(ops(align_op(one, "justify"))); + + EXPECT_EQ(align_of(one), TextAlign::justify); + ASSERT_TRUE(one.as_paragraph().style().margin.left.has_value()); + EXPECT_EQ(one.as_paragraph().style().margin.left->to_string(), "1in"); + EXPECT_EQ(align_of(two), std::nullopt); +} + +TEST(DocumentEdit, an_align_op_under_a_named_style_overrides_it) { + const Document document = aligned_text(); + const Element three = paragraph_at(document, 2); + ASSERT_EQ(align_of(three), TextAlign::center); + + document.edit(ops(align_op(three, "left"))); + + EXPECT_EQ(align_of(three), TextAlign::left); +} + +TEST(DocumentEdit, two_paragraphs_aligned_alike_share_one_style) { + const Document document = aligned_text(); + + document.edit(ops(align_op(paragraph_at(document, 0), "center") + "," + + align_op(paragraph_at(document, 1), "center"))); + + // one for the named style, one for the two paragraphs + const std::string content(document.save_to_memory().memory_data().value()); + const std::string center = R"(fo:text-align="center")"; + std::size_t count = 0; + for (std::size_t at = content.find(center); at != std::string::npos; + at = content.find(center, at + 1)) { + ++count; + } + EXPECT_EQ(count, 2U); +} + +TEST(DocumentEdit, an_align_op_survives_a_save) { + const Document document = aligned_text(); + document.edit(ops(align_op(paragraph_at(document, 0), "center") + "," + + align_op(paragraph_at(document, 3), "right"))); + + const Document saved = reopened(document); + + EXPECT_EQ(align_of(paragraph_at(saved, 0)), TextAlign::center); + EXPECT_EQ(align_of(paragraph_at(saved, 1)), std::nullopt); + EXPECT_EQ(align_of(paragraph_at(saved, 3)), TextAlign::right); +} + +TEST(DocumentEdit, an_align_op_with_an_unknown_value_refuses) { + const Document document = aligned_text(); + const Element paragraph = paragraph_at(document, 3); + + EXPECT_THROW(document.edit(ops(align_op(paragraph, "middle"))), + std::invalid_argument); + EXPECT_THROW( + document.edit(ops(R"({"op":"setParagraphStyle","id":)" + + id_of(paragraph) + R"(,"style":{"indent":"1in"}})")), + std::invalid_argument); + EXPECT_THROW(document.edit(ops(align_op(run_at(document, 3, 0), "left"))), + std::invalid_argument); + EXPECT_EQ(align_of(paragraph), std::nullopt); +} + +TEST(DocumentEdit, a_read_only_engine_refuses_an_align_op) { + const Document document = + DecodedFile(open_strategy::open_file(std::make_shared( + std::string(R"({\rtf1 hello})")), + {}, Logger::null())) + .as_document_file() + .document(); + const Element paragraph = nth_paragraph(document, 0); + ASSERT_TRUE(paragraph); + + EXPECT_THROW(document.edit(ops(align_op(paragraph, "center"))), + UnsupportedOperation); +} + +TEST(DocumentEdit, a_paragraph_style_the_handle_does_not_write_refuses) { + const Document document = aligned_text(); + ParagraphStyle style; + style.text_align = TextAlign::center; + style.line_height = Measure("12pt"); + + EXPECT_THROW(paragraph_at(document, 3).as_paragraph().set_style(style), + UnsupportedOperation); + EXPECT_EQ(align_of(paragraph_at(document, 3)), std::nullopt); +} + +TEST(DocumentEdit, docx_an_align_op_writes_jc_in_schema_order) { + const Document document = + docx_of(R"()" + R"(one)" + R"(two)"); + const Element one = nth_paragraph(document, 0); + const Element two = nth_paragraph(document, 1); + + document.edit(ops(align_op(one, "justify") + "," + align_op(two, "center"))); + + EXPECT_EQ(align_of(one), TextAlign::justify); + EXPECT_EQ(align_of(two), TextAlign::center); + const std::string xml = part_of(document, "word/document.xml"); + EXPECT_NE(xml.find(R"()" + R"()"), + std::string::npos); + EXPECT_NE(xml.find(R"()"), + std::string::npos); +} + +TEST(DocumentEdit, docx_an_align_op_replaces_the_jc_there_is) { + const Document document = docx_of( + R"(one)"); + const Element paragraph = nth_paragraph(document, 0); + ASSERT_EQ(align_of(paragraph), TextAlign::right); + + document.edit(ops(align_op(paragraph, "left"))); + + EXPECT_EQ(align_of(paragraph), TextAlign::left); + const std::string xml = part_of(document, "word/document.xml"); + EXPECT_EQ(xml.find("w:jc"), xml.rfind("w:jc")); +} + +TEST(DocumentEdit, docx_an_align_op_survives_a_save) { + const Document document = docx_of(docx_paragraphs); + document.edit(ops(align_op(nth_paragraph(document, 1), "justify"))); + + const Document saved = reopened(document); + + EXPECT_EQ(align_of(nth_paragraph(saved, 0)), std::nullopt); + EXPECT_EQ(align_of(nth_paragraph(saved, 1)), TextAlign::justify); + EXPECT_EQ(nth_run(saved, 1).as_text().style().font_weight, FontWeight::bold); +} + +TEST(DocumentEdit, pptx_an_align_op_writes_algn) { + const Document document = + pptx_of(R"(one)" + R"(two)"); + const Element one = nth_paragraph(document, 0); + const Element two = nth_paragraph(document, 1); + + document.edit(ops(align_op(one, "center") + "," + align_op(two, "justify"))); + + EXPECT_EQ(align_of(one), TextAlign::center); + EXPECT_EQ(align_of(two), TextAlign::justify); + const std::string xml = part_of(document, "ppt/slides/slide1.xml"); + EXPECT_NE(xml.find(R"()"), + std::string::npos); + EXPECT_NE(xml.find(R"()"), std::string::npos); +} + +TEST(DocumentEdit, pptx_an_align_op_survives_a_save) { + const Document document = pptx_of(pptx_paragraphs); + document.edit(ops(align_op(nth_paragraph(document, 0), "right"))); + + const Document saved = reopened(document); + + EXPECT_EQ(align_of(nth_paragraph(saved, 0)), TextAlign::right); + EXPECT_EQ(align_of(nth_paragraph(saved, 1)), std::nullopt); +} + +TEST(DocumentEdit, pptx_an_align_relative_to_the_direction_refuses) { + const Document document = pptx_of(pptx_paragraphs); + ParagraphStyle style; + style.text_align = TextAlign::start; + + EXPECT_THROW(nth_paragraph(document, 0).as_paragraph().set_style(style), + UnsupportedOperation); + EXPECT_EQ(part_of(document, "ppt/slides/slide1.xml").find("a:pPr"), + std::string::npos); +} From 85f797d7000b386b804bb33ccdcd5f19adfa354b Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 29 Sep 2026 07:47:34 +0200 Subject: [PATCH 2/5] feat(html): the document editor aligns paragraphs odr.editing.format({align}) sends one setParagraphStyle for each paragraph that the selection reaches, and a collapsed caret aligns its own paragraph. A style can hold align and run keys together. The editor checks the paragraphs first and aligns them last, so a refused mark leaves no paragraph aligned. It writes text-align on the x-p, as the renderer does. onSelectionChange reports `align` where the paragraphs agree. Two alignments of one paragraph fold into one op. The formatJustify* input types are chords, and under scope `paragraph` they refuse as every chord does. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Tigcf9he3w1aHfNPJoEGsd --- src/odr/internal/html/frontend/document.js | 160 +++++++++++++++++++-- src/odr/internal/html/frontend/editing.js | 5 +- test/browser/text/tests.html | 97 ++++++++++++- 3 files changed, 247 insertions(+), 15 deletions(-) diff --git a/src/odr/internal/html/frontend/document.js b/src/odr/internal/html/frontend/document.js index 94642c146..75eb34a52 100644 --- a/src/odr/internal/html/frontend/document.js +++ b/src/odr/internal/html/frontend/document.js @@ -180,12 +180,13 @@ } function withStyle(op, style) { - return { op: "setTextStyle", id: op.id, style: merged(op.style, style) }; + return { op: op.op, id: op.id, style: merged(op.style, style) }; } /// Folds what a save need not carry: several edits to one run are the text /// it ends at, a run created and typed into is one insert, and two marks on - /// one run are one where the later keys win. + /// one run, or two alignments of one paragraph, are one where the later + /// keys win. function coalesce(ops) { var result = []; for (var i = 0; i < ops.length; ++i) { @@ -205,6 +206,13 @@ continue; } } + if (op.op === "setParagraphStyle") { + at = foldable(result, op, ["setParagraphStyle"], []); + if (at !== -1) { + result[at] = withStyle(result[at], op.style); + continue; + } + } result.push(op); } return result; @@ -415,6 +423,16 @@ }; } + /// Puts @p element's `style` attribute back to @p before. + function restoreStyle(element, before) { + // Chrome serialises a declaration set through `style` into an empty + // attribute after a bare `removeAttribute`, so it is stated first + element.setAttribute("style", before === null ? "" : before); + if (before === null) { + element.removeAttribute("style"); + } + } + /// States @p style on @p run as the renderer writes it; undo puts the /// attribute back as it was. function setRunStyle(run, style) { @@ -425,12 +443,22 @@ paint(run, style); }, revert: function () { - // Chrome serialises a declaration set through `style` into an empty - // attribute after a bare `removeAttribute`, so it is stated first - run.setAttribute("style", before === null ? "" : before); - if (before === null) { - run.removeAttribute("style"); - } + restoreStyle(run, before); + }, + }; + } + + /// States @p style on @p paragraph as `translate_paragraph_style` writes + /// it; undo puts the attribute back as it was. + function setParagraphStyle(paragraph, style) { + var before = paragraph.getAttribute("style"); + return { + ops: [{ op: "setParagraphStyle", id: idOf(paragraph), style: style }], + apply: function () { + paragraph.style.textAlign = style.align; + }, + revert: function () { + restoreStyle(paragraph, before); }, }; } @@ -475,6 +503,17 @@ formatStrikeThrough: "strikethrough", }; + // the values of `align`, which `setParagraphStyle` carries + var alignments = ["left", "center", "right", "justify"]; + + // the input types an alignment chord raises, and the value each one states + var aligning = { + formatJustifyLeft: "left", + formatJustifyCenter: "center", + formatJustifyRight: "right", + formatJustifyFull: "justify", + }; + /// `#rrggbb` for a computed `rgb(…)`, null for a transparent one. function hexOf(computed) { var match = /^rgba?\((\d+),\s*(\d+),\s*(\d+)(?:,\s*([\d.]+))?\)$/.exec( @@ -532,6 +571,21 @@ return result; } + /// The alignment @p paragraph shows, as the wire spells it; `start` and + /// `end` resolved against its direction. + function alignOf(paragraph) { + var computed = window.getComputedStyle(paragraph); + var align = computed.textAlign; + var rtl = computed.direction === "rtl"; + if (align === "start") { + return rtl ? "right" : "left"; + } + if (align === "end") { + return rtl ? "left" : "right"; + } + return alignments.indexOf(align) === -1 ? null : align; + } + /// Writes @p style into @p run's `style` attribute as `translate_text_style` /// does: one `text-decoration` for both lines, none for a highlight removed. function paint(run, style) { @@ -588,7 +642,8 @@ return all.slice(all.indexOf(from), all.indexOf(to) + 1); } - /// Whether @p style names only properties the wire carries. + /// Whether @p style names only properties the wire carries, and an + /// `align` it knows. function knownStyle(style) { if (style === null || typeof style !== "object") { return false; @@ -596,7 +651,11 @@ var any = false; for (var key in style) { if (Object.prototype.hasOwnProperty.call(style, key)) { - if (properties.indexOf(key) === -1) { + if (key === "align") { + if (alignments.indexOf(style.align) === -1) { + return false; + } + } else if (properties.indexOf(key) === -1) { return false; } any = true; @@ -605,6 +664,33 @@ return any; } + /// The run keys of @p style, or null where it has none. + function textKeysOf(style) { + var result = null; + for (var key in style) { + if (Object.prototype.hasOwnProperty.call(style, key) && key !== "align") { + result = result || {}; + result[key] = style[key]; + } + } + return result; + } + + /// The paragraphs @p at reaches, in document order; null where one between + /// its ends cannot be named. + function paragraphsAt(at) { + var first = at.start.paragraph; + var last = at.end.paragraph; + if (first === null || last === null) { + return null; + } + if (first === last) { + return [first]; + } + var between = paragraphsBetween(first, last); + return between === null ? null : [first].concat(between, [last]); + } + /// A collapsed range grown to the word the caret sits in, as Word does; /// null at a word boundary. A range that is not collapsed is itself. function wordAround(at) { @@ -772,21 +858,45 @@ refuse(null, "outOfScope", at); return false; } + // every paragraph has to be nameable before any run is marked, or a + // refusal would leave half a gesture on the page + var paragraphs = []; + if (style.align !== undefined) { + paragraphs = paragraphsAt(at); + if (paragraphs === null) { + refuse(null, "range", at); + return false; + } + } + var align = function () { + for (var i = 0; i < paragraphs.length; ++i) { + perform(setParagraphStyle(paragraphs[i], { align: style.align })); + } + }; + + var text = textKeysOf(style); + if (text === null) { + align(); + reportSelection(true); + return true; + } if (isCollapsed(at)) { var word = wordAround(at); if (word === null) { - pending = pendingApplies(at) ? merged(pending, style) : style; + align(); + pending = pendingApplies(at) ? merged(pending, text) : text; pendingAt = at.start; reportSelection(true); return true; } at = word; } - var marked = markRange(at, style); + var marked = markRange(at, text); if (marked === null) { refuse(null, "range", at); return false; } + align(); selectRange(marked); return true; } @@ -799,6 +909,18 @@ } var covering = isCollapsed(at) ? wordAround(at) || at : at; var summary = summaryOf(coveredRuns(covering)); + var paragraphs = paragraphsAt(at); + if (paragraphs !== null) { + var align = alignOf(paragraphs[0]); + for (var i = 1; i < paragraphs.length && align !== null; ++i) { + if (alignOf(paragraphs[i]) !== align) { + align = null; + } + } + if (align !== null) { + summary.align = align; + } + } return pendingApplies(at) ? merged(summary, pending) : summary; } @@ -1510,6 +1632,20 @@ return; } + var alignment = aligning[type]; + if (alignment !== undefined) { + event.preventDefault(); + if (!odr.takesKeys("shortcuts")) { + return; + } + if (odr.editing.scope() === "paragraph") { + refuse(event, "outOfScope", at); + return; + } + format({ align: alignment }, at); + return; + } + var text = replacing[type]; if (text === undefined) { refuse(event, named[type] || "unsupportedEdit", at); diff --git a/src/odr/internal/html/frontend/editing.js b/src/odr/internal/html/frontend/editing.js index b36de0ffd..5d513c37e 100644 --- a/src/odr/internal/html/frontend/editing.js +++ b/src/odr/internal/html/frontend/editing.js @@ -155,8 +155,9 @@ /// States @p style on the selection: `bold`, `italic`, `underline`, /// `strikethrough` (a bool), `color` (`#rrggbb`), `size` (`14pt`), and - /// `highlight` (`#rrggbb` or null) on text, `fill` (the same) and `align` - /// (`left`, `center`, `right`) on cells. False where refused. + /// `highlight` (`#rrggbb` or null) on text, `align` (`left`, `center`, + /// `right`, and `justify` on text) on every paragraph or cell it reaches, + /// and `fill` (`#rrggbb` or null) on cells. False where refused. format: function (style) { for (var i = editors.length - 1; i >= 0; --i) { if (typeof editors[i].format === "function") { diff --git a/test/browser/text/tests.html b/test/browser/text/tests.html index 745366bfa..c16d471e5 100644 --- a/test/browser/text/tests.html +++ b/test/browser/text/tests.html @@ -792,6 +792,96 @@ odr.editing.format({ bold: true }) === true && ops().length === pictureOps ); + // ------------------------------------------------------- alignment + + reset(); + select(run(31).firstChild, 2); + check("center with a caret in a paragraph is taken", odr.editing.format({ align: "center" }) === true); + check( + "and aligns the paragraph as the renderer writes it, cutting no run", + paragraph(30).style.textAlign === "center" && + run(31).textContent === "third" && + run(31).getAttribute("style") === null + ); + check( + "as one setParagraphStyle", + ops().length === 1 && + ops()[0].op === "setParagraphStyle" && + ops()[0].id === 30 && + ops()[0].style.align === "center" + ); + odr.editing.format({ align: "justify" }); + check( + "a second alignment folds into the first", + ops().length === 1 && ops()[0].style.align === "justify" + ); + document.dispatchEvent(new Event("selectionchange")); + check( + "and the selection reports it", + selections[selections.length - 1].align === "justify" + ); + check( + "an alignment can be taken back", + odr.editing.undo() === true && odr.editing.undo() === true + ); + check( + "leaving the paragraph as the renderer wrote it", + paragraph(30).getAttribute("style") === "display:block" && ops().length === 0 + ); + + reset(); + selectRuns(11, 2, 21, 2); + check("right over two paragraphs is taken", odr.editing.format({ align: "right" }) === true); + check( + "aligning both", + paragraph(10).style.textAlign === "right" && + paragraph(20).style.textAlign === "right" && + paragraph(30).style.textAlign === "" + ); + document.dispatchEvent(new Event("selectionchange")); + check( + "which the selection reports", + selections[selections.length - 1].align === "right" + ); + selectRuns(11, 2, 31, 2); + document.dispatchEvent(new Event("selectionchange")); + check( + "and leaves out where the paragraphs differ", + selections[selections.length - 1].align === undefined + ); + + reset(); + selectRuns(31, 0, 31, 3); + check( + "an alignment beside a mark is taken", + odr.editing.format({ bold: true, align: "center" }) === true + ); + check( + "marking the run and aligning its paragraph", + run(31).style.fontWeight === "bold" && + run(31).style.textAlign === "" && + paragraph(30).style.textAlign === "center" && + ops().some(function (op) { + return op.op === "setTextStyle" && op.style.align === undefined; + }) && + ops().some(function (op) { + return op.op === "setParagraphStyle" && op.id === 30; + }) + ); + check("an alignment the wire has not is refused", odr.editing.format({ align: "middle" }) === false); + check("as an edit we cannot replay", refusals.pop() === "unsupportedEdit 1007"); + selectRuns(61, 1, 65, 3); + check( + "a paragraph holding a text box aligns", + odr.editing.format({ align: "center" }) === true && + paragraph(60).style.textAlign === "center" + ); + + reset(); + select(run(21).firstChild, 1); + check("the center chord is taken", input("formatJustifyCenter") === "taken"); + check("and aligns the paragraph", paragraph(20).style.textAlign === "center"); + // ---------------------------------------------- scope paragraph reset(); @@ -810,6 +900,10 @@ selectRuns(11, 2, 31, 2); check("a format over two paragraphs is refused", odr.editing.format({ bold: true }) === false); check("as out of scope", refusals.pop().indexOf("outOfScope ") === 0); + select(run(53).firstChild, 2); + check("an alignment in one paragraph is taken", odr.editing.format({ align: "right" }) === true); + check("an alignment chord is refused", input("formatJustifyRight") === "refused"); + check("as out of scope", refusals.pop() === "outOfScope 1010"); select(run(11).firstChild, 5); check("typing inside a run is taken", input("insertText", "X") === "taken"); @@ -864,11 +958,12 @@ } check( - "the log names no paragraph operation", + "the log names no structural paragraph operation", ops().every(function (op) { return ( op.op === "setText" || op.op === "setTextStyle" || + op.op === "setParagraphStyle" || op.op === "removeElement" || op.op === "insertText" ); From 061d63386ef712bd76ed8f74ab70bb6453b5bc9b Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 29 Sep 2026 07:47:34 +0200 Subject: [PATCH 3/5] feat(bindings): Paragraph::set_style in python, java, objective-c and npm Python binds Paragraph.set_style as it is. Java and Objective-C get Paragraph.setStyle and -[ODRParagraph setStyle:error:]. A ParagraphStyle can now be built and its fields written, so a caller builds the delta. Any field but textAlign refuses before the call reaches C++. The npm package's setParagraphStyle(id, style) takes the object that the page's odr.editing.format takes, and replays it through the envelope, so the parser of the wire checks it. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Tigcf9he3w1aHfNPJoEGsd --- .../include/OdrCoreObjC/ODRDocumentElement.h | 4 ++ apple/include/OdrCoreObjC/ODRStyle.h | 16 ++++---- apple/src/ODRDocumentElement.mm | 7 ++++ apple/src/ODRPrivate.h | 3 ++ apple/src/ODRStyle.mm | 21 ++++++++++ apple/tests/OdrCoreTests.swift | 33 ++++++++++++++++ jni/java/app/opendocument/core/Paragraph.java | 10 +++++ .../app/opendocument/core/ParagraphStyle.java | 23 +++++++---- jni/src/jni_convert.hpp | 3 ++ jni/src/jni_document.cpp | 10 +++++ jni/src/jni_style.cpp | 35 +++++++++++++++++ .../app/opendocument/core/DocumentTest.java | 39 +++++++++++++++++++ python/src/bind_document.cpp | 1 + python/tests/test_document.py | 35 +++++++++++++++++ wasm/js/index.d.ts | 12 ++++++ wasm/js/index.js | 7 ++++ wasm/src/wasm_document.cpp | 21 ++++++++++ wasm/tests/edit.test.mjs | 35 +++++++++++++++++ 18 files changed, 299 insertions(+), 16 deletions(-) diff --git a/apple/include/OdrCoreObjC/ODRDocumentElement.h b/apple/include/OdrCoreObjC/ODRDocumentElement.h index 322cb2dc3..13ea25b92 100644 --- a/apple/include/OdrCoreObjC/ODRDocumentElement.h +++ b/apple/include/OdrCoreObjC/ODRDocumentElement.h @@ -194,6 +194,10 @@ NS_SWIFT_NAME(Paragraph) @interface ODRParagraph : ODRElement @property(nonatomic, readonly) ODRParagraphStyle *style; @property(nonatomic, readonly) ODRTextStyle *textStyle; +/// States the non-nil properties of `style` on the paragraph and leaves the +/// rest. Only `textAlign` is written; any other refuses with +/// `ODRErrorUnsupportedOperation`. +- (BOOL)setStyle:(ODRParagraphStyle *)style error:(NSError **)error; @end /// `odr::Span`. diff --git a/apple/include/OdrCoreObjC/ODRStyle.h b/apple/include/OdrCoreObjC/ODRStyle.h index 7a0121c43..42deb28fc 100644 --- a/apple/include/OdrCoreObjC/ODRStyle.h +++ b/apple/include/OdrCoreObjC/ODRStyle.h @@ -173,19 +173,19 @@ NS_SWIFT_NAME(TextStyle) NS_SWIFT_NAME(ParagraphStyle) @interface ODRParagraphStyle : NSObject /// `ODRTextAlign`, boxed. -@property(nonatomic, readonly, nullable) NSNumber *textAlign; +@property(nonatomic, nullable) NSNumber *textAlign; /// `ODRTextDirection`, boxed; `nil` where the style says nothing. -@property(nonatomic, readonly, nullable) NSNumber *direction; +@property(nonatomic, nullable) NSNumber *direction; @property(nonatomic, readonly) ODRDirectionalMeasure *margin; -@property(nonatomic, readonly, nullable) ODRMeasure *lineHeight; -@property(nonatomic, readonly, nullable) ODRMeasure *textIndent; +@property(nonatomic, nullable) ODRMeasure *lineHeight; +@property(nonatomic, nullable) ODRMeasure *textIndent; /// `ODRBreakType`, boxed; `nil` where the style says nothing. -@property(nonatomic, readonly, nullable) NSNumber *breakBefore; +@property(nonatomic, nullable) NSNumber *breakBefore; /// `ODRBreakType`, boxed; `nil` where the style says nothing. -@property(nonatomic, readonly, nullable) NSNumber *breakAfter; +@property(nonatomic, nullable) NSNumber *breakAfter; -- (instancetype)init NS_UNAVAILABLE; -+ (instancetype)new NS_UNAVAILABLE; +/// Every property `nil`, and no side of `margin` stated. +- (instancetype)init; @end /// Table style — `odr::TableStyle`. diff --git a/apple/src/ODRDocumentElement.mm b/apple/src/ODRDocumentElement.mm index 0f62b93d3..f596bfd1e 100644 --- a/apple/src/ODRDocumentElement.mm +++ b/apple/src/ODRDocumentElement.mm @@ -510,6 +510,13 @@ - (ODRTextStyle *)textStyle { nil); } +- (BOOL)setStyle:(ODRParagraphStyle *)style error:(NSError **)error { + return guarded(error, [&] { + self.handle.as_paragraph().set_style([style handle]); + return YES; + }); +} + @end @implementation ODRSpan diff --git a/apple/src/ODRPrivate.h b/apple/src/ODRPrivate.h index c6f6ab873..5edd688f8 100644 --- a/apple/src/ODRPrivate.h +++ b/apple/src/ODRPrivate.h @@ -109,6 +109,9 @@ NS_ASSUME_NONNULL_BEGIN @interface ODRParagraphStyle (Private) + (instancetype)styleWithHandle:(const odr::ParagraphStyle &)handle; +/// The set properties as a `ParagraphStyle`; throws `UnsupportedOperation` +/// for any but `textAlign`, which no engine writes. +- (odr::ParagraphStyle)handle; @end @interface ODRTableStyle (Private) diff --git a/apple/src/ODRStyle.mm b/apple/src/ODRStyle.mm index 99632695f..0a0b8df2a 100644 --- a/apple/src/ODRStyle.mm +++ b/apple/src/ODRStyle.mm @@ -258,6 +258,27 @@ + (instancetype)styleWithHandle:(const odr::TextStyle &)handle { @implementation ODRParagraphStyle +- (instancetype)init { + if ((self = [super init]) != nil) { + _margin = [ODRDirectionalMeasure + directionalWithHandle:odr::DirectionalStyle()]; + } + return self; +} + +- (odr::ParagraphStyle)handle { + if (_direction != nil || _margin.right != nil || _margin.top != nil || + _margin.left != nil || _margin.bottom != nil || _lineHeight != nil || + _textIndent != nil || _breakBefore != nil || _breakAfter != nil) { + throw odr::UnsupportedOperation(); + } + odr::ParagraphStyle result; + if (_textAlign != nil) { + result.text_align = static_cast(_textAlign.integerValue); + } + return result; +} + + (instancetype)styleWithHandle:(const odr::ParagraphStyle &)handle { ODRParagraphStyle *const result = [[ODRParagraphStyle alloc] init]; result->_textAlign = box_enum(handle.text_align); diff --git a/apple/tests/OdrCoreTests.swift b/apple/tests/OdrCoreTests.swift index f32a66864..be47446a1 100644 --- a/apple/tests/OdrCoreTests.swift +++ b/apple/tests/OdrCoreTests.swift @@ -420,6 +420,39 @@ final class DocumentSaveTests: XCTestCase { } } + func testSetStyleAlignsAParagraph() throws { + let document = try self.document() + let root = try XCTUnwrap(try document.rootElement()) + let paragraph = try XCTUnwrap(root.firstDescendant(ofType: Paragraph.self)) + + let style = ParagraphStyle() + style.textAlign = NSNumber(value: TextAlign.center.rawValue) + try paragraph.setStyle(style) + + let saved = try XCTUnwrap(try document.saveToMemory()) + let path = URL(fileURLWithPath: try temporaryDirectory()) + .appendingPathComponent("aligned.odt") + try saved.write(to: path) + + let reloaded = try DecodedFile.decode(path: path.path) + .asDocumentFile().document() + let reloadedRoot = try XCTUnwrap(try reloaded.rootElement()) + let aligned = try XCTUnwrap(reloadedRoot.firstDescendant(ofType: Paragraph.self)).style + XCTAssertEqual(aligned.textAlign?.intValue, TextAlign.center.rawValue) + } + + func testSetParagraphStyleRefusesALineHeight() throws { + let document = try self.document() + let root = try XCTUnwrap(try document.rootElement()) + let paragraph = try XCTUnwrap(root.firstDescendant(ofType: Paragraph.self)) + + let style = ParagraphStyle() + style.lineHeight = Measure(string: "12pt") + XCTAssertThrowsError(try paragraph.setStyle(style)) { error in + XCTAssertEqual((error as NSError).code, ODRError.unsupportedOperation.rawValue) + } + } + func testSetCellStyleFillsACell() throws { let document = try DecodedFile.decode(path: try Fixture.ods()) .asDocumentFile().document() diff --git a/jni/java/app/opendocument/core/Paragraph.java b/jni/java/app/opendocument/core/Paragraph.java index dedeb6075..dd4c54c3f 100644 --- a/jni/java/app/opendocument/core/Paragraph.java +++ b/jni/java/app/opendocument/core/Paragraph.java @@ -10,11 +10,21 @@ public ParagraphStyle style() { return styleNative(handle()); } + /** + * States the non-null fields of {@code style} on the paragraph and leaves the rest. Only {@code + * textAlign} is written; any other field is refused. + */ + public void setStyle(ParagraphStyle style) { + setStyleNative(handle(), style); + } + public TextStyle textStyle() { return textStyleNative(handle()); } private native ParagraphStyle styleNative(long handle); + private native void setStyleNative(long handle, ParagraphStyle style); + private native TextStyle textStyleNative(long handle); } diff --git a/jni/java/app/opendocument/core/ParagraphStyle.java b/jni/java/app/opendocument/core/ParagraphStyle.java index 9afe55a34..1f420a98b 100644 --- a/jni/java/app/opendocument/core/ParagraphStyle.java +++ b/jni/java/app/opendocument/core/ParagraphStyle.java @@ -1,17 +1,24 @@ package app.opendocument.core; -/** Style of a paragraph. Mirrors {@code odr::ParagraphStyle}; fields may be {@code null}. */ +/** + * Style of a paragraph. Mirrors {@code odr::ParagraphStyle}; a {@code null} field is one the + * document does not state. A caller builds one for {@link Paragraph#setStyle}: every field left + * {@code null} is left alone on the paragraph. + */ public final class ParagraphStyle { - public final TextAlign textAlign; + public TextAlign textAlign; /** The base direction the paragraph's text runs in; {@code null} if the style says nothing. */ - public final TextDirection direction; - public final DirectionalMeasure margin; - public final Measure lineHeight; - public final Measure textIndent; + public TextDirection direction; + public DirectionalMeasure margin; + public Measure lineHeight; + public Measure textIndent; /** A break the author put before the paragraph; {@code null} if the style says nothing. */ - public final BreakType breakBefore; + public BreakType breakBefore; /** A break the author put after the paragraph; {@code null} if the style says nothing. */ - public final BreakType breakAfter; + public BreakType breakAfter; + + /** Every field {@code null}. */ + public ParagraphStyle() {} ParagraphStyle( int textAlign, diff --git a/jni/src/jni_convert.hpp b/jni/src/jni_convert.hpp index 8e6ef6bb7..875da5889 100644 --- a/jni/src/jni_convert.hpp +++ b/jni/src/jni_convert.hpp @@ -58,6 +58,9 @@ odr::TextStyle text_style_from_java(JNIEnv *env, jobject style); /// The fields a Java `TableCellStyle` states; a padding or a border is /// refused, since no engine writes one. odr::TableCellStyle table_cell_style_from_java(JNIEnv *env, jobject style); +/// The fields a Java `ParagraphStyle` states; any but `textAlign` is refused, +/// since no engine writes one. +odr::ParagraphStyle paragraph_style_from_java(JNIEnv *env, jobject style); /// Optional enum to a Java-side code; -1 encodes absent. template jint enum_code(const std::optional &value) { diff --git a/jni/src/jni_document.cpp b/jni/src/jni_document.cpp index d1083ceac..1670d02ff 100644 --- a/jni/src/jni_document.cpp +++ b/jni/src/jni_document.cpp @@ -611,6 +611,16 @@ Java_app_opendocument_core_Paragraph_styleNative(JNIEnv *env, jobject, }); } +extern "C" JNIEXPORT void JNICALL +Java_app_opendocument_core_Paragraph_setStyleNative(JNIEnv *env, jobject, + jlong handle, + jobject style) { + guarded(env, [&] { + element(handle).as_paragraph().set_style( + odr_jni::paragraph_style_from_java(env, style)); + }); +} + extern "C" JNIEXPORT jobject JNICALL Java_app_opendocument_core_Paragraph_textStyleNative(JNIEnv *env, jobject, jlong handle) { diff --git a/jni/src/jni_style.cpp b/jni/src/jni_style.cpp index 5765b9182..c8014dc01 100644 --- a/jni/src/jni_style.cpp +++ b/jni/src/jni_style.cpp @@ -402,6 +402,41 @@ odr::TableCellStyle table_cell_style_from_java(JNIEnv *env, return result; } +odr::ParagraphStyle paragraph_style_from_java(JNIEnv *env, + const jobject style) { + odr::ParagraphStyle result; + if (style == nullptr) { + return result; + } + jclass cls = env->GetObjectClass(style); + const auto field = [&](const char *name, const char *signature) { + return env->GetObjectField(style, env->GetFieldID(cls, name, signature)); + }; + + for (const auto &[name, signature] : + {std::pair{"direction", "Lapp/opendocument/core/TextDirection;"}, + std::pair{"margin", "Lapp/opendocument/core/DirectionalMeasure;"}, + std::pair{"lineHeight", "Lapp/opendocument/core/Measure;"}, + std::pair{"textIndent", "Lapp/opendocument/core/Measure;"}, + std::pair{"breakBefore", "Lapp/opendocument/core/BreakType;"}, + std::pair{"breakAfter", "Lapp/opendocument/core/BreakType;"}}) { + if (const jobject value = field(name, signature); value != nullptr) { + env->DeleteLocalRef(value); + env->DeleteLocalRef(cls); + throw odr::UnsupportedOperation(); + } + } + const jobject text_align = + field("textAlign", "Lapp/opendocument/core/TextAlign;"); + result.text_align = enum_from_java(env, text_align); + if (text_align != nullptr) { + env->DeleteLocalRef(text_align); + } + + env->DeleteLocalRef(cls); + return result; +} + odr::DirectionalStyle directional_measure_from_java(JNIEnv *env, jobject value) { odr::DirectionalStyle result; diff --git a/jni/tests/app/opendocument/core/DocumentTest.java b/jni/tests/app/opendocument/core/DocumentTest.java index 615c1fca7..1247a56e9 100644 --- a/jni/tests/app/opendocument/core/DocumentTest.java +++ b/jni/tests/app/opendocument/core/DocumentTest.java @@ -192,6 +192,45 @@ void setStyleMarksARun() throws IOException { assertNull(styled.fontStyle); } + private static Paragraph firstParagraph(Element element) { + if (element.type() == ElementType.PARAGRAPH) { + return element.asParagraph(); + } + for (Element child : element.children()) { + Paragraph found = firstParagraph(child); + if (found != null) { + return found; + } + } + return null; + } + + @Test + void setStyleAlignsAParagraph() throws IOException { + Document document = openDocument(); + + ParagraphStyle style = new ParagraphStyle(); + style.textAlign = TextAlign.CENTER; + firstParagraph(document.rootElement()).setStyle(style); + + Path path = tempDir.resolve("aligned.odt"); + Files.write(path, document.saveToMemory()); + Document reloaded = Odr.open(path.toString()).asDocumentFile().document(); + + assertEquals(TextAlign.CENTER, firstParagraph(reloaded.rootElement()).style().textAlign); + } + + @Test + void setParagraphStyleRefusesWhatNoEngineWrites() throws IOException { + Document document = openDocument(); + Paragraph paragraph = firstParagraph(document.rootElement()); + + ParagraphStyle style = new ParagraphStyle(); + style.textAlign = TextAlign.CENTER; + style.lineHeight = new Measure(12, "pt"); + assertThrows(OdrException.UnsupportedOperation.class, () -> paragraph.setStyle(style)); + } + @Test void setCellStyleFillsACell() throws IOException { Path ods = TestFiles.odsFile(tempDir); diff --git a/python/src/bind_document.cpp b/python/src/bind_document.cpp index 6c2a40cf0..7851e48fa 100644 --- a/python/src/bind_document.cpp +++ b/python/src/bind_document.cpp @@ -213,6 +213,7 @@ void odr_python::bind_document(py::module_ &m) { bind_element(m, "Paragraph") .def("style", &odr::Paragraph::style) + .def("set_style", &odr::Paragraph::set_style, py::arg("style")) .def("text_style", &odr::Paragraph::text_style); bind_element(m, "Span").def("style", &odr::Span::style); diff --git a/python/tests/test_document.py b/python/tests/test_document.py index 5cc043a19..3eecdbe06 100644 --- a/python/tests/test_document.py +++ b/python/tests/test_document.py @@ -261,3 +261,38 @@ def test_set_cell_style_refuses_what_no_engine_writes(ods_path): cell_style.wrap_text = True with pytest.raises(pyodr.UnsupportedOperation): sheet.set_cell_style(0, 0, cell_style, pyodr.TextStyle()) + + +def first_paragraph(element): + if element.type() == pyodr.ElementType.paragraph: + return element.as_paragraph() + for child in element.children(): + found = first_paragraph(child) + if found is not None: + return found + return None + + +def test_set_style_aligns_a_paragraph(odt_path, tmp_path): + document = pyodr.open(str(odt_path)).as_document_file().document() + + style = pyodr.ParagraphStyle() + style.text_align = pyodr.TextAlign.center + first_paragraph(document.root_element()).set_style(style) + + path = tmp_path / "aligned.odt" + path.write_bytes(document.save_to_memory()) + reloaded = pyodr.open(str(path)).as_document_file().document() + + assert first_paragraph(reloaded.root_element()).style().text_align == ( + pyodr.TextAlign.center + ) + + +def test_set_paragraph_style_refuses_what_no_engine_writes(odt_path): + document = pyodr.open(str(odt_path)).as_document_file().document() + + style = pyodr.ParagraphStyle() + style.line_height = pyodr.Measure("12pt") + with pytest.raises(pyodr.UnsupportedOperation): + first_paragraph(document.root_element()).set_style(style) diff --git a/wasm/js/index.d.ts b/wasm/js/index.d.ts index f39b448bb..f69b330f7 100644 --- a/wasm/js/index.d.ts +++ b/wasm/js/index.d.ts @@ -31,6 +31,11 @@ export interface TextStyle { size?: string; } +/** What `Document.setParagraphStyle` states on a paragraph. */ +export interface ParagraphStyle { + align?: 'left' | 'center' | 'right' | 'justify'; +} + /** What `Document.setCellStyle` states on a cell: the keys of `TextStyle` * but `highlight`, and the cell's own ground and alignment. */ export interface CellStyle extends Omit { @@ -214,6 +219,13 @@ export declare class Document { * @throws OdrError `invalid_argument` for a property it does not know */ setCellStyle(sheet: number, column: number, row: number, style: CellStyle): this; + /** + * States `style` on one paragraph and leaves what it does not name. The + * same object the page's `odr.editing.format` takes. + * @throws OdrError `invalid_argument` for a property or a value it does + * not know + */ + setParagraphStyle(id: number, style: ParagraphStyle): this; /** `afterId` of 0 splits before every child. */ splitParagraph(paragraphId: number, afterId?: number): number; mergeParagraphWithNext(paragraphId: number): this; diff --git a/wasm/js/index.js b/wasm/js/index.js index 39c7b56fa..22fe1435f 100644 --- a/wasm/js/index.js +++ b/wasm/js/index.js @@ -120,6 +120,13 @@ export class Document { return this; } + // States `style` - `{align}`, as the page's `odr.editing.format` takes it + // - on one paragraph. + setParagraphStyle(id, style) { + unwrap(this.#core.setParagraphStyle(this.#handle, id, style)); + return this; + } + // States `style` - the text keys of `setTextStyle`, and `fill` and // `align` - on the cell at a position, as `odr.editing.format` takes it. setCellStyle(sheet, column, row, style) { diff --git a/wasm/src/wasm_document.cpp b/wasm/src/wasm_document.cpp index 1d17db51f..60e4c61ce 100644 --- a/wasm/src/wasm_document.cpp +++ b/wasm/src/wasm_document.cpp @@ -108,6 +108,26 @@ emscripten::val set_text_style(const Handle handle, const double id, }); } +/// @p style as the page spells it - `{align: "center"}` - replayed through +/// the envelope, which parses it. +emscripten::val set_paragraph_style(const Handle handle, const double id, + const emscripten::val style) { + return guarded([&] { + Session &s = session(handle); + if (style.isUndefined() || style.isNull() || + style.typeOf().as() != "object") { + throw std::invalid_argument("setParagraphStyle takes a style object"); + } + const std::string json = + emscripten::val::global("JSON").call("stringify", style); + document_of(s).edit( + R"({"version":2,"ops":[{"op":"setParagraphStyle","id":)" + + std::to_string(element_of(s, id).identifier()) + R"(,"style":)" + json + + "}]}"); + return ok(); + }); +} + /// @p style as the page's `odr.editing.format` takes it for a cell, replayed /// through the envelope, which parses it. emscripten::val set_cell_style(const Handle handle, const double sheet, @@ -201,6 +221,7 @@ EMSCRIPTEN_BINDINGS(odr_document) { emscripten::function("appendText", &odr::wasm::append_text); emscripten::function("setTextStyle", &odr::wasm::set_text_style); emscripten::function("setCellStyle", &odr::wasm::set_cell_style); + emscripten::function("setParagraphStyle", &odr::wasm::set_paragraph_style); emscripten::function("splitParagraph", &odr::wasm::split_paragraph); emscripten::function("mergeParagraphWithNext", &odr::wasm::merge_paragraph_with_next); diff --git a/wasm/tests/edit.test.mjs b/wasm/tests/edit.test.mjs index 43cb9de22..a2c8afc3d 100644 --- a/wasm/tests/edit.test.mjs +++ b/wasm/tests/edit.test.mjs @@ -13,6 +13,12 @@ function firstEditableRunId(html) { return Number(match[1]); } +function firstEditableParagraphId(html) { + const match = html.match(/]*data-odr-id="(\d+)"/); + assert.ok(match, 'the editable render carries no paragraph id'); + return Number(match[1]); +} + describe('edit', () => { let odr; before(async () => { @@ -169,6 +175,35 @@ describe('edit', () => { } }); + it('aligns a paragraph by id and saves the alignment', () => { + const doc = odr.open(minimalOdt('hello'), { editable: true }); + try { + const id = firstEditableParagraphId(doc.render(0).html); + doc.setParagraphStyle(id, { align: 'center' }); + assert.match(doc.render(0).html, /text-align:center/); + + const reopened = odr.open(doc.save()); + try { + assert.match(reopened.render(0).html, /text-align:center/); + } finally { + reopened.close(); + } + } finally { + doc.close(); + } + }); + + it('refuses an alignment it does not know', () => { + const doc = odr.open(minimalOdt('hello'), { editable: true }); + try { + const id = firstEditableParagraphId(doc.render(0).html); + assert.throws(() => doc.setParagraphStyle(id, { align: 'middle' }), OdrError); + assert.throws(() => doc.setParagraphStyle(id, 'center'), OdrError); + } finally { + doc.close(); + } + }); + it('removes an element by id', () => { const doc = odr.open(minimalOdt('hello'), { editable: true }); try { From 634929b5f36db8d090bfdd03b00a5674b3814564 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 29 Sep 2026 07:53:21 +0200 Subject: [PATCH 4/5] test: advance the reference output to the paragraph alignment Twelve docx pages now show text-align:justify where the file states w:jc="both", and the embedded document.js and editing.js align paragraphs. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Tigcf9he3w1aHfNPJoEGsd --- test/data.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/data.cmake b/test/data.cmake index 936bd5f33..6865e669c 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 "fd0026d631e17d7a72ed8fcee90a92ef67189f54") + REVISION "6976b72662d5eed8c482632740ac89f718036d7c") odr_test_data( PATH "reference-output/odr-private" URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git" - REVISION "a91e3d2b693d7d3bd169aa2923eac76515019108") + REVISION "4e6b97d47a832bdc972cc552314b9090eabf1d55") From 79ce2dc3bf4f1e252fb4fe39ff51521fc92d76fa Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 29 Sep 2026 08:22:44 +0200 Subject: [PATCH 5/5] refactor(document): shorten the paragraph alignment Paragraph::set_style returns early when text_align is not set, so the engines no longer check it and no longer write an empty pPr. The tests share one tree walk for nth_run and nth_paragraph. The changelog entry and some doc comments are shorter. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_0168TSFHrPyguyvsxTXLXqMD --- CHANGELOG.md | 12 ++---- apple/src/ODRPrivate.h | 2 +- src/odr/document.hpp | 9 ++--- src/odr/document_element.cpp | 3 ++ src/odr/internal/abstract/document.hpp | 2 +- src/odr/internal/odf/odf_document.cpp | 2 - src/odr/internal/odf/odf_style.cpp | 12 ++---- src/odr/internal/odf/odf_style.hpp | 8 ++-- .../ooxml_presentation_document.cpp | 38 +++++++++---------- .../ooxml/text/ooxml_text_document.cpp | 12 +++--- test/src/document_edit_test.cpp | 26 +++++-------- 11 files changed, 50 insertions(+), 76 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fd77a436..4002d7ef1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,15 +16,9 @@ The release run heads these entries with the version and opens a fresh ## Unreleased -- A paragraph of an odt, odp, odg, docx or pptx file takes a horizontal - alignment: `Paragraph::set_style` and the `setParagraphStyle` op write - `text_align`. The document editor aligns every paragraph that the selection - reaches through `odr.editing.format({align})`, with `left`, `center`, - `right` or `justify`. `onSelectionChange` reports `align`. The python, - Java, Objective-C and npm bindings expose it, and in Java and Objective-C a - `ParagraphStyle` can now be built and written. - -- **Fix**: a docx paragraph with `w:jc="both"` was not justified. It now is. +- A paragraph of an odt, odp, odg, docx or pptx file takes an alignment + (`Paragraph::set_style`, `setParagraphStyle`, `odr.editing.format({align})`, + and the bindings). The docx reader now reads `w:jc="both"` as justified. - The python, Java, Objective-C and npm bindings style a sheet cell (`Sheet::set_cell_style`). In Java and Objective-C, a `TableCellStyle` can diff --git a/apple/src/ODRPrivate.h b/apple/src/ODRPrivate.h index 5edd688f8..eccb1c43e 100644 --- a/apple/src/ODRPrivate.h +++ b/apple/src/ODRPrivate.h @@ -110,7 +110,7 @@ NS_ASSUME_NONNULL_BEGIN @interface ODRParagraphStyle (Private) + (instancetype)styleWithHandle:(const odr::ParagraphStyle &)handle; /// The set properties as a `ParagraphStyle`; throws `UnsupportedOperation` -/// for any but `textAlign`, which no engine writes. +/// for any property but `textAlign`, since no engine writes one. - (odr::ParagraphStyle)handle; @end diff --git a/src/odr/document.hpp b/src/odr/document.hpp index e8896a34f..daaf1b265 100644 --- a/src/odr/document.hpp +++ b/src/odr/document.hpp @@ -56,11 +56,10 @@ class Document final { /// `{"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` and - /// `setTextStyle` name a text element, and `setParagraphStyle` a paragraph, - /// 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, @ref Text::set_style or - /// @ref Paragraph::set_style and needs none of this. + /// `setTextStyle` and `setParagraphStyle` name an 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, + /// @ref Text::set_style or @ref Paragraph::set_style 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. diff --git a/src/odr/document_element.cpp b/src/odr/document_element.cpp index 3bb9f1ca3..6416fc5b5 100644 --- a/src/odr/document_element.cpp +++ b/src/odr/document_element.cpp @@ -535,6 +535,9 @@ void Paragraph::set_style(const ParagraphStyle &style) const { style.break_after.has_value()) { throw UnsupportedOperation(); } + if (!style.text_align.has_value()) { + return; + } m_adapter2->paragraph_set_style(m_identifier, style); } diff --git a/src/odr/internal/abstract/document.hpp b/src/odr/internal/abstract/document.hpp index 8150ba10c..df60d68aa 100644 --- a/src/odr/internal/abstract/document.hpp +++ b/src/odr/internal/abstract/document.hpp @@ -355,7 +355,7 @@ class ParagraphAdapter { paragraph_style(ElementIdentifier element_id) const = 0; [[nodiscard]] virtual TextStyle paragraph_text_style(ElementIdentifier element_id) const = 0; - /// States the set fields of @p style on the paragraph and leaves the rest. + /// Writes `text_align` of @p style, which is set, on the paragraph. virtual void paragraph_set_style([[maybe_unused]] const ElementIdentifier element_id, [[maybe_unused]] const ParagraphStyle &style) const { diff --git a/src/odr/internal/odf/odf_document.cpp b/src/odr/internal/odf/odf_document.cpp index a7b322964..531c96624 100644 --- a/src/odr/internal/odf/odf_document.cpp +++ b/src/odr/internal/odf/odf_document.cpp @@ -786,8 +786,6 @@ class ElementAdapter final : public AdapterBase { paragraph_text_style(const ElementIdentifier element_id) const override { return get_intermediate_style(element_id).text_style; } - /// Points the paragraph at a fresh automatic style: a copy of the one it - /// shows plus the delta, or a child of a named one. void paragraph_set_style(const ElementIdentifier element_id, const ParagraphStyle &style) const override { pugi::xml_node node = get_node(element_id); diff --git a/src/odr/internal/odf/odf_style.cpp b/src/odr/internal/odf/odf_style.cpp index a38021e0e..fdaeb0ee9 100644 --- a/src/odr/internal/odf/odf_style.cpp +++ b/src/odr/internal/odf/odf_style.cpp @@ -895,10 +895,8 @@ StyleRegistry::create_paragraph_style(pugi::xml_node automatic_styles, const char *base_name, const ParagraphStyle &style) { const std::string base = base_name != nullptr ? base_name : ""; - const std::string key = fmt::format( - "{}|{}", base, - style.text_align ? std::to_string(static_cast(*style.text_align)) - : "-"); + const std::string key = + fmt::format("{}|{}", base, static_cast(*style.text_align)); if (const auto it = m_created_paragraph_styles.find(key); it != std::end(m_created_paragraph_styles)) { return it->second; @@ -908,10 +906,8 @@ StyleRegistry::create_paragraph_style(pugi::xml_node automatic_styles, "P", m_next_paragraph_style); const std::string name = node.attribute("style:name").value(); - if (style.text_align.has_value()) { - xml::set_attribute(properties_of(node, "style:paragraph-properties"), - "fo:text-align", text_align_value(*style.text_align)); - } + xml::set_attribute(properties_of(node, "style:paragraph-properties"), + "fo:text-align", text_align_value(*style.text_align)); m_index_style[name] = node; generate_style_(name, node); diff --git a/src/odr/internal/odf/odf_style.hpp b/src/odr/internal/odf/odf_style.hpp index 02ea9e2e0..decbe4302 100644 --- a/src/odr/internal/odf/odf_style.hpp +++ b/src/odr/internal/odf/odf_style.hpp @@ -94,8 +94,7 @@ class StyleRegistry final { const char *base_name, const TableCellStyle &cell, const TextStyle &text); - /// A paragraph style carrying the `text_align` of @p style, made as a cell - /// style is. + /// A paragraph style carrying the `text_align` of @p style. std::string create_paragraph_style(pugi::xml_node automatic_styles, const char *base_name, const ParagraphStyle &style); @@ -133,9 +132,8 @@ class StyleRegistry final { void generate_master_pages_(Document &); - /// A style of @p family under @p automatic_styles named @p prefix and the - /// first free number from @p next: a copy of the automatic style - /// @p base_name names, a child of a named one. + /// A fresh `` style: a copy of the automatic style @p base_name, + /// or a child of the named one. pugi::xml_node create_style_(pugi::xml_node automatic_styles, const std::string &base_name, const char *family, const char *prefix, std::uint32_t &next); diff --git a/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp b/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp index 2bb11684c..b4c379f59 100644 --- a/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp +++ b/src/odr/internal/ooxml/presentation/ooxml_presentation_document.cpp @@ -302,24 +302,22 @@ class ElementAdapter final : public AdapterBase { void paragraph_set_style(const ElementIdentifier element_id, const ParagraphStyle &style) const override { const char *algn = nullptr; - if (style.text_align.has_value()) { - switch (*style.text_align) { - case TextAlign::left: - algn = "l"; - break; - case TextAlign::right: - algn = "r"; - break; - case TextAlign::center: - algn = "ctr"; - break; - case TextAlign::justify: - algn = "just"; - break; - case TextAlign::start: - case TextAlign::end: - throw UnsupportedOperation(); - } + switch (*style.text_align) { + case TextAlign::left: + algn = "l"; + break; + case TextAlign::right: + algn = "r"; + break; + case TextAlign::center: + algn = "ctr"; + break; + case TextAlign::justify: + algn = "just"; + break; + case TextAlign::start: + case TextAlign::end: + throw UnsupportedOperation(); } pugi::xml_node node = get_node(element_id); pugi::xml_node properties = node.child("a:pPr"); @@ -327,9 +325,7 @@ class ElementAdapter final : public AdapterBase { // the schema wants it ahead of the runs properties = node.prepend_child("a:pPr"); } - if (algn != nullptr) { - xml::set_attribute(properties, "algn", algn); - } + xml::set_attribute(properties, "algn", algn); } [[nodiscard]] TextStyle diff --git a/src/odr/internal/ooxml/text/ooxml_text_document.cpp b/src/odr/internal/ooxml/text/ooxml_text_document.cpp index 6e9da4020..65b523813 100644 --- a/src/odr/internal/ooxml/text/ooxml_text_document.cpp +++ b/src/odr/internal/ooxml/text/ooxml_text_document.cpp @@ -397,14 +397,12 @@ class ElementAdapter final : public AdapterBase { if (!properties) { properties = node.prepend_child("w:pPr"); } - if (style.text_align.has_value()) { - pugi::xml_node jc = properties.child("w:jc"); - if (!jc) { - jc = xml::insert_in_sequence(properties, "w:jc", - paragraph_property_order); - } - xml::set_attribute(jc, "w:val", jc_value(*style.text_align)); + pugi::xml_node jc = properties.child("w:jc"); + if (!jc) { + jc = + xml::insert_in_sequence(properties, "w:jc", paragraph_property_order); } + xml::set_attribute(jc, "w:val", jc_value(*style.text_align)); } [[nodiscard]] TextStyle diff --git a/test/src/document_edit_test.cpp b/test/src/document_edit_test.cpp index 8a131fa42..5325812e9 100644 --- a/test/src/document_edit_test.cpp +++ b/test/src/document_edit_test.cpp @@ -896,11 +896,12 @@ Document pptx_of(const std::string ¶graphs) { paragraphs + R"()"}}); } -/// The @p ordinal -th run anywhere in @p document. -Element nth_run(const Document &document, const std::uint32_t ordinal) { +/// The @p ordinal -th element of @p type anywhere in @p document. +Element nth_of_type(const Document &document, const ElementType type, + const std::uint32_t ordinal) { std::uint32_t seen = 0; const auto walk = [&](this auto &&self, const Element element) -> Element { - if (element.type() == ElementType::text && seen++ == ordinal) { + if (element.type() == type && seen++ == ordinal) { return element; } for (const Element child : element.children()) { @@ -913,6 +914,10 @@ Element nth_run(const Document &document, const std::uint32_t ordinal) { return walk(document.root_element()); } +Element nth_run(const Document &document, const std::uint32_t ordinal) { + return nth_of_type(document, ElementType::text, ordinal); +} + /// The part @p path of @p document saved, empty elements spelt `` /// whichever way the writer spells them. std::string part_of(const Document &document, const std::string &path) { @@ -1223,21 +1228,8 @@ std::optional align_of(const Element paragraph) { return paragraph.as_paragraph().style().text_align; } -/// The @p ordinal -th paragraph anywhere in @p document. Element nth_paragraph(const Document &document, const std::uint32_t ordinal) { - std::uint32_t seen = 0; - const auto walk = [&](this auto &&self, const Element element) -> Element { - if (element.type() == ElementType::paragraph && seen++ == ordinal) { - return element; - } - for (const Element child : element.children()) { - if (const Element found = self(child)) { - return found; - } - } - return {}; - }; - return walk(document.root_element()); + return nth_of_type(document, ElementType::paragraph, ordinal); } } // namespace