Resolve a percentage odf margin, and give AnchorType a neutral value

CHANGELOG.md

@@ -16,6 +16,14 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- **Breaking**: `AnchorType` gains `none` as its first value, so every later
+  ordinal shifts by one. `Frame::anchor_type()` answers it for a frame that
+  does not exist, instead of `as_char`, which a real frame also answers.
+
+- **Fix**: a percentage `fo:margin` on an odf paragraph is resolved against the
+  parent style ([OpenDocument] 16.2) instead of being dropped. LibreOffice
+  writes `fo:margin="100%"` for "whatever the parent had".
+
 - **Fix**: `Text::set_content` on an `.xlsx` run did nothing and said nothing.
   An xlsx declares `edit`, so a caller had no way to learn the write was
   dropped; it now throws `UnsupportedOperation`, as every other engine that

apple/include/OdrCoreObjC/ODRDocumentElement.h

@@ -54,7 +54,9 @@ typedef NS_ENUM(NSInteger, ODRShapeType) {
 } NS_SWIFT_NAME(ShapeType);
 
 typedef NS_ENUM(NSInteger, ODRAnchorType) {
-  ODRAnchorTypeAsChar = 0,
+  /// The frame does not exist, so it is anchored nowhere.
+  ODRAnchorTypeNone = 0,
+  ODRAnchorTypeAsChar,
   ODRAnchorTypeAtChar,
   ODRAnchorTypeAtFrame,
   ODRAnchorTypeAtPage,

apple/src/ODRDocumentElement.mm

@@ -49,6 +49,7 @@
 ODR_SAME_ENUM(ODRShapeTypeLine, odr::ShapeType::line);
 ODR_SAME_ENUM(ODRShapeTypeCustom, odr::ShapeType::custom);
 
+ODR_SAME_ENUM(ODRAnchorTypeNone, odr::AnchorType::none);
 ODR_SAME_ENUM(ODRAnchorTypeAsChar, odr::AnchorType::as_char);
 ODR_SAME_ENUM(ODRAnchorTypeAtChar, odr::AnchorType::at_char);
 ODR_SAME_ENUM(ODRAnchorTypeAtFrame, odr::AnchorType::at_frame);

jni/java/app/opendocument/core/AnchorType.java

@@ -2,7 +2,13 @@
 
 /** Mirrors {@code odr::AnchorType}; constant order must match the C++ declaration. */
 public enum AnchorType {
-  AS_CHAR, AT_CHAR, AT_FRAME, AT_PAGE, AT_PARAGRAPH;
+  /** The frame does not exist, so it is anchored nowhere. */
+  NONE,
+  AS_CHAR,
+  AT_CHAR,
+  AT_FRAME,
+  AT_PAGE,
+  AT_PARAGRAPH;
 
   static AnchorType fromNative(int code) {
     return code < 0 ? null : values()[code];

python/src/bind_document.cpp

@@ -78,6 +78,7 @@ void odr_python::bind_document(py::module_ &m) {
       .value("custom", odr::ShapeType::custom);
 
   py::enum_<odr::AnchorType>(m, "AnchorType")
+      .value("none", odr::AnchorType::none)
       .value("as_char", odr::AnchorType::as_char)
       .value("at_char", odr::AnchorType::at_char)
       .value("at_frame", odr::AnchorType::at_frame)

src/odr/document_element.cpp

@@ -640,10 +640,8 @@ ShapeType Frame::shape_type() const {
 }
 
 AnchorType Frame::anchor_type() const {
-  // `AnchorType` has no neutral value, so a frame that does not exist answers
-  // the commonest one. Ask @ref Element::operator bool to tell the two apart.
   return exists_() ? m_adapter2->frame_anchor_type(m_identifier)
-                   : AnchorType::as_char;
+                   : AnchorType::none;
 }
 
 std::optional<Measure> Frame::x() const {

src/odr/document_element.hpp

@@ -123,6 +123,7 @@ enum class ShapeType {
 
 /// Collection of anchor types.
 enum class AnchorType {
+  none, ///< the frame does not exist, so it is anchored nowhere
   as_char,
   at_char,
   at_frame,

src/odr/internal/odf/odf_style.cpp

@@ -32,6 +32,21 @@ std::optional<Measure> read_measure(const pugi::xml_attribute attribute) {
   return {};
 }
 
+/// A percentage margin is of @p parent, the same margin in the parent style
+/// ([OpenDocument] 16.2) - not of the containing block, which is what css
+/// reads a `%` as.
+std::optional<Measure> resolve_margin(const Measure &margin,
+                                      const std::optional<Measure> &parent) {
+  if (margin.unit().name() != "%") {
+    return margin;
+  }
+  if (!parent.has_value()) {
+    return {};
+  }
+  return Measure(parent->magnitude() * margin.magnitude() * 1e-2,
+                 parent->unit());
+}
+
 std::optional<FontWeight>
 read_font_weight(const pugi::xml_attribute attribute) {
   if (!attribute) {
@@ -412,50 +427,29 @@ void Style::resolve_paragraph_style_(const pugi::xml_node node,
           paragraph_properties.attribute("style:writing-mode"))) {
     result.direction = direction;
   }
+  // the shorthand is the four sides, each against its own inherited value
   if (const std::optional<Measure> margin =
           read_measure(paragraph_properties.attribute("fo:margin"))) {
-    // TODO a percentage margin is dropped. css takes `%` here with the same
-    // meaning, so passing it through would work - but it moves the reference
-    // output for every document that uses one.
-    if (margin->unit().name() != "%") {
-      result.margin = DirectionalStyle(margin);
-    }
+    result.margin.right = resolve_margin(*margin, result.margin.right);
+    result.margin.top = resolve_margin(*margin, result.margin.top);
+    result.margin.left = resolve_margin(*margin, result.margin.left);
+    result.margin.bottom = resolve_margin(*margin, result.margin.bottom);
   }
   if (const std::optional<Measure> margin_right =
           read_measure(paragraph_properties.attribute("fo:margin-right"))) {
-    // TODO a percentage margin is dropped. css takes `%` here with the same
-    // meaning, so passing it through would work - but it moves the reference
-    // output for every document that uses one.
-    if (margin_right->unit().name() != "%") {
-      result.margin.right = margin_right;
-    }
+    result.margin.right = resolve_margin(*margin_right, result.margin.right);
   }
   if (const std::optional<Measure> margin_top =
           read_measure(paragraph_properties.attribute("fo:margin-top"))) {
-    // TODO a percentage margin is dropped. css takes `%` here with the same
-    // meaning, so passing it through would work - but it moves the reference
-    // output for every document that uses one.
-    if (margin_top->unit().name() != "%") {
-      result.margin.top = margin_top;
-    }
+    result.margin.top = resolve_margin(*margin_top, result.margin.top);
   }
   if (const std::optional<Measure> margin_left =
           read_measure(paragraph_properties.attribute("fo:margin-left"))) {
-    // TODO a percentage margin is dropped. css takes `%` here with the same
-    // meaning, so passing it through would work - but it moves the reference
-    // output for every document that uses one.
-    if (margin_left->unit().name() != "%") {
-      result.margin.left = margin_left;
-    }
+    result.margin.left = resolve_margin(*margin_left, result.margin.left);
   }
   if (const std::optional<Measure> margin_bottom =
           read_measure(paragraph_properties.attribute("fo:margin-bottom"))) {
-    // TODO a percentage margin is dropped. css takes `%` here with the same
-    // meaning, so passing it through would work - but it moves the reference
-    // output for every document that uses one.
-    if (margin_bottom->unit().name() != "%") {
-      result.margin.bottom = margin_bottom;
-    }
+    result.margin.bottom = resolve_margin(*margin_bottom, result.margin.bottom);
   }
   if (const std::optional<Measure> line_height =
           read_measure(paragraph_properties.attribute("fo:line-height"))) {

test/src/enum_ordinals_test.cpp

@@ -191,11 +191,12 @@ TEST(EnumOrdinals, shape_type) {
 }
 
 TEST(EnumOrdinals, anchor_type) {
-  EXPECT_EQ(ordinal(AnchorType::as_char), 0);
-  EXPECT_EQ(ordinal(AnchorType::at_char), 1);
-  EXPECT_EQ(ordinal(AnchorType::at_frame), 2);
-  EXPECT_EQ(ordinal(AnchorType::at_page), 3);
-  EXPECT_EQ(ordinal(AnchorType::at_paragraph), 4);
+  EXPECT_EQ(ordinal(AnchorType::none), 0);
+  EXPECT_EQ(ordinal(AnchorType::as_char), 1);
+  EXPECT_EQ(ordinal(AnchorType::at_char), 2);
+  EXPECT_EQ(ordinal(AnchorType::at_frame), 3);
+  EXPECT_EQ(ordinal(AnchorType::at_page), 4);
+  EXPECT_EQ(ordinal(AnchorType::at_paragraph), 5);
 }
 
 TEST(EnumOrdinals, value_type) {

test/src/internal/odf/odf_flat_file_test.cpp

@@ -718,3 +718,91 @@ TEST(FlatOpenDocumentFile, it_saves_back_as_one_xml_file) {
                 .content(),
             "Hello");
 }
+
+namespace {
+
+/// One paragraph, styled by @p properties over @p parent_properties.
+std::string one_paragraph(const std::string &properties,
+                          const std::string &parent_properties) {
+  return flat_text(
+      R"(<text:p text:style-name="P1">one</text:p>)",
+      R"(<office:automatic-styles>)"
+      R"(<style:style style:name="Base" style:family="paragraph">)"
+      R"(<style:paragraph-properties )" +
+          parent_properties +
+          R"(/></style:style>)"
+          R"(<style:style style:name="P1" style:family="paragraph")"
+          R"( style:parent-style-name="Base">)"
+          R"(<style:paragraph-properties )" +
+          properties +
+          R"(/></style:style>)"
+          R"(</office:automatic-styles>)");
+}
+
+/// @p properties over a parent stating `fo:margin-left="2cm"`.
+std::string margin_over_parent(const std::string &properties) {
+  return one_paragraph(properties, R"(fo:margin-left="2cm")");
+}
+
+DirectionalStyle<Measure> margins_of(const std::string &source) {
+  const Document document =
+      open(File::from_memory(source)).as_document_file().document();
+  return first_of_type(document.root_element(), ElementType::paragraph)
+      .as_paragraph()
+      .style()
+      .margin;
+}
+
+std::optional<Measure> margin_left_of(const std::string &source) {
+  return margins_of(source).left;
+}
+
+} // namespace
+
+/// The shorthand states all four sides.
+TEST(FlatOpenDocumentFile, a_margin_shorthand_states_every_side) {
+  const DirectionalStyle<Measure> margin =
+      margins_of(one_paragraph(R"(fo:margin="1cm")", ""));
+
+  for (const std::optional<Measure> &side :
+       {margin.right, margin.top, margin.left, margin.bottom}) {
+    ASSERT_TRUE(side.has_value());
+    EXPECT_EQ(side->to_string(), "1cm");
+  }
+}
+
+/// [OpenDocument] 16.2: a percentage is of the same property in the parent
+/// style, not of the containing block.
+TEST(FlatOpenDocumentFile, a_percentage_margin_is_of_the_parent_style) {
+  const std::optional<Measure> margin =
+      margin_left_of(margin_over_parent(R"(fo:margin-left="50%")"));
+
+  ASSERT_TRUE(margin.has_value());
+  EXPECT_DOUBLE_EQ(margin->magnitude(), 1);
+  EXPECT_EQ(margin->unit().name(), "cm");
+}
+
+/// The shorthand resolves each side against its own inherited value.
+TEST(FlatOpenDocumentFile, a_percentage_margin_shorthand_keeps_the_parent) {
+  const std::optional<Measure> margin =
+      margin_left_of(margin_over_parent(R"(fo:margin="100%")"));
+
+  ASSERT_TRUE(margin.has_value());
+  EXPECT_DOUBLE_EQ(margin->magnitude(), 2);
+  EXPECT_EQ(margin->unit().name(), "cm");
+}
+
+/// Nothing to be a percentage of leaves no margin at all.
+TEST(FlatOpenDocumentFile, a_percentage_margin_without_a_parent_states_none) {
+  EXPECT_FALSE(
+      margin_left_of(one_paragraph(R"(fo:margin-left="25%")", "")).has_value());
+}
+
+/// A percentage margin never reaches css, where `%` is of the containing
+/// block's width.
+TEST(FlatOpenDocumentFile, a_percentage_margin_is_resolved_before_the_css) {
+  const std::string html = render(margin_over_parent(R"(fo:margin="100%")"));
+
+  EXPECT_EQ(0U, count(html, "margin-left:100%"));
+  EXPECT_EQ(1U, count(html, "margin-left:2cm"));
+}