Bug 958714 part 1: Remove special case for flex & grid items' percent block-axis margin/padding resolution, to align with other browsers. r?mats draft
authorDaniel Holbert <dholbert@cs.stanford.edu>
Tue, 30 Jan 2018 22:24:51 -0500
changeset 749213 1deb9b472ce46cfce864875c816429700b849b4c
parent 749209 1fb377cec16e9d3100d74a909bd8d1d6e0462f7c
child 749214 9d523be9fa10f648dc90aea33fe79b6dd57d68be
push id97355
push userdholbert@mozilla.com
push dateWed, 31 Jan 2018 03:25:14 +0000
reviewersmats
bugs958714
milestone60.0a1
Bug 958714 part 1: Remove special case for flex & grid items' percent block-axis margin/padding resolution, to align with other browsers. r?mats Now, flex and grid items will resolve percent margin and padding against their container's *inline-size*, even if the percent margin/padding is in the block axis. This matches the CSS2 behavior that's always existed in block containers. MozReview-Commit-ID: K3YXHpdqRHa
layout/generic/ReflowInput.cpp
layout/reftests/css-grid/grid-auto-min-sizing-definite-001-ref.html
layout/reftests/css-grid/grid-item-sizing-percent-001-ref.html
layout/reftests/css-grid/grid-item-sizing-percent-001.html
layout/reftests/css-grid/grid-item-sizing-px-001.html
layout/reftests/css-grid/reftest.list
layout/reftests/w3c-css/submitted/flexbox/flexbox-mbp-horiz-004-ref.xhtml
layout/reftests/w3c-css/submitted/flexbox/flexbox-mbp-horiz-004.xhtml
testing/web-platform/tests/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-mbp-horiz-004-ref.xhtml
testing/web-platform/tests/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-mbp-horiz-004.xhtml
--- a/layout/generic/ReflowInput.cpp
+++ b/layout/generic/ReflowInput.cpp
@@ -2213,24 +2213,21 @@ IsSideCaption(nsIFrame* aFrame, const ns
 // For everything else: the CSS21 spec requires that margin and padding
 // percentage values are calculated with respect to the inline-size of the
 // containing block, even for margin & padding in the block axis.
 static LogicalSize
 OffsetPercentBasis(const nsIFrame*    aFrame,
                    WritingMode        aWM,
                    const LogicalSize& aContainingBlockSize)
 {
+  // XXX The next patch in this series will get rid of this function and have
+  // the upstream/downstream code just work with the one nscoord value that
+  // we'll be dealing with now (which is aContainingBlockSize.ISize(aWM)).
   LogicalSize offsetPercentBasis = aContainingBlockSize;
-  if (MOZ_LIKELY(!aFrame->GetParent() ||
-                 !aFrame->GetParent()->IsFlexOrGridContainer())) {
-    offsetPercentBasis.BSize(aWM) = offsetPercentBasis.ISize(aWM);
-  } else if (offsetPercentBasis.BSize(aWM) == NS_AUTOHEIGHT) {
-    offsetPercentBasis.BSize(aWM) = 0;
-  }
-
+  offsetPercentBasis.BSize(aWM) = offsetPercentBasis.ISize(aWM);
   return offsetPercentBasis;
 }
 
 // XXX refactor this code to have methods for each set of properties
 // we are computing: width,height,line-height; margin; offsets
 
 void
 ReflowInput::InitConstraints(nsPresContext* aPresContext,
--- a/layout/reftests/css-grid/grid-auto-min-sizing-definite-001-ref.html
+++ b/layout/reftests/css-grid/grid-auto-min-sizing-definite-001-ref.html
@@ -62,28 +62,28 @@ b40 {
   z-index: 1; position:relative;
 }
 
 .h.r {
   height: 42px;
   width: 42px;
   /* This margin-left is 20% of 98px-wide grid area */
   margin-left: 19.6px;
-  /* This padding-bottom is 10% of 47.5px tall grid area */
+  /* This padding-bottom is 10% of 98px wide grid area */
   /* This padding-left   is 30% of 98px wide grid area */
-  padding: 1px 3px 4.75px 29.4px;
+  padding: 1px 3px 9.8px 29.4px;
 }
 .v.r {
   height: 42px;
   width: 42px;
   /* This margin-left is 20% of 54px-wide grid area */
   margin-left: 10.8px;
-  /* This padding-bottom is 10% of 102.5px tall grid area */
+  /* This padding-bottom is 10% of 54px wide grid area */
   /* This padding-left   is 30% of 54px wide grid area */
-  padding: 1px 3px 10.25px 16.2px;
+  padding: 1px 3px 5.4px 16.2px;
 }
 
 .r { position:relative; }
 
 .t6 { width:46px; }
 .t8 { width:118px; height: 102.5px; }
 
 xx {
--- a/layout/reftests/css-grid/grid-item-sizing-percent-001-ref.html
+++ b/layout/reftests/css-grid/grid-item-sizing-percent-001-ref.html
@@ -25,18 +25,18 @@ body,html { color:black; background:whit
 #abs > div div {
   position: absolute;
 }
 .pbox {
   box-sizing: border-box;
 }
 .p { padding:2px 3px; }
 .m { margin:2px 3px; }
-.c1.p,.c2.p { padding:5px 9px; }
-.c1.m,.c2.m { margin:5px 9px; }
+.c1.p,.c2.p { padding:6px 9px; }
+.c1.m,.c2.m { margin:6px 9px; }
 .b { border:solid black; }
 
 #t1 { width:50px; height: 20px; }
 #t2 { width:50px; height: 20px; }
 #t0 { width:60px; height: 30px; }
 #t3 { width:60px; height: 30px; }
 #t4 { width:150px; height: 50px; }
 #t5 { width:50px; height: 20px; }
--- a/layout/reftests/css-grid/grid-item-sizing-percent-001.html
+++ b/layout/reftests/css-grid/grid-item-sizing-percent-001.html
@@ -43,18 +43,18 @@ body,html { color:black; background:whit
 }
 #abs > div div {
   position: absolute;
 }
 .pbox {
   box-sizing: border-box;
 }
 
-.p { padding:10% 6%; }
-.m { margin:10% 6%; }
+.p { padding:4% 6%; }
+.m { margin:4% 6%; }
 .b { border:solid black; }
 
 </style>
 </head>
 <body>
 
 <div style="float:left">
 <div class="grid"><div class="item"></div></div>
--- a/layout/reftests/css-grid/grid-item-sizing-px-001.html
+++ b/layout/reftests/css-grid/grid-item-sizing-px-001.html
@@ -41,18 +41,18 @@ body,html { color:black; background:whit
   position: absolute;
 }
 .pbox {
   box-sizing: border-box;
 }
 
 .p { padding:2px 3px; }
 .m { margin:2px 3px; }
-.c1.p,.c2.p { padding:5px 9px; }
-.c1.m,.c2.m { margin:5px 9px; }
+.c1.p,.c2.p { padding:6px 9px; }
+.c1.m,.c2.m { margin:6px 9px; }
 .b { border:solid black; }
 
 </style>
 </head>
 <body>
 
 <div style="float:left">
 <div class="grid"><div class="item"></div></div>
--- a/layout/reftests/css-grid/reftest.list
+++ b/layout/reftests/css-grid/reftest.list
@@ -38,17 +38,17 @@ skip-if(Android) == grid-placement-defin
 skip-if(Android) fuzzy-if(winWidget,1,32) == grid-placement-auto-implicit-001.html grid-placement-auto-implicit-001-ref.html
 == grid-placement-abspos-implicit-001.html grid-placement-abspos-implicit-001-ref.html
 == rtl-grid-placement-definite-001.html rtl-grid-placement-definite-001-ref.html
 == rtl-grid-placement-auto-row-sparse-001.html rtl-grid-placement-auto-row-sparse-001-ref.html
 == vlr-grid-placement-auto-row-sparse-001.html vlr-grid-placement-auto-row-sparse-001-ref.html
 == vrl-grid-placement-auto-row-sparse-001.html vrl-grid-placement-auto-row-sparse-001-ref.html
 == grid-relpos-items-001.html grid-relpos-items-001-ref.html
 == grid-item-sizing-percent-001.html grid-item-sizing-percent-001-ref.html
-== grid-item-sizing-percent-002.html grid-item-sizing-percent-002-ref.html
+fails == grid-item-sizing-percent-002.html grid-item-sizing-percent-002-ref.html # bug 1434397
 == grid-item-sizing-percent-003.html grid-item-sizing-percent-003-ref.html
 == grid-item-sizing-percent-004.html grid-item-sizing-percent-004-ref.html
 == grid-item-sizing-px-001.html grid-item-sizing-percent-001-ref.html
 == grid-item-dir-001.html grid-item-dir-001-ref.html
 fuzzy-if(winWidget,70,130) fuzzy-if(cocoaWidget,85,180) == grid-col-max-sizing-max-content-001.html grid-col-max-sizing-max-content-001-ref.html
 fuzzy-if(winWidget,70,130) fuzzy-if(cocoaWidget,85,180) == grid-col-max-sizing-max-content-002.html grid-col-max-sizing-max-content-002-ref.html
 == grid-min-max-content-sizing-001.html grid-min-max-content-sizing-001-ref.html
 == grid-min-max-content-sizing-002.html grid-min-max-content-sizing-002-ref.html
--- a/layout/reftests/w3c-css/submitted/flexbox/flexbox-mbp-horiz-004-ref.xhtml
+++ b/layout/reftests/w3c-css/submitted/flexbox/flexbox-mbp-horiz-004-ref.xhtml
@@ -1,52 +1,35 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!--
      Any copyright is dedicated to the Public Domain.
      http://creativecommons.org/publicdomain/zero/1.0/
 -->
 <!-- Reference case - identical to the testcase, but with with the flex items'
-     vertical margin and padding values set to 0 by default, and then set to
-     specific pixel values for the items that have a 50px percent-basis.
+     margin and padding values set to explicit pixel values.
 -->
 <html xmlns="http://www.w3.org/1999/xhtml">
   <head>
     <title>CSS Reftest Reference</title>
     <link rel="author" title="Daniel Holbert" href="mailto:dholbert@mozilla.com"/>
     <style>
       div { border: 0; }
       div.flexbox {
         width: 200px;
         display: flex;
         margin-bottom: 2px;
         border: 1px dotted black;
       }
       div.height50 { height: 50px; }
 
-      .marginA  { margin:  0  8% 0  4%; }
-      .marginB  { margin:  0 10% 0 14%; }
-      .paddingA { padding: 0  6% 0  2%; }
-      .paddingB { padding: 0  8% 0 12%; }
+      .marginA  { margin:  20px 16px 12px  8px; }
+      .marginB  { margin:  16px 20px 24px 28px; }
+      .paddingA { padding: 16px 12px  8px  4px; }
+      .paddingB { padding: 12px 16px 20px 24px; }
 
-      div.height50 > .marginA {
-        margin-top: 5px;
-        margin-bottom: 3px;
-      }
-      div.height50 > .marginB {
-        margin-top: 4px;
-        margin-bottom: 6px;
-      }
-      div.height50 > .paddingA {
-        padding-top: 4px;
-        padding-bottom: 2px;
-      }
-      div.height50 > .paddingB {
-        padding-top: 3px;
-        padding-bottom: 5px;
-      }
 
       div.child1 {
         flex: none;
         width: 10px;
         height: 10px;
         background: lightgreen;
       }
       div.child2 {
--- a/layout/reftests/w3c-css/submitted/flexbox/flexbox-mbp-horiz-004.xhtml
+++ b/layout/reftests/w3c-css/submitted/flexbox/flexbox-mbp-horiz-004.xhtml
@@ -1,26 +1,24 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!--
      Any copyright is dedicated to the Public Domain.
      http://creativecommons.org/publicdomain/zero/1.0/
 -->
-<!-- Testcase with percent-valued padding and/or margin on flex items. The spec
-     says that percentage values on padding/margin-top and -bottom should be
-     resolved against the flex container's height (not its width, as would
-     be the case in a block).
+<!-- Testcase with percent-valued padding and/or margin on flex items.
+     The spec allows these to be resolved against the flex container's
+     inline size (regardless of which axis the percent padding/margin is in).
 -->
 <html xmlns="http://www.w3.org/1999/xhtml">
   <head>
     <title>CSS Test: Testing percent-valued padding and margin on flex items</title>
     <link rel="author" title="Daniel Holbert" href="mailto:dholbert@mozilla.com"/>
-    <link rel="help" href="http://www.w3.org/TR/css-flexbox-1/#layout-algorithm"/>
+    <link rel="help" href="https://www.w3.org/TR/css-flexbox-1/#item-margins"/>
     <link rel="match" href="flexbox-mbp-horiz-004-ref.xhtml"/>
     <style>
-      div { border: 0; }
       div.flexbox {
         width: 200px;
         display: flex;
         margin-bottom: 2px;
         border: 1px dotted black;
       }
       div.height50 { height: 50px; }
 
@@ -47,27 +45,29 @@
         height: 10px;
         width: 100%;
         background: lightgrey;
       }
 
     </style>
   </head>
   <body>
-    <!-- Flex container is auto-height - vertical margin and padding should
-         resolve to 0, since they don't have anything to resolve % against. -->
+    <!-- Flex container is auto-height - this shouldn't impact percent
+         margin/padding resolution, since they resolve against container's
+         inline-size, i.e. its width in this case. -->
     <div class="flexbox"
          ><div class="child1 paddingA"><div class="filler"/></div
          ><div class="child2 paddingB"><div class="filler"/></div
          ><div class="child1 marginA"></div
          ><div class="child2 marginB"></div
     ></div>
 
-    <!-- Flex container has height: 50px - vertical margin and padding should
-         resolve % values against that. -->
+    <!-- Flex container has height: 50px - again, this shouldn't impact percent
+         margin/padding resolution, since they resolve against container's
+         inline-size, i.e. its width in this case. -->
     <div class="flexbox height50"
          ><div class="child1 paddingA"><div class="filler"/></div
          ><div class="child2 paddingB"><div class="filler"/></div
          ><div class="child1 marginA"></div
          ><div class="child2 marginB"></div
     ></div>
 
     <!-- ...and now with align-items: flex-end, so we can see the margin-bottom
--- a/testing/web-platform/tests/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-mbp-horiz-004-ref.xhtml
+++ b/testing/web-platform/tests/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-mbp-horiz-004-ref.xhtml
@@ -1,52 +1,35 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!--
      Any copyright is dedicated to the Public Domain.
      http://creativecommons.org/publicdomain/zero/1.0/
 -->
 <!-- Reference case - identical to the testcase, but with with the flex items'
-     vertical margin and padding values set to 0 by default, and then set to
-     specific pixel values for the items that have a 50px percent-basis.
+     margin and padding values set to explicit pixel values.
 -->
 <html xmlns="http://www.w3.org/1999/xhtml">
   <head>
     <title>CSS Reftest Reference</title>
     <link rel="author" title="Daniel Holbert" href="mailto:dholbert@mozilla.com"/>
     <style>
       div { border: 0; }
       div.flexbox {
         width: 200px;
         display: flex;
         margin-bottom: 2px;
         border: 1px dotted black;
       }
       div.height50 { height: 50px; }
 
-      .marginA  { margin:  0  8% 0  4%; }
-      .marginB  { margin:  0 10% 0 14%; }
-      .paddingA { padding: 0  6% 0  2%; }
-      .paddingB { padding: 0  8% 0 12%; }
+      .marginA  { margin:  20px 16px 12px  8px; }
+      .marginB  { margin:  16px 20px 24px 28px; }
+      .paddingA { padding: 16px 12px  8px  4px; }
+      .paddingB { padding: 12px 16px 20px 24px; }
 
-      div.height50 > .marginA {
-        margin-top: 5px;
-        margin-bottom: 3px;
-      }
-      div.height50 > .marginB {
-        margin-top: 4px;
-        margin-bottom: 6px;
-      }
-      div.height50 > .paddingA {
-        padding-top: 4px;
-        padding-bottom: 2px;
-      }
-      div.height50 > .paddingB {
-        padding-top: 3px;
-        padding-bottom: 5px;
-      }
 
       div.child1 {
         flex: none;
         width: 10px;
         height: 10px;
         background: lightgreen;
       }
       div.child2 {
--- a/testing/web-platform/tests/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-mbp-horiz-004.xhtml
+++ b/testing/web-platform/tests/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-mbp-horiz-004.xhtml
@@ -1,26 +1,24 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!--
      Any copyright is dedicated to the Public Domain.
      http://creativecommons.org/publicdomain/zero/1.0/
 -->
-<!-- Testcase with percent-valued padding and/or margin on flex items. The spec
-     says that percentage values on padding/margin-top and -bottom should be
-     resolved against the flex container's height (not its width, as would
-     be the case in a block).
+<!-- Testcase with percent-valued padding and/or margin on flex items.
+     The spec allows these to be resolved against the flex container's
+     inline size (regardless of which axis the percent padding/margin is in).
 -->
 <html xmlns="http://www.w3.org/1999/xhtml">
   <head>
     <title>CSS Test: Testing percent-valued padding and margin on flex items</title>
     <link rel="author" title="Daniel Holbert" href="mailto:dholbert@mozilla.com"/>
-    <link rel="help" href="http://www.w3.org/TR/css-flexbox-1/#layout-algorithm"/>
+    <link rel="help" href="https://www.w3.org/TR/css-flexbox-1/#item-margins"/>
     <link rel="match" href="flexbox-mbp-horiz-004-ref.xhtml"/>
     <style>
-      div { border: 0; }
       div.flexbox {
         width: 200px;
         display: flex;
         margin-bottom: 2px;
         border: 1px dotted black;
       }
       div.height50 { height: 50px; }
 
@@ -47,27 +45,29 @@
         height: 10px;
         width: 100%;
         background: lightgrey;
       }
 
     </style>
   </head>
   <body>
-    <!-- Flex container is auto-height - vertical margin and padding should
-         resolve to 0, since they don't have anything to resolve % against. -->
+    <!-- Flex container is auto-height - this shouldn't impact percent
+         margin/padding resolution, since they resolve against container's
+         inline-size, i.e. its width in this case. -->
     <div class="flexbox"
          ><div class="child1 paddingA"><div class="filler"/></div
          ><div class="child2 paddingB"><div class="filler"/></div
          ><div class="child1 marginA"></div
          ><div class="child2 marginB"></div
     ></div>
 
-    <!-- Flex container has height: 50px - vertical margin and padding should
-         resolve % values against that. -->
+    <!-- Flex container has height: 50px - again, this shouldn't impact percent
+         margin/padding resolution, since they resolve against container's
+         inline-size, i.e. its width in this case. -->
     <div class="flexbox height50"
          ><div class="child1 paddingA"><div class="filler"/></div
          ><div class="child2 paddingB"><div class="filler"/></div
          ><div class="child1 marginA"></div
          ><div class="child2 marginB"></div
     ></div>
 
     <!-- ...and now with align-items: flex-end, so we can see the margin-bottom