Bug 1908069 - Add border/padding/margin support to . r=emilio

This is the first patch of the series that actually take
border/padding/margin into account for MathML layout. One subtility is
that Gecko distinguishes the ink nsBoundingMetrics box and the
ReflowOutput box for accurate positioning of math components contrary
to the current version of MathML Core [1].

For MathML Core and the WPT tests, it is assumed that border/padding are
used to inflate the content box [2] [3] and it does not really matter
whether we inflate the ink bounding box, or simply adjust its offsets
inside the normal bounding box. We choose the former and introduce a
helper function InflateReflowAndBoundingMetrics for that purpose, which
will be reused in follow-up patches. After inflating these boxes, the
offsets of children and painted items must also be adjusted by the
left/top border+padding.

Regarding margins, MathML Core says that we should use "margin boxes"
and WPT tests simply check that the layout of MathML constructions
containing mspace elements with margins is not changed if we move
these margins inside the mspace's width/height/depth attributes. To
pass these tests, we similarly need to inflate both the ink and
non-ink bounding boxes by the margin.

However, the margins can be negative possibly leading to negative
dimensions of "margin boxes". To make the code more generic, we
simplify adjust all usages of the children's nsBoundingMetrics and
ReflowOutput to include these margins. When calling FinishReflowChild
at the end, the offsets of children must be adjusted by these margins.

This patch also reimplements the extra one-pixel padding around a
fraction using rules from MathML Core's UA stylesheet at
https://w3c.github.io/mathml-core/#user-agent-stylesheet

[1] w3c/mathml-core#78
[2] https://github.com/web-platform-tests/wpt/blob/27c8d234ee83a4b15f36ad10cc26c951b1782fb6/mathml/relations/css-styling/padding-border-margin/border-002.html#L25
[4] https://github.com/web-platform-tests/wpt/blob/27c8d234ee83a4b15f36ad10cc26c951b1782fb6/mathml/relations/css-styling/padding-border-margin/padding-002.html#L26
[3] https://github.com/web-platform-tests/wpt/blob/27c8d234ee83a4b15f36ad10cc26c951b1782fb6/mathml/relations/css-styling/padding-border-margin/margin-003.html#L50

Differential Revision: https://phabricator.services.mozilla.com/D216670

Author: fred-wang

layout/mathml/mathml.css

@@ -72,6 +72,10 @@ math[display="inline" i] {
 
 */
 
+/* Fractions */
+mfrac {
+  padding-inline: 1px;
+}
 
 /**************************************************************************/
 /* merror                                                                 */

layout/mathml/nsMathMLContainerFrame.cpp

@@ -93,6 +93,46 @@ void nsMathMLContainerFrame::ClearSavedChildMetrics() {
   }
 }
 
