[mathml] Use accent/accentunder and AccentBaseHeight for underover layout

Partial support for the accent/accentunder attributes on the munder,
mover and munderover elements has been added in the UA stylesheet [1].

This CL modifies the munderover layout algorithm to take these boolean
attributes into account when determining gaps and shifts parameters as
specified in [2] and [3]. Additionally, for overscript we also
rely on the OpenType MATH.MathConstants.AccentBaseHeight parameter as
specified in [3].

This makes the remaining underover-parameters-3.html test pass. Note
that underover-parameters-4.html no longer corresponds to what is in
the MathML Core specification [4] and we will ignore it in our
implementation.

Finally, this CL extends underover-parameters-3.html to also test
dynamic changes of accent/accentunder attributes, using various
syntaxes for true/false.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2184131
[2] https://mathml-refresh.github.io/mathml-core/#base-with-underscript
[3] https://mathml-refresh.github.io/mathml-core/#base-with-overscript
[4] https://github.com/web-platform-tests/wpt/commit/aab53f17c0f688da550311ddc919c1702fb8681b

Bug: 6606, 1124285, 1124289
Change-Id: Ie589a2f2a00e6f49b15376bd83762d34c914fd65
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2391083Reviewed-by: Rob Buis <rbuis@igalia.com>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Frédéric Wang <fwang@igalia.com>
Cr-Commit-Position: refs/heads/master@{#804233}

third_party/blink/renderer/core/layout/ng/mathml/ng_math_under_over_layout_algorithm.cc

@@ -37,6 +37,9 @@ UnderOverVerticalParameters GetUnderOverVerticalParameters(
     bool is_base_large_operator,
     bool is_base_stretchy_in_inline_axis) {
   UnderOverVerticalParameters parameters;
+  const SimpleFontData* font_data = style.GetFont().PrimaryFont();
+  if (!font_data)
+    return parameters;
 
   // https://mathml-refresh.github.io/mathml-core/#dfn-default-fallback-constant
   const float default_fallback_constant = 0;
@@ -62,6 +65,7 @@ UnderOverVerticalParameters GetUnderOverVerticalParameters(
             .value_or(default_fallback_constant));
     parameters.under_extra_descender = LayoutUnit();
     parameters.over_extra_ascender = LayoutUnit();
+    parameters.accent_base_height = LayoutUnit();
     parameters.use_under_over_bar_fallback = false;
     return parameters;
   }
@@ -86,6 +90,7 @@ UnderOverVerticalParameters GetUnderOverVerticalParameters(
             .value_or(default_fallback_constant));
     parameters.under_extra_descender = LayoutUnit();
     parameters.over_extra_ascender = LayoutUnit();
+    parameters.accent_base_height = LayoutUnit();
     parameters.use_under_over_bar_fallback = false;
     return parameters;
   }
@@ -111,10 +116,27 @@ UnderOverVerticalParameters GetUnderOverVerticalParameters(
       MathConstant(style,
                    OpenTypeMathSupport::MathConstants::kOverbarExtraAscender)
           .value_or(default_rule_thickness));
+  parameters.accent_base_height = LayoutUnit(
+      MathConstant(style, OpenTypeMathSupport::MathConstants::kAccentBaseHeight)
+          .value_or(font_data->GetFontMetrics().XHeight() / 2));
   parameters.use_under_over_bar_fallback = true;
   return parameters;
 }
 
