Bug 1374540 part 4: Change nsFrame::ComputeSize to treat used flex-basis:content as 'max-content'. r?mats draft
authorDaniel Holbert <dholbert@cs.stanford.edu>
Fri, 13 Apr 2018 12:17:51 -0700
changeset 781927 55dd4eaee9eaf051ae1e72878cdb0220b563554a
parent 781926 ec54ec22e70fe0d4d8a615bf571c4d1111329fc1
push id106442
push userdholbert@mozilla.com
push dateFri, 13 Apr 2018 19:18:31 +0000
reviewersmats
bugs1374540
milestone61.0a1
Bug 1374540 part 4: Change nsFrame::ComputeSize to treat used flex-basis:content as 'max-content'. r?mats This brings us into alignment with the spec and makes us pass some web-platform tests, along with the reftests that I've included for this bug. MozReview-Commit-ID: KoKPi18svGE
layout/generic/nsFrame.cpp
layout/reftests/w3c-css/submitted/flexbox/reftest.list
testing/web-platform/meta/css/css-flexbox/flexbox_flex-formatting-interop.html.ini
testing/web-platform/meta/css/css-flexbox/flexbox_flex-none-wrappable-content.html.ini
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -5639,27 +5639,29 @@ nsFrame::ComputeSize(gfxContext*        
   if (isFlexItem) {
     // Flex items use their "flex-basis" property in place of their main-size
     // property for sizing purposes, *unless* they have "flex-basis:auto", in
     // which case they use their main-size property after all.
     flexMainAxis = nsFlexContainerFrame::IsItemInlineAxisMainAxis(this) ?
       eLogicalAxisInline : eLogicalAxisBlock;
 
     // NOTE: The logic here should match the similar chunk for updating
-    // mainAxisCoord in nsFrame::ComputeSizeWithIntrinsicDimensions().
+    // 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);
 
     if (nsFlexContainerFrame::IsUsedFlexBasisContent(flexBasis,
                                                      mainAxisCoord)) {
-      // XXXdholbert Technically we shouldn't be using 'auto' here. A later
-      // patch will convert this to max-content instead.
-      static const nsStyleCoord autoStyleCoord(eStyleUnit_Auto);
-      mainAxisCoord = &autoStyleCoord;
+      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.
       mainAxisCoord = flexBasis;
     } // else: flex-basis is 'auto', which is deferring to some explicit value
       // in mainAxisCoord. So we proceed w/o touching mainAxisCoord.
   }
 
@@ -5891,17 +5893,18 @@ nsFrame::ComputeSizeWithIntrinsicDimensi
       }
 
     } else {
       // Flex items use their "flex-basis" property in place of their main-size
       // property (e.g. "width") for sizing purposes, *unless* they have
       // "flex-basis:auto", in which case they use their main-size property
       // after all.
       // NOTE: The logic here should match the similar chunk for updating
-      // mainAxisCoord in nsFrame::ComputeSize().
+      // mainAxisCoord in nsFrame::ComputeSize() (aside from using a different
+      // dummy value in the IsUsedFlexBasisContent() case).
       const nsStyleCoord* flexBasis = &(stylePos->mFlexBasis);
       auto& mainAxisCoord = (flexMainAxis == eLogicalAxisInline
                              ? inlineStyleCoord : blockStyleCoord);
 
       if (nsFlexContainerFrame::IsUsedFlexBasisContent(flexBasis,
                                                        mainAxisCoord)) {
         // If we get here, we're resolving the flex base size for a flex item,
         // and we fall into the flexbox spec section 9.2 step 3, substep C (if
--- a/layout/reftests/w3c-css/submitted/flexbox/reftest.list
+++ b/layout/reftests/w3c-css/submitted/flexbox/reftest.list
@@ -98,18 +98,18 @@ fuzzy-if(Android,158,32) == flexbox-alig
 == flexbox-collapsed-item-horiz-002.html flexbox-collapsed-item-horiz-002-ref.html
 == flexbox-collapsed-item-horiz-003.html flexbox-collapsed-item-horiz-003-ref.html
 
 # Tests for "flex-basis: content"
 == flexbox-flex-basis-content-001a.html flexbox-flex-basis-content-001-ref.html
 == flexbox-flex-basis-content-001b.html flexbox-flex-basis-content-001-ref.html
 == flexbox-flex-basis-content-002a.html flexbox-flex-basis-content-002-ref.html
 == flexbox-flex-basis-content-002b.html flexbox-flex-basis-content-002-ref.html
-fails == flexbox-flex-basis-content-003a.html flexbox-flex-basis-content-003-ref.html # bug 1374540
-fails == flexbox-flex-basis-content-003b.html flexbox-flex-basis-content-003-ref.html # bug 1374540
+== flexbox-flex-basis-content-003a.html flexbox-flex-basis-content-003-ref.html
+== flexbox-flex-basis-content-003b.html flexbox-flex-basis-content-003-ref.html
 == flexbox-flex-basis-content-004a.html flexbox-flex-basis-content-004-ref.html
 == flexbox-flex-basis-content-004b.html flexbox-flex-basis-content-004-ref.html
 
 # Tests for flex-flow shorthand property
 == flexbox-flex-flow-001.html flexbox-flex-flow-001-ref.html
 == flexbox-flex-flow-002.html flexbox-flex-flow-002-ref.html
 
 # Tests for flex-wrap property
deleted file mode 100644
--- a/testing/web-platform/meta/css/css-flexbox/flexbox_flex-formatting-interop.html.ini
+++ /dev/null
@@ -1,2 +0,0 @@
-[flexbox_flex-formatting-interop.html]
-  expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/css/css-flexbox/flexbox_flex-none-wrappable-content.html.ini
+++ /dev/null
@@ -1,2 +0,0 @@
-[flexbox_flex-none-wrappable-content.html]
-  expected: FAIL