From 03ca3dcbff175c102c9d3a3e433dbc9f89983fdf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Wang?= Date: Thu, 1 Aug 2024 12:58:06 +0000 Subject: [PATCH] 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] https://github.com/w3c/mathml-core/issues/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 --- layout/mathml/mathml.css | 4 ++ layout/mathml/nsMathMLContainerFrame.cpp | 40 ++++++++++++ layout/mathml/nsMathMLContainerFrame.h | 7 +++ layout/mathml/nsMathMLmfracFrame.cpp | 63 ++++++++++++------- .../default-mfrac-padding-style.html.ini | 3 - .../fractions/frac-bar-002.html.ini | 2 +- .../padding-border-margin/border-002.html.ini | 6 -- .../padding-border-margin/margin-003.html.ini | 3 - .../padding-002.html.ini | 6 -- 9 files changed, 94 insertions(+), 40 deletions(-) delete mode 100644 testing/web-platform/meta/mathml/presentation-markup/fractions/default-mfrac-padding-style.html.ini diff --git a/layout/mathml/mathml.css b/layout/mathml/mathml.css index a1cf371f4a426..c15fb7bcf5ce2 100644 --- a/layout/mathml/mathml.css +++ b/layout/mathml/mathml.css @@ -72,6 +72,10 @@ math[display="inline" i] { */ +/* Fractions */ +mfrac { + padding-inline: 1px; +} /**************************************************************************/ /* merror */ diff --git a/layout/mathml/nsMathMLContainerFrame.cpp b/layout/mathml/nsMathMLContainerFrame.cpp index 83038674164ed..e175195921050 100644 --- a/layout/mathml/nsMathMLContainerFrame.cpp +++ b/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( diff --git a/layout/mathml/nsMathMLContainerFrame.h b/layout/mathml/nsMathMLContainerFrame.h index f028cd320a0a9..acfedd2851f27 100644 --- a/layout/mathml/nsMathMLContainerFrame.h +++ b/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, diff --git a/layout/mathml/nsMathMLmfracFrame.cpp b/layout/mathml/nsMathMLmfracFrame.cpp index d28cc3f904e7a..a7d2f828430ea 100644 --- a/layout/mathml/nsMathMLmfracFrame.cpp +++ b/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 diff --git a/testing/web-platform/meta/mathml/presentation-markup/fractions/default-mfrac-padding-style.html.ini b/testing/web-platform/meta/mathml/presentation-markup/fractions/default-mfrac-padding-style.html.ini deleted file mode 100644 index 12c9dffcc99d7..0000000000000 --- a/testing/web-platform/meta/mathml/presentation-markup/fractions/default-mfrac-padding-style.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[default-mfrac-padding-style.html] - [Default CSS properties on mfrac] - expected: FAIL diff --git a/testing/web-platform/meta/mathml/presentation-markup/fractions/frac-bar-002.html.ini b/testing/web-platform/meta/mathml/presentation-markup/fractions/frac-bar-002.html.ini index 11b44665457c0..2dd093f3e9636 100644 --- a/testing/web-platform/meta/mathml/presentation-markup/fractions/frac-bar-002.html.ini +++ b/testing/web-platform/meta/mathml/presentation-markup/fractions/frac-bar-002.html.ini @@ -1,2 +1,2 @@ [frac-bar-002.html] - expected: FAIL + expected: [PASS, FAIL] # bug 1911053 diff --git a/testing/web-platform/meta/mathml/relations/css-styling/padding-border-margin/border-002.html.ini b/testing/web-platform/meta/mathml/relations/css-styling/padding-border-margin/border-002.html.ini index 9d0b2abf122b7..b731c19e80621 100644 --- a/testing/web-platform/meta/mathml/relations/css-styling/padding-border-margin/border-002.html.ini +++ b/testing/web-platform/meta/mathml/relations/css-styling/padding-border-margin/border-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 diff --git a/testing/web-platform/meta/mathml/relations/css-styling/padding-border-margin/margin-003.html.ini b/testing/web-platform/meta/mathml/relations/css-styling/padding-border-margin/margin-003.html.ini index 8e93cfbf49840..175f76d41faff 100644 --- a/testing/web-platform/meta/mathml/relations/css-styling/padding-border-margin/margin-003.html.ini +++ b/testing/web-platform/meta/mathml/relations/css-styling/padding-border-margin/margin-003.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 diff --git a/testing/web-platform/meta/mathml/relations/css-styling/padding-border-margin/padding-002.html.ini b/testing/web-platform/meta/mathml/relations/css-styling/padding-border-margin/padding-002.html.ini index da204fc932442..443dc80abafd1 100644 --- a/testing/web-platform/meta/mathml/relations/css-styling/padding-border-margin/padding-002.html.ini +++ b/testing/web-platform/meta/mathml/relations/css-styling/padding-border-margin/padding-002.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