+// https://mathml-refresh.github.io/mathml-core/#underscripts-and-overscripts-munder-mover-munderover
+bool HasAccent(const NGBlockNode& node, bool accent_under) {
+  DCHECK(node);
+  auto* underover = To<MathMLUnderOverElement>(node.GetLayoutBox()->GetNode());
+  auto script_type = underover->GetScriptType();
+  DCHECK(script_type == MathScriptType::kUnderOver ||
+         (accent_under && script_type == MathScriptType::kUnder) ||
+         (!accent_under && script_type == MathScriptType::kOver));
+
+  base::Optional<bool> attribute_value =
+      accent_under ? underover->AccentUnder() : underover->Accent();
+  return attribute_value && *attribute_value;
+}
+
 }  // namespace
 
 NGMathUnderOverLayoutAlgorithm::NGMathUnderOverLayoutAlgorithm(
@@ -195,6 +217,17 @@ scoped_refptr<const NGLayoutResult> NGMathUnderOverLayoutAlgorithm::Layout() {
       Style(), is_base_large_operator, is_base_stretchy_in_inline_axis);
   // TODO(crbug.com/1124301): handle stretchy operators.
 
+  auto base_space = CreateConstraintSpaceForMathChild(
+      Node(), ChildAvailableSize(), ConstraintSpace(), base);
+  auto base_layout_result = base.Layout(base_space);
+  auto base_margins =
+      ComputeMarginsFor(base_space, base.Style(), ConstraintSpace());
+
+  NGBoxFragment base_fragment(
+      ConstraintSpace().GetWritingMode(), ConstraintSpace().Direction(),
+      To<NGPhysicalBoxFragment>(base_layout_result->PhysicalFragment()));
+  LayoutUnit base_ascent = base_fragment.BaselineOrSynthesize();
+
   // All children are positioned centered relative to the container (and
   // therefore centered relative to themselves).
   if (over) {
@@ -219,7 +252,12 @@ scoped_refptr<const NGLayoutResult> NGMathUnderOverLayoutAlgorithm::Layout() {
     over.StoreMargins(ConstraintSpace(), over_margins);
     if (parameters.use_under_over_bar_fallback) {
       block_offset += over_fragment.BlockSize();
-      block_offset += parameters.over_gap_min;
+      if (HasAccent(Node(), false)) {
+        if (base_ascent < parameters.accent_base_height)
+          block_offset += parameters.accent_base_height - base_ascent;
+      } else {
+        block_offset += parameters.over_gap_min;
+      }
     } else {
       LayoutUnit over_ascent = over_fragment.BaselineOrSynthesize();
       block_offset +=
@@ -229,16 +267,6 @@ scoped_refptr<const NGLayoutResult> NGMathUnderOverLayoutAlgorithm::Layout() {
     block_offset += over_margins.block_end;
   }
 
-  auto base_space = CreateConstraintSpaceForMathChild(
-      Node(), ChildAvailableSize(), ConstraintSpace(), base);
-  auto base_layout_result = base.Layout(base_space);
-  auto base_margins =
-      ComputeMarginsFor(base_space, base.Style(), ConstraintSpace());
-
-  NGBoxFragment base_fragment(
-      ConstraintSpace().GetWritingMode(), ConstraintSpace().Direction(),
-      To<NGPhysicalBoxFragment>(base_layout_result->PhysicalFragment()));
-
   block_offset += base_margins.block_start;
   LogicalOffset base_offset = {
       content_start_offset.inline_offset + base_margins.inline_start +
@@ -263,7 +291,8 @@ scoped_refptr<const NGLayoutResult> NGMathUnderOverLayoutAlgorithm::Layout() {
         To<NGPhysicalBoxFragment>(under_layout_result->PhysicalFragment()));
     block_offset += under_margins.block_start;
     if (parameters.use_under_over_bar_fallback) {
-      block_offset += parameters.under_gap_min;
+      if (!HasAccent(Node(), true))
+        block_offset += parameters.under_gap_min;
     } else {
       LayoutUnit under_ascent = under_fragment.BaselineOrSynthesize();
       block_offset += std::max(parameters.under_gap_min,
@@ -283,7 +312,6 @@ scoped_refptr<const NGLayoutResult> NGMathUnderOverLayoutAlgorithm::Layout() {
     block_offset += under_margins.block_end;
   }
 
-  LayoutUnit base_ascent = base_fragment.BaselineOrSynthesize();
   container_builder_.SetBaseline(base_offset.block_offset + base_ascent);
 
   block_offset += BorderScrollbarPadding().block_end;

third_party/blink/renderer/core/mathml/mathml_attribute_names.json5

@@ -7,6 +7,8 @@
   },
 
   data: [
+    "accent",
+    "accentunder",
     "definitionURL",
     "depth",
     "display",

third_party/blink/renderer/core/mathml/mathml_element.cc

@@ -110,6 +110,16 @@ void MathMLElement::ParseAttribute(const AttributeModificationParams& param) {
   Element::ParseAttribute(param);
 }
 
+base::Optional<bool> MathMLElement::BooleanAttribute(
+    const QualifiedName& name) const {
+  const AtomicString& value = FastGetAttribute(name);
+  if (EqualIgnoringASCIICase(value, "true"))
+    return true;
+  if (EqualIgnoringASCIICase(value, "false"))
+    return false;
+  return base::nullopt;
+}
+
 base::Optional<Length> MathMLElement::AddMathLengthToComputedStyle(
     const CSSToLengthConversionData& conversion_data,
     const QualifiedName& attr_name,

third_party/blink/renderer/core/mathml/mathml_element.h

@@ -30,6 +30,9 @@ class CORE_EXPORT MathMLElement : public Element {
     return HasLocalName(name.LocalName());
   }
 
+  bool IsMathMLElement() const =
+      delete;  // This will catch anyone doing an unnecessary check.
+
  protected:
   bool IsPresentationAttribute(const QualifiedName&) const override;
   void CollectStyleForPresentationAttribute(
@@ -45,8 +48,8 @@ class CORE_EXPORT MathMLElement : public Element {
 
   void ParseAttribute(const AttributeModificationParams&) override;
 
-  bool IsMathMLElement() const =
-      delete;  // This will catch anyone doing an unnecessary check.
+  // https://mathml-refresh.github.io/mathml-core/#dfn-boolean
+  base::Optional<bool> BooleanAttribute(const QualifiedName& name) const;
 };
 
 template <typename T>

third_party/blink/renderer/core/mathml/mathml_operator_element.cc

@@ -192,16 +192,6 @@ void MathMLOperatorElement::ComputeDictionaryCategory() {
   }
 }
 
-base::Optional<bool> MathMLOperatorElement::BooleanAttribute(
-    const QualifiedName& name) const {
-  const AtomicString& value = FastGetAttribute(name);
-  if (EqualIgnoringASCIICase(value, "true"))
-    return true;
-  if (EqualIgnoringASCIICase(value, "false"))
-    return false;
-  return base::nullopt;
-}
-
 void MathMLOperatorElement::ComputeOperatorProperty(OperatorPropertyFlag flag) {
   DCHECK(properties_.dirty_flags & flag);
   const auto& name = OperatorPropertyFlagToAttributeName(flag);

third_party/blink/renderer/core/mathml/mathml_operator_element.h

@@ -52,7 +52,6 @@ class CORE_EXPORT MathMLOperatorElement final : public MathMLElement {
   void ComputeOperatorProperty(OperatorPropertyFlag);
   void SetOperatorFormDirty();
   void ParseAttribute(const AttributeModificationParams&) final;
-  base::Optional<bool> BooleanAttribute(const QualifiedName& name) const;
   void SetOperatorPropertyDirtyFlagIfNeeded(const AttributeModificationParams&,
                                             const OperatorPropertyFlag&,
                                             bool& needs_layout);

third_party/blink/renderer/core/mathml/mathml_under_over_element.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "third_party/blink/renderer/core/mathml/mathml_under_over_element.h"
+#include "third_party/blink/renderer/core/layout/layout_object.h"
 
 namespace blink {
 
@@ -10,4 +11,25 @@ MathMLUnderOverElement::MathMLUnderOverElement(const QualifiedName& tagName,
                                                Document& document)
     : MathMLScriptsElement(tagName, document) {}
 
+base::Optional<bool> MathMLUnderOverElement::Accent() const {
+  return BooleanAttribute(mathml_names::kAccentAttr);
+}
+
+base::Optional<bool> MathMLUnderOverElement::AccentUnder() const {
+  return BooleanAttribute(mathml_names::kAccentunderAttr);
+}
+
+void MathMLUnderOverElement::ParseAttribute(
+    const AttributeModificationParams& param) {
+  if ((param.name == mathml_names::kAccentAttr ||
+       param.name == mathml_names::kAccentunderAttr) &&
+      GetLayoutObject() && GetLayoutObject()->IsMathML() &&
+      param.new_value != param.old_value) {
+    GetLayoutObject()
+        ->SetNeedsLayoutAndIntrinsicWidthsRecalcAndFullPaintInvalidation(
+            layout_invalidation_reason::kAttributeChanged);
+  }
+  MathMLScriptsElement::ParseAttribute(param);
+}
+
 }  // namespace blink

third_party/blink/renderer/core/mathml/mathml_under_over_element.h

@@ -14,6 +14,11 @@ class Document;
 class CORE_EXPORT MathMLUnderOverElement final : public MathMLScriptsElement {
  public:
   MathMLUnderOverElement(const QualifiedName& tagName, Document& document);
+  base::Optional<bool> Accent() const;
+  base::Optional<bool> AccentUnder() const;
+
+ private:
+  void ParseAttribute(const AttributeModificationParams&) final;
 };
 
 template <>

third_party/blink/web_tests/TestExpectations

@@ -1253,7 +1253,6 @@ crbug.com/6606 external/wpt/mathml/presentation-markup/operators/mo-movablelimit
 crbug.com/6606 external/wpt/mathml/presentation-markup/operators/mo-paint-lspace-rspace.html [ Failure ]
 crbug.com/6606 external/wpt/mathml/presentation-markup/operators/operator-dictionary-001.html [ Failure ]
 crbug.com/6606 external/wpt/mathml/presentation-markup/operators/operator-dictionary-002.html [ Failure ]
-crbug.com/6606 external/wpt/mathml/presentation-markup/scripts/underover-parameters-3.html [ Failure ]
 crbug.com/6606 external/wpt/mathml/presentation-markup/scripts/underover-parameters-4.html [ Failure ]
 crbug.com/6606 external/wpt/mathml/presentation-markup/spaces/space-like-001.html [ Failure ]
 crbug.com/6606 external/wpt/mathml/presentation-markup/spaces/space-like-002.html [ Failure ]

third_party/blink/web_tests/external/wpt/mathml/presentation-markup/scripts/underover-parameters-3.html

@@ -43,6 +43,35 @@
   setup({ explicit_done: true });
   window.addEventListener("load", () => { document.fonts.ready.then(runTests); });
 
+  function getBooleanValue(element, name) {
+      return (element.getAttribute(name) || "").toLowerCase() === "true";
+  }
+  let dynamicBooleanAttributeChanges = {
+      "Invert boolean value using absent attribute": function(element, name) {
+          if (getBooleanValue(element, name)) {
+              element.removeAttribute(name);
+          } else {
+              element.setAttribute(name, "true");
+          }
+      },
+
+      "Invert boolean value using invalid attribute": function(element, name) {
+          if (getBooleanValue(element, name)) {
+              element.setAttribute(name, "invalid");
+          } else {
+              element.setAttribute(name, "true");
+          }
+      },
+
+      "Change case of boolean attribute": function(element, name) {
+          if (getBooleanValue(element, name)) {
+              element.setAttribute(name, "TrUe");
+          } else {
+              element.setAttribute(name, "FaLsE");
+          }
+      }
+  };
+
   function runTests() {
     test(function() {
       assert_true(MathMLFeatureDetection.has_mspace());
@@ -192,6 +221,46 @@
       }
     }, "AccentBaseHeight, OverbarVerticalGap");
 
+    test(function() {
+      assert_true(MathMLFeatureDetection.has_mspace());
+      var v = 7000 * emToPx;
+
+      for (var j = 1; j <= 4; j++) {
+        var elId = `el005${j}`;
+        var baseId = `base005${j}`;
+        var underId = `under005${j}`;
+
+        for (name in dynamicBooleanAttributeChanges) {
+            let element = document.getElementById(elId);
+            dynamicBooleanAttributeChanges[name](element, "accentunder");
+            var value =  getBooleanValue(element, "accentunder");
+            var gap = value ? 0 : v;
+            assert_approx_equals(getBox(underId).top - getBox(baseId).bottom,
+                                 gap, epsilon,
+                                 `${elId}: gap between base and underscript ; ${name}`);
+        };
+      }
+    }, "Dynamic change of accentunder attribute");
+
+    test(function() {
+      assert_true(MathMLFeatureDetection.has_mspace());
+      v = 11000 * emToPx;
+      for (var j = 1; j <= 4; j++) {
+        var elId = `el006${j}`;
+        var refId = `base006${j}`;
+        var overId = `over006${j}`;
+        for (name in dynamicBooleanAttributeChanges) {
+            let element = document.getElementById(elId);
+            dynamicBooleanAttributeChanges[name](element, "accent");
+            var value =  getBooleanValue(element, "accent");
+            assert_approx_equals(getBox(refId).bottom - getBox(overId).bottom,
+                                 value ? axisBaseHeight : shortBaseHeight + v,
+                                 epsilon,
+                                 `${elId}: accent=${value} ; short base ; ${name}`);
+        }
+      }
+    }, "Dynamic change of accent attribute");
+
     done();
   }
 </script>
@@ -329,5 +398,54 @@
         </munderover>
       </math>
     </p>
+    <hr/>
+    <p>
+      <math style="font-family: accentbaseheight4000underbarverticalgap7000;">
+        <mspace id="ref005" height="1em" width="3em" style="background: green"/>
+        <munder style="background: cyan" id="el0051">
+          <mspace id="base0051" height="5em" width="1em" style="background: black"/>
+          <mspace id="under0051" height="1em" width="3em" style="background: blue"/>
+        </munder>
+        <munder style="background: cyan" id="el0052" accentunder="true">
+          <mspace id="base0052" height="5em" width="1em" style="background: black"/>
+          <mspace id="under0052" height="1em" width="3em" style="background: blue"/>
+        </munder>
+        <munderover style="background: cyan" id="el0053">
+          <mspace id="base0053" height="5em" width="1em" style="background: black"/>
+          <mspace id="under0053" height="1em" width="3em" style="background: blue"/>
+          <mspace id="over0053" height="1em" width="3em" style="background: red"/>
+        </munderover>
+        <munderover style="background: cyan" id="el0054" accentunder="true">
+          <mspace id="base0054" height="5em" width="1em" style="background: black"/>
+          <mspace id="under0054" height="1em" width="3em" style="background: blue"/>
+          <mspace id="over0054" height="1em" width="3em" style="background: red"/>
+        </munderover>
+      </math>
+    </p>
+    <hr/>
+    <p>
+      <math style="font-family: accentbaseheight4000overbarverticalgap11000;">
+        <mspace id="ref006" height="1em" width="3em" style="background: green"/>
+        <mover style="background: cyan" id="el0061">
+          <mspace id="base0061" height="3em" width="1em" style="background: black"/>
+          <mspace id="over0061" height="1em" width="3em" style="background: red"/>
+        </mover>
+        <mover style="background: cyan" id="el0062" accent="true">
+          <mspace id="base0062" height="3em" width="1em" style="background: black"/>
+          <mspace id="over0062" height="1em" width="3em" style="background: red"/>
+        </mover>
+        <munderover style="background: cyan" id="el0063">
+          <mspace id="base0063" height="3em" width="1em" style="background: black"/>
+          <mspace id="under0063" height="1em" width="3em" style="background: blue"/>
+          <mspace id="over0063" height="1em" width="3em" style="background: red"/>
+        </munderover>
+        <munderover style="background: cyan" id="el0064" accent="true">
+          <mspace id="base0064" height="3em" width="1em" style="background: black"/>
+          <mspace id="under0064" height="1em" width="3em" style="background: blue"/>
+          <mspace id="over0064" height="1em" width="3em" style="background: red"/>
+        </munderover>
+      </math>
+    </p>
+
 </body>
 </html>