Bug 1639963 - Fix margin collapsing with aspect-ratio. r=emilio

Basically, we treat aspect-ratio (together with inline size) as a non-auto block size. This means the block is not empty when using aspect-ratio. Also, add 2 tentative wpts for this, based on the current spec issue examples. w3c/csswg-drafts#5328 Differential Revision: https://phabricator.services.mozilla.com/D84452
sidvishnoi · Aug 17, 2020 · 658e5ae · 658e5ae
1 parent 765ef7f
commit 658e5ae
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 1 deletion.
diff --git a/layout/generic/nsBlockFrame.cpp b/layout/generic/nsBlockFrame.cpp
@@ -1858,8 +1858,11 @@ void nsBlockFrame::ComputeFinalSize(const ReflowInput& aReflowInput,
         ComputeFinalBSize(aReflowInput, aState.mReflowStatus,
                           aState.mBCoord + nonCarriedOutBDirMargin,
                           borderPadding, aState.mConsumedBSize);
-
     // Don't carry out a block-end margin when our BSize is fixed.
+    //
+    // Note: this also includes the case that aReflowInput.ComputedBSize() is
+    // calculated from aspect-ratio. i.e. Don't carry out block margin-end if it
+    // is replaced by the block size from aspect-ratio and inline size.
     aMetrics.mCarriedOutBEndMargin.Zero();
   } else if (!IsComboboxControlFrame() &&
              aReflowInput.mStyleDisplay->IsContainSize()) {
@@ -3343,6 +3346,10 @@ static inline bool IsNonAutoNonZeroBSize(const StyleSize& aCoord) {
   return true;
 }
 
+static bool BehavesLikeInitialValueOnBlockAxis(const StyleSize& aCoord) {
+  return aCoord.IsAuto() || aCoord.IsExtremumLength();
+}
+
 /* virtual */
 bool nsBlockFrame::IsSelfEmpty() {
   // Blocks which are margin-roots (including inline-blocks) cannot be treated
@@ -3360,6 +3367,15 @@ bool nsBlockFrame::IsSelfEmpty() {
     return false;
   }
 
+  // FIXME: Bug 1646100 - Take intrinsic size into account.
+  // FIXME: Handle the case that both inline and block sizes are auto.
+  // https://github.com/w3c/csswg-drafts/issues/5060.
+  // Note: block-size could be zero or auto/intrinsic keywords here.
+  if (BehavesLikeInitialValueOnBlockAxis(position->BSize(wm)) &&
+      position->mAspectRatio.HasFiniteRatio()) {
+    return false;
+  }
+
   const nsStyleBorder* border = StyleBorder();
   const nsStylePadding* padding = StylePadding();
 

diff --git a/.../css/css-sizing/aspect-ratio/block-aspect-ratio-with-margin-collapsing-001.tentative.html b/.../css/css-sizing/aspect-ratio/block-aspect-ratio-with-margin-collapsing-001.tentative.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<title>CSS aspect-ratio: margin-collapsing with aspect-ratio on child element</title>
+<link rel="author" title="Mozilla" href="https://www.mozilla.org/">
+<link rel="help" href="https://drafts.csswg.org/css-sizing-4/#aspect-ratio">
+<link rel="help" href="https://drafts.csswg.org/css2/#collapsing-margins">
+<link rel="match" href="../../reference/ref-filled-green-100px-square.xht" />
+
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+
+<div style="background: green; width: 100px; height: 25px; margin-bottom: -50px;"></div>
+<div style="background: green; width: 100px; margin: 0px;">
+  <div style="margin-top: 50px; margin-bottom: 100px;
+              width: 100px; aspect-ratio: 2/1"></div>
+</div>
+<div style="background: green; width: 100px; height: 25px; margin-top: -100px;"></div>
diff --git a/.../css/css-sizing/aspect-ratio/block-aspect-ratio-with-margin-collapsing-002.tentative.html b/.../css/css-sizing/aspect-ratio/block-aspect-ratio-with-margin-collapsing-002.tentative.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<title>CSS aspect-ratio: margin-collapsing with aspect-ratio on parent element</title>
+<link rel="author" title="Mozilla" href="https://www.mozilla.org/">
+<link rel="help" href="https://drafts.csswg.org/css-sizing-4/#aspect-ratio">
+<link rel="help" href="https://drafts.csswg.org/css2/#collapsing-margins">
+<link rel="match" href="../../reference/ref-filled-green-100px-square.xht" />
+
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+
+<div style="background: green; width: 100px; height: 25px; margin-bottom: -200px;"></div>
+<div style="background: green; width: 100px; margin: 0px; aspect-ratio: 2/1">
+  <div style="width: 100px; margin-top: 50px; margin-bottom: 200px;"></div>
+</div>
+<div style="background: green; width: 100px; height: 25px;"></div>