Bug 405952. Add scrollbar width to pref and min widths for overflow:scroll elements but for no other overflow values. r+sr=bzbarsky,dbaron,dholbert, a=beltzner

layout/forms/nsListControlFrame.cpp

@@ -525,14 +525,31 @@ nsListControlFrame::CalcHeightOfARow()
   return heightOfARow;
 }
 
+nscoord
+nsListControlFrame::GetPrefWidth(nsIRenderingContext *aRenderingContext)
+{
+  nscoord result;
+  DISPLAY_PREF_WIDTH(this, result);
+
+  // Always add scrollbar widths to the pref-width of the scrolled
+  // content. Combobox frames depend on this happening in the dropdown,
+  // and standalone listboxes are overflow:scroll so they need it too.
+  result = GetScrolledFrame()->GetPrefWidth(aRenderingContext);
+  nsBoxLayoutState bls(PresContext(), aRenderingContext);
+  result = NSCoordSaturatingAdd(result, GetDesiredScrollbarSizes(&bls).LeftRight());
+
+  return result;
+}
+
 nscoord
 nsListControlFrame::GetMinWidth(nsIRenderingContext *aRenderingContext)
 {
-  // Scrollframes typically have an intrinsic min width of 0, but
-  // that's not how we want to behave.
   nscoord result;
   DISPLAY_MIN_WIDTH(this, result);
 
+  // Always add scrollbar widths to the min-width of the scrolled
+  // content. Combobox frames depend on this happening in the dropdown,
+  // and standalone listboxes are overflow:scroll so they need it too.
   result = GetScrolledFrame()->GetMinWidth(aRenderingContext);
   nsBoxLayoutState bls(PresContext(), aRenderingContext);
   result += GetDesiredScrollbarSizes(&bls).LeftRight();

layout/forms/nsListControlFrame.h

@@ -91,7 +91,7 @@ public:
   NS_IMETHOD SetInitialChildList(nsIAtom*        aListName,
                                  nsIFrame*       aChildList);
 
-  // Our min width is our pref width
+  virtual nscoord GetPrefWidth(nsIRenderingContext *aRenderingContext);
   virtual nscoord GetMinWidth(nsIRenderingContext *aRenderingContext);
 
   NS_IMETHOD Reflow(nsPresContext*          aCX,

layout/generic/nsGfxScrollFrame.cpp

@@ -699,32 +699,34 @@ nsHTMLScrollFrame::PlaceScrollArea(const ScrollReflowState& aState)
                                              NS_FRAME_NO_MOVE_VIEW);
 }
 
+nscoord
+nsHTMLScrollFrame::GetIntrinsicVScrollbarWidth(nsIRenderingContext *aRenderingContext)
+{
+  nsGfxScrollFrameInner::ScrollbarStyles ss = GetScrollbarStyles();
+  if (ss.mVertical != NS_STYLE_OVERFLOW_SCROLL || !mInner.mVScrollbarBox)
+    return 0;
+
+  nsBoxLayoutState bls(PresContext(), aRenderingContext);
+  nsSize vScrollbarPrefSize(0, 0);
+  GetScrollbarMetrics(bls, mInner.mVScrollbarBox,
+                      nsnull, &vScrollbarPrefSize, PR_TRUE);
+  return vScrollbarPrefSize.width;
+}
+
 /* virtual */ nscoord
 nsHTMLScrollFrame::GetMinWidth(nsIRenderingContext *aRenderingContext)
 {
   nscoord result = mInner.mScrolledFrame->GetMinWidth(aRenderingContext);
   DISPLAY_MIN_WIDTH(this, result);
-  return result;
+  return result + GetIntrinsicVScrollbarWidth(aRenderingContext);
 }
 
 /* virtual */ nscoord
 nsHTMLScrollFrame::GetPrefWidth(nsIRenderingContext *aRenderingContext)
 {
-  nscoord result;
+  nscoord result = mInner.mScrolledFrame->GetPrefWidth(aRenderingContext);
   DISPLAY_PREF_WIDTH(this, result);
-  result = mInner.mScrolledFrame->GetPrefWidth(aRenderingContext);
-
-  nsGfxScrollFrameInner::ScrollbarStyles ss = GetScrollbarStyles();
-  if (ss.mVertical != NS_STYLE_OVERFLOW_HIDDEN && // ideal?
-      mInner.mVScrollbarBox) {
-    nsBoxLayoutState bls(PresContext(), aRenderingContext);
-    nsSize vScrollbarPrefSize(0, 0);
-    GetScrollbarMetrics(bls, mInner.mVScrollbarBox,
-                        nsnull, &vScrollbarPrefSize, PR_TRUE);
-    result = NSCoordSaturatingAdd(result, vScrollbarPrefSize.width);
-  }
-
-  return result;
+  return NSCoordSaturatingAdd(result, GetIntrinsicVScrollbarWidth(aRenderingContext));
 }
 
 NS_IMETHODIMP