+nsMargin nsMathMLContainerFrame::GetBorderPaddingForPlace(
+    const PlaceFlags& aFlags) {
+  if (aFlags.contains(PlaceFlag::IgnoreBorderPadding)) {
+    return nsMargin();
+  }
+
+  if (aFlags.contains(PlaceFlag::IntrinsicSize)) {
+    // Bug 1910859: Should we provide separate left and right border/padding?
+    return nsMargin(0, IntrinsicISizeOffsets().BorderPadding(), 0, 0);
+  }
+
+  return GetUsedBorderAndPadding();
+}
+
+/* static */
+nsMargin nsMathMLContainerFrame::GetMarginForPlace(const PlaceFlags& aFlags,
+                                                   nsIFrame* aChild) {
+  if (aFlags.contains(PlaceFlag::IntrinsicSize)) {
+    // Bug 1910859: Should we provide separate left and right margin?
+    return nsMargin(0, aChild->IntrinsicISizeOffsets().margin, 0, 0);
+  }
+
+  return aChild->GetUsedMargin();
+}
+
+void nsMathMLContainerFrame::InflateReflowAndBoundingMetrics(
+    const nsMargin& aBorderPadding, ReflowOutput& aReflowOutput,
+    nsBoundingMetrics& aBoundingMetrics) {
+  // Bug 1910858: It is not really clear what is the right way to update the
+  // ink bounding box when adding border or padding. Below, we assume that
+  // border/padding inflate it.
+  aBoundingMetrics.rightBearing += aBorderPadding.LeftRight();
+  aBoundingMetrics.width += aBorderPadding.LeftRight();
+  aReflowOutput.mBoundingMetrics = aBoundingMetrics;
+  aReflowOutput.Width() += aBorderPadding.LeftRight();
+  aReflowOutput.SetBlockStartAscent(aReflowOutput.BlockStartAscent() +
+                                    aBorderPadding.top);
+  aReflowOutput.Height() += aBorderPadding.TopBottom();
+}
+
 // helper to get the preferred size that a container frame should use to fire
 // the stretch on its stretchy child frames.
 void nsMathMLContainerFrame::GetPreferredStretchSize(

layout/mathml/nsMathMLContainerFrame.h

@@ -291,6 +291,13 @@ class nsMathMLContainerFrame : public nsContainerFrame, public nsMathMLFrame {
   // SaveReflowAndBoundingMetricsFor() from all child frames.
   void ClearSavedChildMetrics();
 
+  nsMargin GetBorderPaddingForPlace(const PlaceFlags& aFlags);
+  static nsMargin GetMarginForPlace(const PlaceFlags& aFlags, nsIFrame* aChild);
+
+  static void InflateReflowAndBoundingMetrics(
+      const nsMargin& aBorderPadding, ReflowOutput& aReflowOutput,
+      nsBoundingMetrics& aBoundingMetrics);
+
   // helper to let the update of presentation data pass through
   // a subtree that may contain non-MathML container frames
   static void PropagatePresentationDataFor(nsIFrame* aFrame,

layout/mathml/nsMathMLmfracFrame.cpp

@@ -170,6 +170,9 @@ nsresult nsMathMLmfracFrame::PlaceInternal(DrawTarget* aDrawTarget,
   GetReflowAndBoundingMetricsFor(frameNum, sizeNum, bmNum);
   GetReflowAndBoundingMetricsFor(frameDen, sizeDen, bmDen);
 
+  nsMargin numMargin = GetMarginForPlace(aFlags, frameNum),
+           denMargin = GetMarginForPlace(aFlags, frameDen);
+
   nsPresContext* presContext = PresContext();
   nscoord onePixel = nsPresContext::CSSPixelsToAppUnits(1);
 
@@ -206,13 +209,12 @@ nsresult nsMathMLmfracFrame::PlaceInternal(DrawTarget* aDrawTarget,
 
   mLineRect.height = mLineThickness;
 
-  // by default, leave at least one-pixel padding at either end, and add
-  // lspace & rspace that may come from  if we are an outermost
+  // Add lspace & rspace that may come from  if we are an outermost
   // embellished container (we fetch values from the core since they may use
   // units that depend on style data, and style changes could have occurred
   // in the core since our last visit there)
-  nscoord leftSpace = onePixel;
-  nscoord rightSpace = onePixel;
+  nscoord leftSpace = 0;
+  nscoord rightSpace = 0;
   if (outermostEmbellished) {
     const bool isRTL = StyleVisibility()->mDirection == StyleDirection::Rtl;
     nsEmbellishData coreData;
@@ -276,8 +278,8 @@ nsresult nsMathMLmfracFrame::PlaceInternal(DrawTarget* aDrawTarget,
           oneDevPixel);
     }
 
-    nscoord actualClearance =
-        (numShift - bmNum.descent) - (bmDen.ascent - denShift);
+    nscoord actualClearance = (numShift - bmNum.descent - numMargin.bottom) -
+                              (bmDen.ascent + denMargin.top - denShift);
     // actualClearance should be >= minClearance
     if (actualClearance < minClearance) {
       nscoord halfGap = (minClearance - actualClearance) / 2;
@@ -313,14 +315,14 @@ nsresult nsMathMLmfracFrame::PlaceInternal(DrawTarget* aDrawTarget,
     }
 
     // adjust numShift to maintain minClearanceNum if needed
-    nscoord actualClearanceNum =
-        (numShift - bmNum.descent) - (axisHeight + actualRuleThickness / 2);
+    nscoord actualClearanceNum = (numShift - bmNum.descent - numMargin.bottom) -
+                                 (axisHeight + actualRuleThickness / 2);
     if (actualClearanceNum < minClearanceNum) {
       numShift += (minClearanceNum - actualClearanceNum);
     }
     // adjust denShift to maintain minClearanceDen if needed
-    nscoord actualClearanceDen =
-        (axisHeight - actualRuleThickness / 2) - (bmDen.ascent - denShift);
+    nscoord actualClearanceDen = (axisHeight - actualRuleThickness / 2) -
+                                 (bmDen.ascent + denMargin.top - denShift);
     if (actualClearanceDen < minClearanceDen) {
       denShift += (minClearanceDen - actualClearanceDen);
     }
@@ -331,40 +333,59 @@ nsresult nsMathMLmfracFrame::PlaceInternal(DrawTarget* aDrawTarget,
 
   // XXX Need revisiting the width. TeX uses the exact width
   // e.g. in $$\huge\frac{\displaystyle\int}{i}$$
-  nscoord width = std::max(bmNum.width, bmDen.width);
-  nscoord dxNum = leftSpace + (width - sizeNum.Width()) / 2;
-  nscoord dxDen = leftSpace + (width - sizeDen.Width()) / 2;
+  nscoord width = std::max(bmNum.width + numMargin.LeftRight(),
+                           bmDen.width + denMargin.LeftRight());
+  nscoord dxNum =
+      leftSpace + (width - sizeNum.Width() - numMargin.LeftRight()) / 2;
+  nscoord dxDen =
+      leftSpace + (width - sizeDen.Width() - denMargin.LeftRight()) / 2;
   width += leftSpace + rightSpace;
 
   mBoundingMetrics.rightBearing =
-      std::max(dxNum + bmNum.rightBearing, dxDen + bmDen.rightBearing);
+      std::max(dxNum + bmNum.rightBearing + numMargin.LeftRight(),
+               dxDen + bmDen.rightBearing + denMargin.LeftRight());
   if (mBoundingMetrics.rightBearing < width - rightSpace)
     mBoundingMetrics.rightBearing = width - rightSpace;
   mBoundingMetrics.leftBearing =
       std::min(dxNum + bmNum.leftBearing, dxDen + bmDen.leftBearing);
   if (mBoundingMetrics.leftBearing > leftSpace)
     mBoundingMetrics.leftBearing = leftSpace;
-  mBoundingMetrics.ascent = bmNum.ascent + numShift;
-  mBoundingMetrics.descent = bmDen.descent + denShift;
+  mBoundingMetrics.ascent = bmNum.ascent + numShift + numMargin.top;
+  mBoundingMetrics.descent = bmDen.descent + denShift + denMargin.bottom;
   mBoundingMetrics.width = width;
 
-  aDesiredSize.SetBlockStartAscent(sizeNum.BlockStartAscent() + numShift);
-  aDesiredSize.Height() = aDesiredSize.BlockStartAscent() + sizeDen.Height() -
-                          sizeDen.BlockStartAscent() + denShift;
+  aDesiredSize.SetBlockStartAscent(numMargin.top + sizeNum.BlockStartAscent() +
+                                   numShift);
+  aDesiredSize.Height() = aDesiredSize.BlockStartAscent() + sizeDen.Height() +
+                          denMargin.bottom - sizeDen.BlockStartAscent() +
+                          denShift;
   aDesiredSize.Width() = mBoundingMetrics.width;
   aDesiredSize.mBoundingMetrics = mBoundingMetrics;
 
+  // Add padding+border.
+  auto borderPadding = GetBorderPaddingForPlace(aFlags);
+  InflateReflowAndBoundingMetrics(borderPadding, aDesiredSize,
+                                  mBoundingMetrics);
+  leftSpace += borderPadding.left;
+  rightSpace += borderPadding.right;
+  width += borderPadding.LeftRight();
+  dxNum += borderPadding.left;
+  dxDen += borderPadding.left;
+
   mReference.x = 0;
   mReference.y = aDesiredSize.BlockStartAscent();
 
   if (!aFlags.contains(PlaceFlag::MeasureOnly)) {
     nscoord dy;
     // place numerator
-    dy = 0;
+    dxNum += numMargin.left;
+    dy = borderPadding.top + numMargin.top;
     FinishReflowChild(frameNum, presContext, sizeNum, nullptr, dxNum, dy,
                       ReflowChildFlags::Default);
     // place denominator
-    dy = aDesiredSize.Height() - sizeDen.Height();
+    dxDen += denMargin.left;
+    dy = aDesiredSize.Height() - sizeDen.Height() - denMargin.bottom -
+         borderPadding.bottom;
     FinishReflowChild(frameDen, presContext, sizeDen, nullptr, dxDen, dy,
                       ReflowChildFlags::Default);
     // place the fraction bar - dy is top of bar

testing/web-platform/meta/mathml/presentation-markup/fractions/default-mfrac-padding-style.html.ini

@@ -1,2 +1,2 @@
 [frac-bar-002.html]
-  expected: FAIL
+  expected: [PASS, FAIL] # bug 1911053

testing/web-platform/meta/mathml/presentation-markup/fractions/frac-bar-002.html.ini

@@ -62,9 +62,6 @@
   [Border properties on mtext]
     expected: FAIL
 
-  [Border properties on mfrac]
-    expected: FAIL
-
   [Border properties on mover]
     expected: FAIL
 
@@ -98,9 +95,6 @@
   [Border properties on mi (rtl)]
     expected: FAIL
 
-  [Border properties on mfrac (rtl)]
-    expected: FAIL
-
   [Border properties on mo (rtl)]
     expected: FAIL
 

testing/web-platform/meta/mathml/relations/css-styling/padding-border-margin/border-002.html.ini

@@ -23,9 +23,6 @@
   [Margin properties on the children of msqrt]
     expected: FAIL
 
-  [Margin properties on the children of mfrac]
-    expected: FAIL
-
   [Margin properties on the children of mphantom]
     expected: FAIL
 

testing/web-platform/meta/mathml/relations/css-styling/padding-border-margin/margin-003.html.ini

@@ -50,9 +50,6 @@
   [Padding properties on msub]
     expected: FAIL
 
-  [Padding properties on mfrac]
-    expected: FAIL
-
   [Padding properties on mrow]
     expected: FAIL
 
@@ -128,9 +125,6 @@
   [Padding properties on mphantom (rtl)]
     expected: FAIL
 
-  [Padding properties on mfrac (rtl)]
-    expected: FAIL
-
   [Padding properties on msqrt (rtl)]
     expected: FAIL