Bug 798235: For flex items in a vertical flex container, treat enumerated values for flex-basis as "auto". r=dbaron
authorDaniel Holbert <dholbert@cs.stanford.edu>
Tue, 16 Oct 2012 19:04:29 -0700
changeset 110481 5c67076a5a05e625fa4f56845fbfce9b5159e5c2
parent 110480 e03f63abb826e42f90a43a5166dcdd961a83d79c
child 110482 a70b58fcf0597681afbe11dce919efbe1851e632
push id16574
push userdholbert@mozilla.com
push dateWed, 17 Oct 2012 02:05:08 +0000
treeherdermozilla-inbound@16adea14324f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs798235
milestone19.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 798235: For flex items in a vertical flex container, treat enumerated values for flex-basis as "auto". r=dbaron
layout/base/nsLayoutUtils.cpp
layout/generic/crashtests/798235-1.html
layout/generic/crashtests/crashtests.list
layout/generic/nsFrame.cpp
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -2859,21 +2859,33 @@ nsLayoutUtils::ComputeSizeWithIntrinsicD
     // "flex-basis:auto", in which case they use their main-size property after
     // all.
     uint32_t flexDirection =
       aFrame->GetParent()->GetStylePosition()->mFlexDirection;
     isHorizontalFlexItem =
       flexDirection == NS_STYLE_FLEX_DIRECTION_ROW ||
       flexDirection == NS_STYLE_FLEX_DIRECTION_ROW_REVERSE;
 
-    if (stylePos->mFlexBasis.GetUnit() != eStyleUnit_Auto) {
+    // NOTE: The logic here should match the similar chunk for determining
+    // widthStyleCoord and heightStyleCoord in nsFrame::ComputeSize().
+    const nsStyleCoord* flexBasis = &(stylePos->mFlexBasis);
+    if (flexBasis->GetUnit() != eStyleUnit_Auto) {
       if (isHorizontalFlexItem) {
-        widthStyleCoord = &(stylePos->mFlexBasis);
+        widthStyleCoord = flexBasis;
       } else {
-        heightStyleCoord = &(stylePos->mFlexBasis);
+        // One caveat for vertical flex items: We don't support enumerated
+        // values (e.g. "max-content") for height properties yet. So, if our
+        // computed flex-basis is an enumerated value, we'll just behave as if
+        // it were "auto", which means "use the main-size property after all"
+        // (which is "height", in this case).
+        // NOTE: Once we support intrinsic sizing keywords for "height",
+        // we should remove this check.
+        if (flexBasis->GetUnit() != eStyleUnit_Enumerated) {
+          heightStyleCoord = flexBasis;
+        }
       }
     }
   }
 #endif // MOZ_FLEXBOX
 
 
   // Handle intrinsic sizes and their interaction with
   // {min-,max-,}{width,height} according to the rules in
new file mode 100644
--- /dev/null
+++ b/layout/generic/crashtests/798235-1.html
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<html>
+<body>
+  <div style="flex-direction: column-reverse; display: inline-flex;">
+    <div style="flex: 1 1 -moz-max-content;"></div>
+  </div>
+</body>
+</html>
--- a/layout/generic/crashtests/crashtests.list
+++ b/layout/generic/crashtests/crashtests.list
@@ -389,8 +389,9 @@ load 718516.html
 load first-letter-638937.html
 load first-letter-638937-2.html
 test-pref(layout.css.flexbox.enabled,true) load 737313-1.html
 test-pref(layout.css.flexbox.enabled,true) load 737313-2.html
 test-pref(layout.css.flexbox.enabled,true) load 737313-3.html
 load 762764-1.html
 load 786740-1.html
 asserts(8) test-pref(layout.css.flexbox.enabled,true) load 798020-1.html
+test-pref(layout.css.flexbox.enabled,true) load 798235-1.html
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -3951,21 +3951,34 @@ nsFrame::ComputeSize(nsRenderingContext 
     // property (e.g. "width") for sizing purposes, *unless* they have
     // "flex-basis:auto", in which case they use their main-size property after
     // all.
     uint32_t flexDirection = mParent->GetStylePosition()->mFlexDirection;
     isHorizontalFlexItem =
       flexDirection == NS_STYLE_FLEX_DIRECTION_ROW ||
       flexDirection == NS_STYLE_FLEX_DIRECTION_ROW_REVERSE;
 
-    if (stylePos->mFlexBasis.GetUnit() != eStyleUnit_Auto) {
+    // NOTE: The logic here should match the similar chunk for determining
+    // widthStyleCoord and heightStyleCoord in
+    // nsLayoutUtils::ComputeSizeWithIntrinsicDimensions().
+    const nsStyleCoord* flexBasis = &(stylePos->mFlexBasis);
+    if (flexBasis->GetUnit() != eStyleUnit_Auto) {
       if (isHorizontalFlexItem) {
-        widthStyleCoord = &(stylePos->mFlexBasis);
+        widthStyleCoord = flexBasis;
       } else {
-        heightStyleCoord = &(stylePos->mFlexBasis);
+        // One caveat for vertical flex items: We don't support enumerated
+        // values (e.g. "max-content") for height properties yet. So, if our
+        // computed flex-basis is an enumerated value, we'll just behave as if
+        // it were "auto", which means "use the main-size property after all"
+        // (which is "height", in this case).
+        // NOTE: Once we support intrinsic sizing keywords for "height",
+        // we should remove this check.
+        if (flexBasis->GetUnit() != eStyleUnit_Enumerated) {
+          heightStyleCoord = flexBasis;
+        }
       }
     }
   }
 #endif // MOZ_FLEXBOX
 
   // Compute width
 
   if (widthStyleCoord->GetUnit() != eStyleUnit_Auto) {