Gap 19: ODT bookmarks keep the text they cover

A bookmark over text (`text:bookmark-start`…`text:bookmark-end`, in one
paragraph or across several) was read as a point at its start, and a
Word bookmark over text was written as a point: a reference to it then
updated to nothing. The flow model gets the end of a bookmark
(`Inline::AnchorEnd`, in flows asked for fields, as ODT's is), the
document gets the bookmark's start and end where they are, and the
writer writes a start and an end (a point, `text:bookmark`, when it
ends where it starts). A bookmark started twice, ended twice or ended
before it starts is kept once, as a point when it has no end after it.

References to bookmarks, LibreOffice's references to headings included,
now update to the bookmarked text (engine test `odt_fields.rs`, through
a save and reopen).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBVQLyDnrG8Tn1yDQcUabC

crates/engine/tests/it/main.rs

@@ -1,6 +1,7 @@
 mod comment_ids;
 mod date_fields;
 mod macro_packages;
+mod odt_fields;
 #[cfg(not(target_arch = "wasm32"))]
 mod odt_sections;
 mod odt_toc;

crates/engine/tests/it/odt_fields.rs

@@ -0,0 +1,57 @@
+//! Fields in an ODT file update in WordCraft as they do in Word: a reference to a bookmark shows
+//! the bookmarked text (a bookmark over text, `text:bookmark-start`…`-end`, not a point). The
+//! ODT is original and synthetic, shaped like LibreOffice's.
+
+use std::io::Write;
+
+use serde_json::json;
+use wordcraft_engine::{Session, io};
+
+const NS: &str = r#"xmlns:office="urn:oasis:names:tc:opendocument:xmlns:office:1.0" xmlns:text="urn:oasis:names:tc:opendocument:xmlns:text:1.0" xmlns:style="urn:oasis:names:tc:opendocument:xmlns:style:1.0" xmlns:fo="urn:oasis:names:tc:opendocument:xmlns:xsl-fo-compatible:1.0""#;
+
+fn odt(body: &str) -> Vec<u8> {
+    let content = format!(r#"<office:document-content {NS}><office:body><office:text>{body}</office:text></office:body></office:document-content>"#);
+    let mut z = zip::ZipWriter::new(std::io::Cursor::new(Vec::new()));
+    let o = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored);
+    for (name, data) in [("mimetype", "application/vnd.oasis.opendocument.text"), ("content.xml", content.as_str())] {
+        z.start_file(name, o).unwrap();
+        z.write_all(data.as_bytes()).unwrap();
+    }
+    z.finish().unwrap().into_inner()
+}
+
+fn text(s: &Session) -> Vec<String> {
+    s.doc.body.iter().filter_map(|b| b.as_para()).map(|p| p.text.replace(wordcraft_doc::para::OBJ, "")).collect()
+}
+
+/// The displayed results of the fields in the body.
+fn results(s: &Session) -> Vec<String> {
+    s.doc
+        .body
+        .iter()
+        .filter_map(|b| b.as_para())
+        .flat_map(|p| p.objects.iter())
+        .filter_map(|o| match o {
+            wordcraft_doc::para::InlineObject::Field { result, .. } => Some(result.clone()),
+            _ => None,
+        })
+        .collect()
+}
+
+#[test]
+fn a_reference_to_a_bookmark_updates_to_its_text() {
+    let doc = io::open_bytes(
+        "refs.odt",
+        &odt(r#"<text:h text:outline-level="1"><text:bookmark-start text:name="__RefHeading___Toc1"/>Planning ahead<text:bookmark-end text:name="__RefHeading___Toc1"/></text:h><text:p>See <text:bookmark-ref text:reference-format="text" text:ref-name="__RefHeading___Toc1">Planning</text:bookmark-ref>.</text:p>"#),
+    )
+    .unwrap();
+    let mut s = Session::new(doc);
+    assert_eq!(results(&s), ["Planning"]);
+    s.run("references.updateFields", &json!({"all": true})).unwrap();
+    assert_eq!(results(&s), ["Planning ahead"], "{:?}", text(&s));
+    // And after a save in ODT and opening again.
+    let again = io::open_bytes("refs.odt", &io::save_bytes("refs.odt", &s.doc).unwrap()).unwrap();
+    let mut s2 = Session::new(again);
+    s2.run("references.updateFields", &json!({"all": true})).unwrap();
+    assert_eq!(results(&s2), ["Planning ahead"]);
+}

crates/formats/src/html/export.rs

@@ -143,7 +143,8 @@ pub fn inlines_html(inlines: &[Inline]) -> String {
             | Inline::Shape { .. }
             | Inline::Group { .. }
             | Inline::FieldStart(_)
-            | Inline::FieldEnd => {}
+            | Inline::FieldEnd
+            | Inline::AnchorEnd(_) => {}
             Inline::Field { result, fmt, .. } => {
                 let (open, close) = fmt_tags(fmt);
                 let at_start = o.is_empty();

crates/formats/src/latex/export.rs

@@ -247,7 +247,8 @@ impl Writer {
                 | Inline::Shape { .. }
                 | Inline::Group { .. }
                 | Inline::FieldStart(_)
-                | Inline::FieldEnd => {}
+                | Inline::FieldEnd
+                | Inline::AnchorEnd(_) => {}
                 Inline::Field { result, fmt, .. } => {
                     let s = self.span(result, fmt);
                     push_apart(&mut o, &s);

crates/formats/src/markdown/export.rs

@@ -141,7 +141,8 @@ fn inlines_md_impl(inlines: &[Inline], in_table: bool) -> String {
             | Inline::Shape { .. }
             | Inline::Group { .. }
             | Inline::FieldStart(_)
-            | Inline::FieldEnd => {}
+            | Inline::FieldEnd
+            | Inline::AnchorEnd(_) => {}
             // Read as its base text by `ruby_as_text` above.
             Inline::Ruby { .. } => {}
         }

crates/formats/src/model.rs

@@ -28,7 +28,7 @@ pub use to_doc::{char_props, part_blocks, to_doc, to_doc_in};
 pub use types::{
     Affix, BoxLook, CODE_STYLE, Cell, FBlock, FComment, FTable, Flow, FlowOpts, Fmt, FmtChange, Img, Inline, Kind, Label, ListInfo, MAX_DEPTH,
     MAX_FONT_NAME, MAX_GROUP_DEPTH, MAX_LINK, MONO_FONT, Member, Meta, Para, RUBY_BASE_SIZE, RevMark, Spacing, clean_text, equations_as_text,
-    flatten_tables, kept_font, kept_link, ruby_as_text, ruby_object,
+    flatten_tables, kept_font, kept_link, ranged_anchors, ruby_as_text, ruby_object,
 };
 pub(crate) use types::{Dropped, begin_import, dropped};
 

crates/formats/src/model/from_doc.rs

@@ -272,6 +272,8 @@ fn rich_inlines(doc: &Document, o: &InlineObject, f: &Fmt, opts: FlowOpts) -> Op
         InlineObject::Shape { .. } | InlineObject::Group { .. } if opts.boxes => vec![drawing_inline(doc, o, opts, 0, (1.0, 1.0))?],
         InlineObject::FieldStart { instr, .. } if opts.fields => vec![Inline::FieldStart(instr.clone())],
         InlineObject::FieldEnd if opts.fields => vec![Inline::FieldEnd],
+        // Where a bookmark ends, so it covers its text (Word's `_GoBack` is the last edit's spot).
+        InlineObject::BookmarkEnd { name } if opts.fields && name != "_GoBack" => vec![Inline::AnchorEnd(name.clone())],
         InlineObject::Field { instr, result, .. } if opts.fields => {
             vec![Inline::Field { instr: instr.clone(), result: result.clone(), fmt: f.clone() }]
         }

crates/formats/src/model/to_doc.rs

@@ -184,6 +184,11 @@ struct Builder<'a> {
     comments: std::collections::HashMap<u32, u32>,
     /// The document's list instance for each flow list number ([`ListInfo::list`]).
     lists: std::collections::HashMap<u32, u32>,
+    /// Bookmarks that cover text ([`super::types::ranged_anchors`]), those started, and those
+    /// ended: each started once and ended once, after its start.
+    ranged: std::collections::HashSet<String>,
+    started: std::collections::HashSet<String>,
+    ended: std::collections::HashSet<String>,
 }
 
 fn rev_key(kind: RevisionKind) -> u8 {
@@ -195,7 +200,8 @@ fn rev_key(kind: RevisionKind) -> u8 {
 }
 
 impl<'a> Builder<'a> {
-    fn new(doc: &'a mut Document) -> Builder<'a> {
+    /// A builder for `blocks` into `doc`.
+    fn new(doc: &'a mut Document, blocks: &[FBlock]) -> Builder<'a> {
         let mut revs = std::collections::HashMap::new();
         for (i, r) in doc.revisions.iter().enumerate() {
             if r.move_name.is_none() {
@@ -203,7 +209,16 @@ impl<'a> Builder<'a> {
                 revs.entry((rev_key(r.kind), mark)).or_insert(i as u32);
             }
         }
-        Builder { doc, revs, run_strings: 0, comments: Default::default(), lists: Default::default() }
+        Builder {
+            doc,
+            revs,
+            run_strings: 0,
+            comments: Default::default(),
+            lists: Default::default(),
+            ranged: super::types::ranged_anchors(blocks),
+            started: Default::default(),
+            ended: Default::default(),
+        }
     }
 
     /// The document revision for a tracked change, added the first time it is seen.
@@ -436,9 +451,18 @@ impl<'a> Builder<'a> {
                         append(&mut out, "", &CharProps::default(), Some(obj));
                     }
                 }
-                Inline::Anchor(name) => {
+                // A bookmark once (the first of a name): a point, or the start of the text it covers.
+                Inline::Anchor(name) if self.started.insert(name.clone()) => {
                     append(&mut out, "", &CharProps::default(), Some(InlineObject::BookmarkStart { name: name.clone() }));
-                    append(&mut out, "", &CharProps::default(), Some(InlineObject::BookmarkEnd { name: name.clone() }));
+                    if !self.ranged.contains(name) {
+                        append(&mut out, "", &CharProps::default(), Some(InlineObject::BookmarkEnd { name: name.clone() }));
+                    }
+                }
+                Inline::Anchor(_) => {}
+                Inline::AnchorEnd(name) => {
+                    if self.ranged.contains(name) && self.started.contains(name) && self.ended.insert(name.clone()) {
+                        append(&mut out, "", &CharProps::default(), Some(InlineObject::BookmarkEnd { name: name.clone() }));
+                    }
                 }
                 Inline::Figure(_) => {}
                 Inline::Comment { id, comment } => {
@@ -715,7 +739,7 @@ pub fn to_doc_in(flow: &Flow, section: wordcraft_doc::SectionProps) -> Document
     if uses_code {
         doc.styles.upsert(code_style());
     }
-    let body = Builder::new(&mut doc).blocks(&flow.blocks, 0);
+    let body = Builder::new(&mut doc, &flow.blocks).blocks(&flow.blocks, 0);
     doc.body = body;
     let m = &flow.meta;
     doc.core.title = m.title.clone();
@@ -731,7 +755,7 @@ pub fn to_doc_in(flow: &Flow, section: wordcraft_doc::SectionProps) -> Document
 /// `blocks` as document blocks of `doc` (their pictures go into its media), for a header,
 /// footer or other part an importer adds itself.
 pub fn part_blocks(doc: &mut Document, blocks: &[FBlock]) -> Blocks {
-    let mut out = Builder::new(doc).blocks(blocks, 0);
+    let mut out = Builder::new(doc, blocks).blocks(blocks, 0);
     if out.is_empty() {
         out.push(para_block(Paragraph::new()));
     }

crates/formats/src/model/types.rs

@@ -158,6 +158,10 @@ pub enum Inline {
     Image(Img),
     /// A bookmark (link target).
     Anchor(String),
+    /// Where bookmark `name`, started by an [`Inline::Anchor`] before it, ends: the bookmark
+    /// covers the text between. Only flows asked for fields ([`FlowOpts::fields`]) hold them;
+    /// an anchor without one is a point.
+    AnchorEnd(String),
     /// A chart or diagram known here only by its alt text: written where alt text has a place
     /// (an HTML or Markdown image), left out of editable formats so it never turns into body text.
     Figure(String),
@@ -273,6 +277,44 @@ impl Default for BoxLook {
     }
 }
 
+/// The bookmarks in `blocks` whose end ([`Inline::AnchorEnd`]) follows their start
+/// ([`Inline::Anchor`]): ranges, not points. Tables' cells, notes, comments and shapes' text
+/// are looked through, in document order.
+pub fn ranged_anchors(blocks: &[FBlock]) -> std::collections::HashSet<String> {
+    fn walk(blocks: &[FBlock], depth: usize, started: &mut std::collections::HashSet<String>, ranged: &mut std::collections::HashSet<String>) {
+        if depth > MAX_DEPTH {
+            return;
+        }
+        for b in blocks {
+            match b {
+                FBlock::Para(p) => {
+                    for i in &p.inlines {
+                        match i {
+                            Inline::Anchor(n) => {
+                                started.insert(n.clone());
+                            }
+                            Inline::AnchorEnd(n) if started.contains(n) => {
+                                ranged.insert(n.clone());
+                            }
+                            Inline::Note { blocks, .. } | Inline::Shape { blocks, .. } => walk(blocks, depth + 1, started, ranged),
+                            Inline::Comment { comment, .. } => walk(&comment.blocks, depth + 1, started, ranged),
+                            _ => {}
+                        }
+                    }
+                }
+                FBlock::Table(t) => {
+                    for c in t.rows.iter().flatten() {
+                        walk(&c.blocks, depth + 1, started, ranged);
+                    }
+                }
+            }
+        }
+    }
+    let (mut started, mut ranged) = Default::default();
+    walk(blocks, 0, &mut started, &mut ranged);
+    ranged
+}
+
 /// `blocks` with each table replaced by the blocks of its cells, row by row (nested tables too),
 /// for places that hold paragraphs only.
 pub fn flatten_tables(blocks: &[FBlock]) -> Vec<FBlock> {
@@ -579,7 +621,8 @@ impl Para {
                 | Inline::Shape { .. }
                 | Inline::Group { .. }
                 | Inline::FieldStart(_)
-                | Inline::FieldEnd => {}
+                | Inline::FieldEnd
+                | Inline::AnchorEnd(_) => {}
             }
         }
         s
@@ -604,7 +647,8 @@ impl Para {
                 | Inline::Shape { .. }
                 | Inline::Group { .. }
                 | Inline::FieldStart(_)
-                | Inline::FieldEnd => {}
+                | Inline::FieldEnd
+                | Inline::AnchorEnd(_) => {}
             }
         }
         s

crates/formats/src/odt/body.rs

@@ -752,14 +752,11 @@ impl Body<'_> {
             }
             "tab" => self.push_text("\t"),
             "line-break" => self.push_text("\n"),
-            "bookmark" | "bookmark-start" => {
-                if let Some(name) = get(a, "text:name") {
-                    if self.para.is_none() {
-                        self.start_para(None, None);
-                    }
-                    if let Some(p) = &mut self.para {
-                        p.inlines.push(Inline::Anchor(name.to_string()));
-                    }
+            // A bookmark (§6.2.1): a point, or the start or end of the text it covers.
+            "bookmark" | "bookmark-start" | "bookmark-end" => {
+                if let Some(name) = get(a, "text:name").filter(|n| !n.is_empty()) {
+                    let name = name.to_string();
+                    self.push_inline(if local(q) == "bookmark-end" { Inline::AnchorEnd(name) } else { Inline::Anchor(name) });
                 }
             }
             _ => return false,

crates/formats/src/odt/package.rs

@@ -500,6 +500,7 @@ pub fn export(doc: &Document) -> Result<Vec<u8>, String> {
         model::FlowOpts { tracked: true, fields: true, notes: true, comments: true, lists: true, chars: true, paras: true, boxes: true },
     );
     let mut w = Writer::default();
+    w.ranged = model::ranged_anchors(&flow.blocks);
     let mut body = String::new();
     let sections = flow_sections(&flow.blocks, &doc.last_section);
     // One master page per section that starts a page; the first one is "Standard".

crates/formats/src/odt/tests_more.rs

@@ -1,5 +1,6 @@
 //! Tests for the ODT features added after the first reader and writer: one module per feature.
 
+mod bookmarks;
 mod char_styles;
 mod chars;
 mod comments;

crates/formats/src/odt/tests_more/bookmarks.rs

@@ -0,0 +1,82 @@
+//! Bookmarks (ODF 1.3 §6.2.1 `text:bookmark`, `text:bookmark-start`/`-end`): a point or the text
+//! between its start and end, in one paragraph or across several, kept as a WordCraft bookmark
+//! over that text (what a reference to it shows) and written back the same way.
+
+use crate::odt::tests::{NS_T, content_of, package};
+use crate::odt::{export, import};
+use wordcraft_doc::Document;
+use wordcraft_doc::para::{InlineObject, OBJ};
+
+/// Each bookmark: (name, the text it covers, paragraphs joined by '|').
+fn bookmarks(d: &Document) -> Vec<(String, String)> {
+    let mut open: Vec<(String, String)> = Vec::new();
+    let mut done = Vec::new();
+    for (i, b) in d.body.iter().enumerate() {
+        let Some(p) = b.as_para() else { continue };
+        if i > 0 {
+            for o in &mut open {
+                o.1.push('|');
+            }
+        }
+        let mut k = 0;
+        for ch in p.text.chars() {
+            if ch == OBJ {
+                match p.objects.get(k) {
+                    Some(InlineObject::BookmarkStart { name }) => open.push((name.clone(), String::new())),
+                    Some(InlineObject::BookmarkEnd { name }) => {
+                        if let Some(at) = open.iter().position(|(n, _)| n == name) {
+                            done.push(open.remove(at));
+                        }
+                    }
+                    _ => {}
+                }
+                k += 1;
+            } else {
+                for o in &mut open {
+                    o.1.push(ch);
+                }
+            }
+        }
+    }
+    done.sort();
+    done
+}
+
+fn open() -> Document {
+    let content = format!(
+        r#"<office:document-content {NS_T}><office:body><office:text><text:p>Before <text:bookmark-start text:name="words"/>important words<text:bookmark-end text:name="words"/> after<text:bookmark text:name="spot"/>.</text:p><text:p>From <text:bookmark-start text:name="span"/>here</text:p><text:p>to there<text:bookmark-end text:name="span"/>.</text:p></office:text></office:body></office:document-content>"#
+    );
+    import(&package(&content, None)).unwrap()
+}
+
+#[test]
+fn a_bookmark_covers_its_text() {
+    assert_eq!(
+        bookmarks(&open()),
+        [("span".to_string(), "here|to there".to_string()), ("spot".into(), String::new()), ("words".into(), "important words".into())]
+    );
+}
+
+#[test]
+fn bookmarks_round_trip() {
+    let d = open();
+    let bytes = export(&d).unwrap();
+    let c = content_of(&bytes);
+    assert!(c.contains(r#"<text:bookmark-start text:name="words"/>"#) && c.contains(r#"<text:bookmark text:name="spot"/>"#), "{c}");
+    assert_eq!(bookmarks(&import(&bytes).unwrap()), bookmarks(&d));
+}
+
+#[test]
+fn hostile_bookmarks_do_not_panic() {
+    for body in [
+        r#"<text:p><text:bookmark-end text:name="never"/>x</text:p>"#,
+        r#"<text:p><text:bookmark-start text:name="open"/>no end</text:p>"#,
+        r#"<text:p><text:bookmark-start text:name="a"/><text:bookmark-start text:name="a"/>twice<text:bookmark-end text:name="a"/></text:p>"#,
+        r#"<text:p><text:bookmark-start/>nameless<text:bookmark-end/></text:p>"#,
+    ] {
+        let content =
+            format!(r#"<office:document-content {NS_T}><office:body><office:text>{body}</office:text></office:body></office:document-content>"#);
+        let d = import(&package(&content, None)).unwrap();
+        let _ = export(&d).unwrap();
+    }
+}

crates/formats/src/odt/writer.rs

@@ -83,6 +83,10 @@ type SecStart = (String, Option<u32>);
 
 #[derive(Default)]
 pub(super) struct Writer {
+    /// Bookmarks that cover text (written as a start and an end), and those started and ended.
+    pub(super) ranged: std::collections::HashSet<String>,
+    bookmarks: std::collections::HashSet<String>,
+    bookmark_ends: std::collections::HashSet<String>,
     /// Indexes written so far (their names).
     pub(super) indexes: usize,
     /// An index's entries are being written: their links are in LibreOffice's Index Link style.
@@ -315,7 +319,22 @@ impl Writer {
                     self.group((*w, *h), float.as_deref(), members, out);
                     prev_space = false;
                 }
-                Inline::Anchor(a) => out.push_str(&format!("<text:bookmark text:name=\"{}\"/>", x(a))),
+                // A bookmark (§6.2.1): over the text up to its end, or a point; each name once.
+                Inline::Anchor(a) if self.bookmarks.insert(a.clone()) => {
+                    // One ending right where it starts is a point.
+                    let empty = matches!(inl.get(idx + 1), Some(Inline::AnchorEnd(e)) if e == a);
+                    if empty {
+                        self.bookmark_ends.insert(a.clone());
+                    }
+                    let el = if self.ranged.contains(a) && !empty { "bookmark-start" } else { "bookmark" };
+                    out.push_str(&format!("<text:{el} text:name=\"{}\"/>", x(a)));
+                }
+                Inline::Anchor(_) => {}
+                Inline::AnchorEnd(a) => {
+                    if self.ranged.contains(a) && self.bookmarks.contains(a) && self.bookmark_ends.insert(a.clone()) {
+                        out.push_str(&format!("<text:bookmark-end text:name=\"{}\"/>", x(a)));
+                    }
+                }
                 Inline::Note { endnote, custom, blocks } => {
                     self.note(*endnote, custom, blocks, out);
                     prev_space = false;

crates/formats/src/rtf/export.rs

@@ -408,7 +408,8 @@ impl Writer {
                 | Inline::Shape { .. }
                 | Inline::Group { .. }
                 | Inline::FieldStart(_)
-                | Inline::FieldEnd => {}
+                | Inline::FieldEnd
+                | Inline::AnchorEnd(_) => {}
             }
         }
     }