Bug 1455976: Give table wrapper boxes a special case during flex base size resolution, so that percent main-sizes can be respected. r?mats draft
authorDaniel Holbert <dholbert@cs.stanford.edu>
Mon, 23 Apr 2018 12:05:40 -0700
changeset 786662 402f4c2dc00b38a795e1c75834117e09ba96d8d6
parent 786478 dfb15917c057f17e5143f7d7c6e1972ba53efc49
push id107554
push userdholbert@mozilla.com
push dateMon, 23 Apr 2018 19:05:51 +0000
reviewersmats
bugs1455976
milestone61.0a1
Bug 1455976: Give table wrapper boxes a special case during flex base size resolution, so that percent main-sizes can be respected. r?mats MozReview-Commit-ID: GB3SCaj9cv1
layout/generic/nsFrame.cpp
layout/reftests/flexbox/flexbox-table-flex-items-2-ref.html
layout/reftests/flexbox/flexbox-table-flex-items-2.html
layout/reftests/flexbox/flexbox-table-flex-items-3-ref.html
layout/reftests/flexbox/flexbox-table-flex-items-3.html
layout/reftests/flexbox/reftest.list
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -5671,18 +5671,25 @@ nsFrame::ComputeSize(gfxContext*        
 
     // NOTE: The logic here should match the similar chunk for updating
     // mainAxisCoord in nsFrame::ComputeSizeWithIntrinsicDimensions() (aside
     // from using a different dummy value in the IsUsedFlexBasisContent() case).
     const nsStyleCoord* flexBasis = &(stylePos->mFlexBasis);
     auto& mainAxisCoord = (flexMainAxis == eLogicalAxisInline
                            ? inlineStyleCoord : blockStyleCoord);
 
+    // NOTE: If we're a table-wrapper frame, we skip this clause and just stick
+    // with 'main-size:auto' behavior (which -- unlike 'content'
+    // i.e. 'max-content' -- will give us the ability to honor percent sizes on
+    // our table-box child when resolving the flex base size). The flexbox spec
+    // doesn't call for this special case, but webcompat & regression-avoidance
+    // seems to require it, for the time being... Tables sure are special.
     if (nsFlexContainerFrame::IsUsedFlexBasisContent(flexBasis,
-                                                     mainAxisCoord)) {
+                                                     mainAxisCoord) &&
+        MOZ_LIKELY(!IsTableWrapperFrame())) {
       static const nsStyleCoord maxContStyleCoord(NS_STYLE_WIDTH_MAX_CONTENT,
                                                   eStyleUnit_Enumerated);
       mainAxisCoord = &maxContStyleCoord;
       // (Note: if our main axis is the block axis, then this 'max-content'
       // value will be treated like 'auto', via the IsAutoBSize() call below.)
     } else if (flexBasis->GetUnit() != eStyleUnit_Auto) {
       // For all other non-'auto' flex-basis values, we just swap in the
       // flex-basis itself for the main-size property.
new file mode 100644
--- /dev/null
+++ b/layout/reftests/flexbox/flexbox-table-flex-items-2-ref.html
@@ -0,0 +1,64 @@
+<!DOCTYPE html>
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>CSS Reftest Reference</title>
+  <link rel="author" title="Daniel Holbert" href="mailto:dholbert@mozilla.com">
+  <link rel="help" href="https://www.w3.org/TR/css-flexbox-1/#flex-base-size">
+  <style>
+  .container {
+    display: flex;
+    width: 100px;
+    border: 1px solid black;
+  }
+
+  /* Two types of flex items: */
+  .table {
+    border: 2px solid teal;
+  }
+  .block {
+    border: 2px solid brown;
+  }
+
+  /* Each flex item gets one of these as its contents,
+     to have a nonzero content size: */
+  ib {
+    display: inline-block;
+    background: blue;
+    border: 1px solid gray;
+    width: 15px;
+    height: 10px;
+  }
+  </style>
+</head>
+<body>
+<!-- auto size: -->
+<div class="container">
+  <div class="table"><ib></ib></div>
+  <div class="block"><ib></ib></div>
+</div>
+
+<!-- px size: -->
+<div class="container">
+  <div class="table" style="width: 30px"><ib></ib></div>
+  <div class="block" style="width: 30px"><ib></ib></div>
+</div>
+
+<!-- % size: -->
+<div class="container">
+  <div class="table" style="width: 30%"><ib></ib></div>
+  <div class="block" style="width: 30%"><ib></ib></div>
+</div>
+
+<!-- calc() size: -->
+<div class="container">
+  <div class="table" style="width: calc(10px + 20%)"><ib></ib></div>
+  <div class="block" style="width: calc(10px + 20%)"><ib></ib></div>
+</div>
+
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/flexbox/flexbox-table-flex-items-2.html
@@ -0,0 +1,73 @@
+<!DOCTYPE html>
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>
+    CSS Test: Testing that implicit "flex-basis: content" on table wrapper box
+    doesn't prevent explicit table size from influencing flex base size.
+  </title>
+  <!-- XXXdholbert NOTE: This probably eventually should move to our
+       upstreamed reftest directory.  But for now, this is just asserting
+       backwards-compatible/interoperable (but not necessary spec-compliant)
+       behavior, per https://github.com/w3c/csswg-drafts/issues/2604 -->
+  <link rel="author" title="Daniel Holbert" href="mailto:dholbert@mozilla.com">
+  <link rel="help" href="https://www.w3.org/TR/css-flexbox-1/#flex-base-size">
+  <link rel="match" href="flexbox-table-flex-items-2.html">
+  <style>
+  .container {
+    display: flex;
+    width: 100px;
+    border: 1px solid black;
+  }
+
+  /* Two types of flex items: */
+  .table {
+    display: table;
+    border: 2px solid teal;
+  }
+  .block {
+    border: 2px solid brown;
+  }
+
+  /* Each flex item gets one of these as its contents,
+     to have a nonzero content size: */
+  ib {
+    display: inline-block;
+    background: blue;
+    border: 1px solid gray;
+    width: 15px;
+    height: 10px;
+  }
+  </style>
+</head>
+<body>
+<!-- auto size: -->
+<div class="container">
+  <div class="table"><ib></ib></div>
+  <div class="block"><ib></ib></div>
+</div>
+
+<!-- px size: -->
+<div class="container">
+  <div class="table" style="width: 30px"><ib></ib></div>
+  <div class="block" style="width: 30px"><ib></ib></div>
+</div>
+
+<!-- % size: -->
+<div class="container">
+  <div class="table" style="width: 30%"><ib></ib></div>
+  <div class="block" style="width: 30%"><ib></ib></div>
+</div>
+
+<!-- calc() size: -->
+<div class="container">
+  <div class="table" style="width: calc(10px + 20%)"><ib></ib></div>
+  <div class="block" style="width: calc(10px + 20%)"><ib></ib></div>
+</div>
+
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/flexbox/flexbox-table-flex-items-3-ref.html
@@ -0,0 +1,66 @@
+<!DOCTYPE html>
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>CSS Reftest Reference</title>
+  <link rel="author" title="Daniel Holbert" href="mailto:dholbert@mozilla.com">
+  <link rel="help" href="https://www.w3.org/TR/css-flexbox-1/#flex-base-size">
+  <style>
+  .container {
+    display: flex;
+    flex-direction: column;
+    height: 100px;
+    float: left;
+    border: 1px solid black;
+  }
+
+  /* Two types of flex items: */
+  .table {
+    border: 2px solid teal;
+  }
+  .block {
+    border: 2px solid brown;
+  }
+
+  /* Each flex item gets one of these as its contents,
+     to have a nonzero content size: */
+  ib {
+    display: inline-block;
+    background: blue;
+    border: 1px solid gray;
+    width: 15px;
+    height: 10px;
+  }
+  </style>
+</head>
+<body>
+<!-- auto size: -->
+<div class="container">
+  <div class="table"><ib></ib></div>
+  <div class="block"><ib></ib></div>
+</div>
+
+<!-- px size: -->
+<div class="container">
+  <div class="table" style="height: 30px"><ib></ib></div>
+  <div class="block" style="height: 30px"><ib></ib></div>
+</div>
+
+<!-- % size: -->
+<div class="container">
+  <div class="table" style="height: 30%"><ib></ib></div>
+  <div class="block" style="height: 30%"><ib></ib></div>
+</div>
+
+<!-- calc() size: -->
+<div class="container">
+  <div class="table" style="height: calc(10px + 20%)"><ib></ib></div>
+  <div class="block" style="height: calc(10px + 20%)"><ib></ib></div>
+</div>
+
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/flexbox/flexbox-table-flex-items-3.html
@@ -0,0 +1,75 @@
+<!DOCTYPE html>
+<!--
+     Any copyright is dedicated to the Public Domain.
+     http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>
+    CSS Test: Testing that implicit "flex-basis: content" on table wrapper box
+    doesn't prevent explicit table size from influencing flex base size.
+  </title>
+  <!-- XXXdholbert NOTE: This probably eventually should move to our
+       upstreamed reftest directory.  But for now, this is just asserting
+       backwards-compatible/interoperable (but not necessary spec-compliant)
+       behavior, per https://github.com/w3c/csswg-drafts/issues/2604 -->
+  <link rel="author" title="Daniel Holbert" href="mailto:dholbert@mozilla.com">
+  <link rel="help" href="https://www.w3.org/TR/css-flexbox-1/#flex-base-size">
+  <link rel="match" href="flexbox-table-flex-items-3.html">
+  <style>
+  .container {
+    display: flex;
+    flex-direction: column;
+    height: 100px;
+    float: left;
+    border: 1px solid black;
+  }
+
+  /* Two types of flex items: */
+  .table {
+    display: table;
+    border: 2px solid teal;
+  }
+  .block {
+    border: 2px solid brown;
+  }
+
+  /* Each flex item gets one of these as its contents,
+     to have a nonzero content size: */
+  ib {
+    display: inline-block;
+    background: blue;
+    border: 1px solid gray;
+    width: 15px;
+    height: 10px;
+  }
+  </style>
+</head>
+<body>
+<!-- auto size: -->
+<div class="container">
+  <div class="table"><ib></ib></div>
+  <div class="block"><ib></ib></div>
+</div>
+
+<!-- px size: -->
+<div class="container">
+  <div class="table" style="height: 30px"><ib></ib></div>
+  <div class="block" style="height: 30px"><ib></ib></div>
+</div>
+
+<!-- % size: -->
+<div class="container">
+  <div class="table" style="height: 30%"><ib></ib></div>
+  <div class="block" style="height: 30%"><ib></ib></div>
+</div>
+
+<!-- calc() size: -->
+<div class="container">
+  <div class="table" style="height: calc(10px + 20%)"><ib></ib></div>
+  <div class="block" style="height: calc(10px + 20%)"><ib></ib></div>
+</div>
+
+</body>
+</html>
--- a/layout/reftests/flexbox/reftest.list
+++ b/layout/reftests/flexbox/reftest.list
@@ -119,8 +119,10 @@ fuzzy-if(skiaContent,1,5) == flexbox-res
 # Tests with widgets as flex items
 fuzzy-if(gtkWidget,1,66) == flexbox-widget-flex-items-1.html flexbox-widget-flex-items-1-ref.html
 fuzzy-if(gtkWidget,1,74) == flexbox-widget-flex-items-2.html flexbox-widget-flex-items-2-ref.html
 skip-if(gtkWidget) == flexbox-widget-flex-items-3.html flexbox-widget-flex-items-3-ref.html # bug 1260965
 fuzzy-if(gtkWidget,1,31) == flexbox-widget-flex-items-4.html flexbox-widget-flex-items-4-ref.html
 
 # Tests for table flex items
 == flexbox-table-flex-items-1.html flexbox-table-flex-items-1-ref.html
+== flexbox-table-flex-items-2.html flexbox-table-flex-items-2-ref.html
+== flexbox-table-flex-items-3.html flexbox-table-flex-items-3-ref.html