feat(html): a sheet writes the same markup editable or not

CHANGELOG.md

@@ -16,6 +16,10 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- A sheet rendered with `HtmlConfig::editable` carries no `contenteditable`
+  and no `data-odr-path`: its editing is an overlay, so the markup states
+  none. A cell's runs fold into the `td` as they do read-only.
+
 - **Breaking** (wire only) `Document::edit` takes an op envelope,
   `{"version": 1, "ops": [...]}`, with `setCell` writing a sheet cell by
   position and `setText` carrying what the `modifiedText` map carried.

docs/design/spreadsheet-editing.md

@@ -52,15 +52,6 @@ results go stale the moment an input changes.
 | Addressing | `DocumentPath` | Already spells a cell by position: `/child:0/cell:A1/...` |
 | Capabilities | `file_type_table.cpp` | `ods` and `xlsx` declare `edit` and `save` (step 0.2, landed); `csv` declares neither. `odr_test` checks the declaration against `Document::is_editable` |
 
-One inconsistency worth fixing on day one: `translate_sheet` stamps
-`contenteditable` on every run inside an `.ods` cell when `config.editable` is
-set — the writer asks the *element* (`element_is_editable`, true for a
-non-repeated cell) and never the document. The reference output for
-`style-color+fixed-1.ods` carries 594 of them. Two consequences: an app that
-turns `editable` on gets a half-working sheet editor (strings save, numbers
-desync), and the editable output lays out differently, because
-`plain_text` refuses to fold an editable run into its `td`.
-
 ## Decisions
 
 ### 1. A cell is the unit of editing, addressed by position
@@ -308,11 +299,10 @@ Each step ships on its own. "Both" means `.ods` and `.xlsx`.
    sheet is 0.36 s today.
 5. **Landed.** `Document::is_editable` true for both; capability rows gained
    `edit` (`xlsx` also `save`); `odr_test` keeps them honest.
-6. Stop `translate_sheet` stamping `contenteditable` on a cell's runs at all.
-   It cannot be gated on `Document::is_editable`, which item 5 makes *true* for
-   a sheet: decision 3 puts a sheet's editing in an overlay, so the markup
-   carries none. Changes the reference output — a reference `.ods` loses 594
-   attributes — so it lands with a regen, on its own.
+6. **Landed.** `translate_sheet` writes its cells through a `WritingState`
+   whose `editable_markup` is false, so no run carries `contenteditable` and
+   `plain_text` folds it into the `td` as it does read-only. It could not be
+   gated on `Document::is_editable`, which item 5 makes *true* for a sheet.
 7. **Landed.** Tests: set a number, a string, clear a cell, and each refusal,
    on both formats, from inline fixtures; save and reopen. The LibreOffice
    oracle (`soffice --convert-to`) stays a by-hand check — it is not in CI, and

src/odr/internal/html/common.hpp

@@ -44,13 +44,21 @@ struct WritingState {
   [[nodiscard]] TextDirection direction() const { return m_direction; }
   void set_direction(const TextDirection direction) { m_direction = direction; }
 
+  /// Whether an editable run says so in the markup - false under a sheet,
+  /// whose editing is an overlay.
+  [[nodiscard]] bool editable_markup() const { return m_editable_markup; }
+  void set_editable_markup(const bool editable) {
+    m_editable_markup = editable;
+  }
+
 private:
   HtmlWriter *m_out;
   const HtmlConfig *m_config;
   HtmlResources *m_resources;
   const Logger *m_logger;
   StyleRegistry *m_styles;
   TextDirection m_direction{TextDirection::left_to_right};
+  bool m_editable_markup{true};
 };
 
 /// Writes the viewport meta tag. Precedence: `config.viewport_content` (raw,

src/odr/internal/html/document_element.cpp