layout/generic/nsGfxScrollFrame.h

@@ -275,6 +275,7 @@ public:
   nsresult ReflowContents(ScrollReflowState* aState,
                           const nsHTMLReflowMetrics& aDesiredSize);
   void PlaceScrollArea(const ScrollReflowState& aState);
+  nscoord GetIntrinsicVScrollbarWidth(nsIRenderingContext *aRenderingContext);
 
   virtual nscoord GetMinWidth(nsIRenderingContext *aRenderingContext);
   virtual nscoord GetPrefWidth(nsIRenderingContext *aRenderingContext);

layout/reftests/bugs/405952-1-ref.html

@@ -0,0 +1,42 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<style>
+.container { margin-top:1em; background:yellow; width:100px; }
+.content { width:100px; height:60px; }
+.scroll-in { overflow:scroll; }
+.scroll { overflow-x:scroll; width:200px; }
+.over { height:50px; }
+body { margin:0; position:absolute; clip: rect(0 100px auto 0); }
+</style>
+</head>
+<body>
+
+<div class="container">
+  <div class="content"></div>
+</div>
+<div class="container scroll">
+  <div class="content"></div>
+</div>
+<div class="container scroll-in over">
+  <div class="content"></div>
+</div>
+<div class="container scroll over">
+  <div class="content"></div>
+</div>
+
+<div class="container">
+  <div class="content"></div>
+</div>
+<div class="container scroll">
+  <div class="content"></div>
+</div>
+<div class="container scroll-in over">
+  <div class="content"></div>
+</div>
+<div class="container scroll over">
+  <div class="content"></div>
+</div>
+
+</body>
+</html>

layout/reftests/bugs/405952-1.html

@@ -0,0 +1,48 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<style>
+.container { float:left; margin-top:1em; clear:both; }
+.auto { overflow:auto; }
+.scroll { overflow:scroll; }
+.content { width:100px; height:60px; background:yellow; }
+.over { height:50px; }
+body { margin:0; position:absolute; clip: rect(0 100px auto 0); }
+</style>
+</head>
+<body>
+
+<!-- test pref-width -->
+
+<div class="container auto">
+  <div class="content"></div>
+</div>
+<div class="container scroll">
+  <div class="content"></div>
+</div>
+<div class="container auto over">
+  <div class="content"></div>
+</div>
+<div class="container scroll over">
+  <div class="content"></div>
+</div>
+
+<!-- test min-width -->
+
+<div style="width:0">
+<div class="container auto">
+  <div class="content"></div>
+</div>
+<div class="container scroll">
+  <div class="content"></div>
+</div>
+<div class="container auto over">
+  <div class="content"></div>
+</div>
+<div class="container scroll over">
+  <div class="content"></div>
+</div>
+</div>
+
+</body>
+</html>

layout/reftests/bugs/reftest.list

@@ -682,6 +682,7 @@ random == 403134-1.html 403134-1-ref.html # bug 405377
 == 405517-1.xhtml 405517-1-ref.xhtml
 == 405577-1.html 405577-1-ref.html
 == 405584-1.html 405584-1-ref.html
+== 405952-1.html 405952-1-ref.html
 == 406073-1.html 406073-1-ref.html
 == 406484-1.html 406484-1-ref.html
 == 406568-1.html 406568-1-ref.html