Bug 785754. Fix handling of max-height for frame classes that still seem to think that the mComputedMinHeight/mComputedMaxHeight of a reflow state are border-box heights, not content-box heights. r=mats
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 27 Aug 2012 15:46:23 -0400
changeset 105636 721ff7492145e230ba54431548dc8afb7865c1c1
parent 105635 5ff334b7781d869e80740bc19f7dabaff1ba609e
child 105637 72755799451cc5e8d79dfc0e1c430634f19df023
push id55
push usershu@rfrn.org
push dateThu, 30 Aug 2012 01:33:09 +0000
reviewersmats
bugs785754
milestone17.0a1
Bug 785754. Fix handling of max-height for frame classes that still seem to think that the mComputedMinHeight/mComputedMaxHeight of a reflow state are border-box heights, not content-box heights. r=mats
layout/forms/nsHTMLButtonControlFrame.cpp
layout/forms/nsMeterFrame.cpp
layout/forms/nsProgressFrame.cpp
layout/reftests/forms/button-max-height-ref.html
layout/reftests/forms/button-max-height.html
layout/reftests/forms/meter/max-height-ref.html
layout/reftests/forms/meter/max-height.html
layout/reftests/forms/meter/reftest.list
layout/reftests/forms/progress/max-height-ref.html
layout/reftests/forms/progress/max-height.html
layout/reftests/forms/progress/reftest.list
layout/reftests/forms/reftest.list
layout/xul/base/src/nsLeafBoxFrame.cpp
--- a/layout/forms/nsHTMLButtonControlFrame.cpp
+++ b/layout/forms/nsHTMLButtonControlFrame.cpp
@@ -221,33 +221,34 @@ nsHTMLButtonControlFrame::Reflow(nsPresC
   
   // Reflow the contents of the button.
   ReflowButtonContents(aPresContext, aDesiredSize, aReflowState, firstKid,
                        focusPadding, aStatus);
 
   aDesiredSize.width = aReflowState.ComputedWidth();
 
   // If computed use the computed value.
-  if (aReflowState.ComputedHeight() != NS_INTRINSICSIZE) 
+  if (aReflowState.ComputedHeight() != NS_INTRINSICSIZE) {
     aDesiredSize.height = aReflowState.ComputedHeight();
-  else
+  } else {
     aDesiredSize.height += focusPadding.TopBottom();
-  
+
+    // Make sure we obey min/max-height in the case when we're doing intrinsic
+    // sizing (we get it for free when we have a non-intrinsic
+    // aReflowState.ComputedHeight()).  Note that we do this before adjusting
+    // for borderpadding, since mComputedMaxHeight and mComputedMinHeight are
+    // content heights.
+    aDesiredSize.height = NS_CSS_MINMAX(aDesiredSize.height,
+                                        aReflowState.mComputedMinHeight,
+                                        aReflowState.mComputedMaxHeight);
+  }
+
   aDesiredSize.width += aReflowState.mComputedBorderPadding.LeftRight();
   aDesiredSize.height += aReflowState.mComputedBorderPadding.TopBottom();
 
-  // Make sure we obey min/max-height.  Note that we do this after adjusting
-  // for borderpadding, since buttons have border-box sizing...
-
-  // XXXbz unless someone overrides that, of course!  We should really consider
-  // exposing nsHTMLReflowState::AdjustComputed* or something.
-  aDesiredSize.height = NS_CSS_MINMAX(aDesiredSize.height,
-                                      aReflowState.mComputedMinHeight,
-                                      aReflowState.mComputedMaxHeight);
-
   aDesiredSize.ascent +=
     aReflowState.mComputedBorderPadding.top + focusPadding.top;
 
   aDesiredSize.SetOverflowAreasToDesiredBounds();
   ConsiderChildOverflow(aDesiredSize.mOverflowAreas, firstKid);
   FinishReflowWithAbsoluteFrames(aPresContext, aDesiredSize, aReflowState, aStatus);
 
   aStatus = NS_FRAME_COMPLETE;
--- a/layout/forms/nsMeterFrame.cpp
+++ b/layout/forms/nsMeterFrame.cpp
@@ -116,19 +116,16 @@ NS_IMETHODIMP nsMeterFrame::Reflow(nsPre
   NS_ASSERTION(barFrame, "The meter frame should have a child with a frame!");
 
   ReflowBarFrame(barFrame, aPresContext, aReflowState, aStatus);
 
   aDesiredSize.width = aReflowState.ComputedWidth() +
                        aReflowState.mComputedBorderPadding.LeftRight();
   aDesiredSize.height = aReflowState.ComputedHeight() +
                         aReflowState.mComputedBorderPadding.TopBottom();
-  aDesiredSize.height = NS_CSS_MINMAX(aDesiredSize.height,
-                                      aReflowState.mComputedMinHeight,
-                                      aReflowState.mComputedMaxHeight);
 
   aDesiredSize.SetOverflowAreasToDesiredBounds();
   ConsiderChildOverflow(aDesiredSize.mOverflowAreas, barFrame);
   FinishAndStoreOverflow(&aDesiredSize);
 
   aStatus = NS_FRAME_COMPLETE;
 
   NS_FRAME_SET_TRUNCATION(aStatus, aReflowState, aDesiredSize);
--- a/layout/forms/nsProgressFrame.cpp
+++ b/layout/forms/nsProgressFrame.cpp
@@ -124,19 +124,16 @@ NS_IMETHODIMP nsProgressFrame::Reflow(ns
   NS_ASSERTION(barFrame, "The progress frame should have a child with a frame!");
 
   ReflowBarFrame(barFrame, aPresContext, aReflowState, aStatus);
 
   aDesiredSize.width = aReflowState.ComputedWidth() +
                        aReflowState.mComputedBorderPadding.LeftRight();
   aDesiredSize.height = aReflowState.ComputedHeight() +
                         aReflowState.mComputedBorderPadding.TopBottom();
-  aDesiredSize.height = NS_CSS_MINMAX(aDesiredSize.height,
-                                      aReflowState.mComputedMinHeight,
-                                      aReflowState.mComputedMaxHeight);
 
   aDesiredSize.SetOverflowAreasToDesiredBounds();
   ConsiderChildOverflow(aDesiredSize.mOverflowAreas, barFrame);
   FinishAndStoreOverflow(&aDesiredSize);
 
   aStatus = NS_FRAME_COMPLETE;
 
   NS_FRAME_SET_TRUNCATION(aStatus, aReflowState, aDesiredSize);
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/button-max-height-ref.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<style>
+  button {
+    height: 50px;
+    padding: 23px;
+    border: 1px solid black;
+    box-sizing: border-box;
+    -moz-box-sizing: border-box;
+    overflow: hidden;
+  }
+</style>
+<button>Some text</button>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/button-max-height.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<style>
+  button {
+    max-height: 50px;
+    padding: 23px;
+    border: 1px solid black;
+    box-sizing: border-box;
+    -moz-box-sizing: border-box;
+    overflow: hidden;
+  }
+</style>
+<button>Some text</button>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/meter/max-height-ref.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<style>
+  meter {
+    height: 50px;
+    padding: 23px;
+    border: 1px solid black;
+    box-sizing: border-box;
+    -moz-box-sizing: border-box;
+    overflow: hidden;
+  }
+</style>
+<meter>Some text</meter>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/meter/max-height.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<style>
+  meter {
+    max-height: 50px;
+    padding: 23px;
+    border: 1px solid black;
+    box-sizing: border-box;
+    -moz-box-sizing: border-box;
+    overflow: hidden;
+  }
+</style>
+<meter>Some text</meter>
--- a/layout/reftests/forms/meter/reftest.list
+++ b/layout/reftests/forms/meter/reftest.list
@@ -19,8 +19,9 @@
 # == transformations.html transformations-ref.html
 
 # default style
 include default-style/reftest.list
 
 # Tests for bugs:
 == block-invalidate.html block-invalidate-ref.html
 == in-cells.html in-cells-ref.html
+== max-height.html max-height-ref.html
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/progress/max-height-ref.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<style>
+  progress {
+    height: 50px;
+    padding: 23px;
+    border: 1px solid black;
+    box-sizing: border-box;
+    -moz-box-sizing: border-box;
+    overflow: hidden;
+  }
+</style>
+<progress>Some text</progress>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/progress/max-height.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<style>
+  progress {
+    max-height: 50px;
+    padding: 23px;
+    border: 1px solid black;
+    box-sizing: border-box;
+    -moz-box-sizing: border-box;
+    overflow: hidden;
+  }
+</style>
+<progress>Some text</progress>
--- a/layout/reftests/forms/progress/reftest.list
+++ b/layout/reftests/forms/progress/reftest.list
@@ -18,8 +18,9 @@
 # The following test is disabled but kept in the repository because the
 # transformations will not behave exactly the same for <progress> and two divs.
 # However, it would be possible to manually check those.
 # == transformations.html transformations-ref.html
 
 # Tests for bugs:
 == block-invalidate.html block-invalidate-ref.html
 == in-cells.html in-cells-ref.html
+== max-height.html max-height-ref.html
--- a/layout/reftests/forms/reftest.list
+++ b/layout/reftests/forms/reftest.list
@@ -58,22 +58,26 @@ fails-if(Android) != textarea-rtl.html t
 != radio-checked-native-notref.html about:blank
 
 == select-multiple.html select-multiple-ref.html
 == select-boguskids.html select-boguskids-ref.html
 == select-dynamic-boguskids.html select-boguskids-ref.html
 
 asserts(2) == button-first-letter-1.html button-first-letter-1-ref.html
 asserts(1) != button-first-letter-1.html button-first-letter-1-noref.html
+== button-max-height.html button-max-height-ref.html
 
 == legend.html legend-ref.html
 
 # placeholder
 include placeholder/reftest.list
 
 # input
 include input/reftest.list
 
+# meter element
+include meter/reftest.list
+
 # output element
 include output/reftest.list
 
 # progress element
 include progress/reftest.list
--- a/layout/xul/base/src/nsLeafBoxFrame.cpp
+++ b/layout/xul/base/src/nsLeafBoxFrame.cpp
@@ -264,28 +264,34 @@ nsLeafBoxFrame::Reflow(nsPresContext*   
 
   if (aReflowState.ComputedHeight() == NS_INTRINSICSIZE) {
     computedSize.height = prefSize.height;
   } else {
     computedSize.height += m.top + m.bottom;
   }
 
   // handle reflow state min and max sizes
-
+  // XXXbz the width handling here seems to be wrong, since
+  // mComputedMin/MaxWidth is a content-box size, whole
+  // computedSize.width is a border-box size...
   if (computedSize.width > aReflowState.mComputedMaxWidth)
     computedSize.width = aReflowState.mComputedMaxWidth;
 
-  if (computedSize.height > aReflowState.mComputedMaxHeight)
-    computedSize.height = aReflowState.mComputedMaxHeight;
-
   if (computedSize.width < aReflowState.mComputedMinWidth)
     computedSize.width = aReflowState.mComputedMinWidth;
 
-  if (computedSize.height < aReflowState.mComputedMinHeight)
-    computedSize.height = aReflowState.mComputedMinHeight;
+  // Now adjust computedSize.height for our min and max computed
+  // height.  The only problem is that those are content-box sizes,
+  // while computedSize.height is a border-box size.  So subtract off
+  // m.TopBottom() before adjusting, then readd it.
+  computedSize.height = NS_MAX(0, computedSize.height - m.TopBottom());
+  computedSize.height = NS_CSS_MINMAX(computedSize.height,
+                                      aReflowState.mComputedMinHeight,
+                                      aReflowState.mComputedMaxHeight);
+  computedSize.height += m.TopBottom();
 
   nsRect r(mRect.x, mRect.y, computedSize.width, computedSize.height);
 
   SetBounds(state, r);
  
   // layout our children
   Layout(state);