@@ -235,12 +235,19 @@ std::optional<double> sheet_print_fit(const Sheet &sheet,
 
 /// A run whose style the box around it can carry instead. Not a background, a
 /// raised run or an editable one: each means something else on the box.
+/// Whether @p element carries `contenteditable`: an editable run of a view
+/// that writes its editing into the markup.
+bool writes_editable(const Element &element, const html::WritingState &state) {
+  return state.editable_markup() && state.config().editable &&
+         element.is_editable();
+}
+
 std::optional<Text> plain_text(const Element &element,
                                const html::WritingState &state) {
   if (element.type() != ElementType::text) {
     return {};
   }
-  if (state.config().editable && element.is_editable()) {
+  if (writes_editable(element, state)) {
     return {};
   }
 
@@ -411,6 +418,10 @@ std::optional<HtmlSheetCut> html::sheet_cut(const Sheet &sheet,
 }
 
 void html::translate_sheet(const Sheet &sheet, const WritingState &state) {
+  // a sheet's editing is an overlay, so its content carries none
+  WritingState sheet_state = state;
+  sheet_state.set_editable_markup(false);
+
   const TableDimensions rendered = sheet_rendered_extent(sheet, state.config());
   const std::uint32_t end_column = rendered.columns;
   const std::uint32_t end_row = rendered.rows;
@@ -594,8 +605,8 @@ void html::translate_sheet(const Sheet &sheet, const WritingState &state) {
         }
       }
 
-      const std::optional<FoldedCell> folded =
-          fold_cell(cell, state, wraps, anchors_shapes, table_row_style.height);
+      const std::optional<FoldedCell> folded = fold_cell(
+          cell, sheet_state, wraps, anchors_shapes, table_row_style.height);
 
       state.out().write_element_begin(
           "td",
@@ -624,13 +635,13 @@ void html::translate_sheet(const Sheet &sheet, const WritingState &state) {
               }()));
       if (column_index == 0 && row_index == 0) {
         for (const Element shape : sheet.shapes()) {
-          translate_element(shape, state);
+          translate_element(shape, sheet_state);
         }
       }
       if (folded.has_value()) {
         state.out().out() << folded->text;
       } else {
-        translate_cell_children(cell, state);
+        translate_cell_children(cell, sheet_state);
       }
       state.out().write_element_end("td");
 
@@ -695,7 +706,7 @@ void html::translate_text(const Element &element, const WritingState &state) {
       HtmlElementOptions()
           .set_inline(true)
           .set_attributes([&](const HtmlAttributeWriterCallback &clb) {
-            if (state.config().editable && element.is_editable()) {
+            if (writes_editable(element, state)) {
               clb("contenteditable", "true");
               clb("data-odr-path", element.document_path().to_string());
             }

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 "19392dfc4e3f4c5507dae231fe96a92fe0ca29f4")
+        REVISION "e8dd6b52062fa8efa82a90f3e31b6c5ea7eeb58a")
 
 odr_test_data(
         PATH "reference-output/odr-private"
         URL "https://github.com/opendocument-app/OpenDocument.test-private.output.git"
-        REVISION "ee3077eaf35b36e084f14b130c4e9236fc9be0f1")
+        REVISION "ed1be07780b703b5400accbae8ccb124e68ebb71")

test/src/html_output_test.cpp

@@ -299,8 +299,7 @@ std::vector<std::pair<std::string, ConfigVariant>> list_variant_cases() {
       {"odr-public/docx/physics.docx", reflow},
 
       // The output a reader gets rather than an editor. A sheet is pinned
-      // too: a cell holding one plain string drops the run around it only
-      // where nothing has to carry `contenteditable`.
+      // too, where the two agree: its editing is an overlay.
       {"odr-public/odt/style-various-1.odt", read_only},
       {"odr-public/ods/file_example_ODS_100.ods", read_only},
   };

test/src/html_test.cpp

@@ -839,15 +839,29 @@ TEST(html, a_cell_holding_one_plain_string_writes_no_box_of_its_own) {
   EXPECT_EQ(page.find("<x-p"), std::string::npos);
 }
 
-// The run is what an editor addresses, so it stays wherever it is written.
-TEST(html, an_editable_cell_keeps_the_run_it_is_addressed_by) {
+// A sheet's editing is an overlay, so `editable` changes none of its markup,
+// and the run folds into its `td` as an editable one refused to.
+TEST(html, an_editable_sheet_writes_the_markup_a_read_only_one_does) {
   HtmlConfig config;
   config.editable = true;
 
-  const std::string page = render_sheet(
-      fods_file(fods_row(fods_cell("one") + fods_cell("two"))), config);
+  const DecodedFile file =
+      fods_file(fods_row(fods_cell("one") + fods_cell("two")));
+  const std::string page = render_sheet(file, config);
+
+  EXPECT_EQ(page.find(R"(contenteditable="true")"), std::string::npos);
+  EXPECT_EQ(page, render_sheet(file, HtmlConfig()));
+}
+
+// A text document still says so in the markup: it has no overlay.
+TEST(html, an_editable_text_document_marks_its_runs) {
+  HtmlConfig config;
+  config.editable = true;
+
+  const std::string page = render_odt(config);
 
-  EXPECT_NE(page.find(R"(<x-s contenteditable="true")"), std::string::npos);
+  EXPECT_NE(page.find(R"(contenteditable="true")"), std::string::npos);
+  EXPECT_NE(page.find("data-odr-path"), std::string::npos);
 }
 
 // #822: a sheet cell does not break its text into lines unless the file says