Bug 364680. Intrinsic widths for columns. r+sr=dbaron
authorroc+@cs.cmu.edu
Thu, 19 Jul 2007 19:21:43 -0700
changeset 3696 b62764a194b0cedd4b5fc12dc6fa8b8c3bf76810
parent 3695 2fd7373869f664d4756193e8f4c4726334fc92e4
child 3697 28e39841249efda56e22498d0a56ff91efa096de
push idunknown
push userunknown
push dateunknown
bugs364680
milestone1.9a7pre
Bug 364680. Intrinsic widths for columns. r+sr=dbaron
layout/generic/nsColumnSetFrame.cpp
layout/reftests/box-properties/reftest.list
layout/reftests/columns/basic-1.html
layout/reftests/columns/basic-ref.html
layout/reftests/columns/min-width-1-ref.html
layout/reftests/columns/min-width-1a.html
layout/reftests/columns/min-width-1b.html
layout/reftests/columns/min-width-1c.html
layout/reftests/columns/pref-width-1-ref.html
layout/reftests/columns/pref-width-1a.html
layout/reftests/columns/pref-width-1b.html
layout/reftests/columns/pref-width-1c.html
layout/reftests/columns/reftest.list
layout/reftests/reftest.list
layout/style/nsCSSParser.cpp
layout/style/nsComputedDOMStyle.cpp
layout/style/nsComputedDOMStyle.h
layout/style/nsStyleStruct.h
layout/style/test/property_database.js
layout/style/test/test_bug365932.html
--- a/layout/generic/nsColumnSetFrame.cpp
+++ b/layout/generic/nsColumnSetFrame.cpp
@@ -64,18 +64,18 @@ public:
   NS_IMETHOD  AppendFrames(nsIAtom*        aListName,
                            nsIFrame*       aFrameList);
   NS_IMETHOD  InsertFrames(nsIAtom*        aListName,
                            nsIFrame*       aPrevFrame,
                            nsIFrame*       aFrameList);
   NS_IMETHOD  RemoveFrame(nsIAtom*        aListName,
                           nsIFrame*       aOldFrame);
 
-  // REVIEW: Now by default the background of a frame receives events,
-  // so this GetFrameForPoint override is no longer necessary.
+  virtual nscoord GetMinWidth(nsIRenderingContext *aRenderingContext);  
+  virtual nscoord GetPrefWidth(nsIRenderingContext *aRenderingContext);
 
   virtual nsIFrame* GetContentInsertionFrame() {
     return GetFirstChild(nsnull)->GetContentInsertionFrame();
   }
   
   NS_IMETHOD BuildDisplayList(nsDisplayListBuilder*   aBuilder,
                               const nsRect&           aDirtyRect,
                               const nsDisplayListSet& aLists);
@@ -191,47 +191,43 @@ static nscoord GetAvailableContentHeight
     return NS_INTRINSICSIZE;
   }
   nscoord borderPaddingHeight =
     aReflowState.mComputedBorderPadding.top +
     aReflowState.mComputedBorderPadding.bottom;
   return PR_MAX(0, aReflowState.availableHeight - borderPaddingHeight);
 }
 
+static nscoord
+GetColumnGap(nsColumnSetFrame* aFrame, const nsStyleColumn* aColStyle) {
+  switch (aColStyle->mColumnGap.GetUnit()) {
+    case eStyleUnit_Coord:
+      return aColStyle->mColumnGap.GetCoordValue();
+    case eStyleUnit_Normal:
+      return aFrame->GetStyleFont()->mFont.size;
+    default:
+      NS_NOTREACHED("Unknown gap type");
+  }
+  return 0;
+}
+
 nsColumnSetFrame::ReflowConfig
 nsColumnSetFrame::ChooseColumnStrategy(const nsHTMLReflowState& aReflowState)
 {
   const nsStyleColumn* colStyle = GetStyleColumn();
   nscoord availContentWidth = GetAvailableContentWidth(aReflowState);
   if (aReflowState.ComputedWidth() != NS_INTRINSICSIZE) {
     availContentWidth = aReflowState.ComputedWidth();
   }
   nscoord colHeight = GetAvailableContentHeight(aReflowState);
   if (aReflowState.mComputedHeight != NS_INTRINSICSIZE) {
     colHeight = aReflowState.mComputedHeight;
   }
 
-  nscoord colGap = 0;
-  switch (colStyle->mColumnGap.GetUnit()) {
-    case eStyleUnit_Coord:
-      colGap = colStyle->mColumnGap.GetCoordValue();
-      break;
-    case eStyleUnit_Percent:
-      if (availContentWidth != NS_INTRINSICSIZE) {
-        colGap = NSToCoordRound(colStyle->mColumnGap.GetPercentValue()*availContentWidth);
-      }
-      break;
-    case eStyleUnit_Normal:
-      colGap = GetStyleFont()->mFont.size;
-      break;
-    default:
-      NS_NOTREACHED("Unknown gap type");
-      break;
-  }
-
+  nscoord colGap = GetColumnGap(this, colStyle);
   PRInt32 numColumns = colStyle->mColumnCount;
 
   nscoord colWidth = NS_INTRINSICSIZE;
   if (colStyle->mColumnWidth.GetUnit() == eStyleUnit_Coord) {
     colWidth = colStyle->mColumnWidth.GetCoordValue();
 
     // Reduce column count if necessary to make columns fit in the
     // available width. Compute max number of columns that fit in
@@ -321,16 +317,68 @@ static void MoveChildTo(nsIFrame* aParen
   aParent->Invalidate(r);
   r -= aChild->GetPosition();
   aChild->SetPosition(aOrigin);
   r += aOrigin;
   aParent->Invalidate(r);
   PlaceFrameView(aChild);
 }
 
+nscoord
+nsColumnSetFrame::GetMinWidth(nsIRenderingContext *aRenderingContext) {
+  nscoord width = 0;
+  if (mFrames.FirstChild()) {
+    width = mFrames.FirstChild()->GetMinWidth(aRenderingContext);
+  }
+  const nsStyleColumn* colStyle = GetStyleColumn();
+  if (colStyle->mColumnWidth.GetUnit() == eStyleUnit_Coord) {
+    // As available width reduces to zero, we reduce our number of columns to one,
+    // and don't enforce the column width, so just return the min of the
+    // child's min-width with any specified column width.
+    width = PR_MIN(width, colStyle->mColumnWidth.GetCoordValue());
+  } else {
+    NS_ASSERTION(colStyle->mColumnCount > 0, "column-count and column-width can't both be auto");
+    // As available width reduces to zero, we still have mColumnCount columns, so
+    // multiply the child's min-width by the number of columns.
+    width *= colStyle->mColumnCount;
+  }
+  // XXX count forced column breaks here? Maybe we should return the child's
+  // min-width times the minimum number of columns.
+  return width;
+}
+
+nscoord
+nsColumnSetFrame::GetPrefWidth(nsIRenderingContext *aRenderingContext) {
+  // Our preferred width is our desired column width, if specified, otherwise the
+  // child's preferred width, times the number of columns, plus the width of any
+  // required column gaps
+  // XXX what about forced column breaks here?
+  const nsStyleColumn* colStyle = GetStyleColumn();
+  nscoord colGap = GetColumnGap(this, colStyle);
+
+  nscoord colWidth;
+  if (colStyle->mColumnWidth.GetUnit() == eStyleUnit_Coord) {
+    colWidth = colStyle->mColumnWidth.GetCoordValue();
+  } else {
+    if (mFrames.FirstChild()) {
+      colWidth = mFrames.FirstChild()->GetPrefWidth(aRenderingContext);
+    } else {
+      colWidth = 0;
+    }
+  }
+
+  PRInt32 numColumns = colStyle->mColumnCount;
+   if (numColumns <= 0) {
+    // if column-count is auto, assume one column
+    numColumns = 1;
+  }
+
+  return colWidth*numColumns + colGap*(numColumns - 1);
+}
+
 PRBool
 nsColumnSetFrame::ReflowChildren(nsHTMLReflowMetrics&     aDesiredSize,
                                  const nsHTMLReflowState& aReflowState,
                                  nsReflowStatus&          aStatus,
                                  const ReflowConfig&      aConfig,
                                  PRBool                   aUnboundedLastColumn,
                                  nsCollapsingMargin*      aBottomMarginCarriedOut) {
   PRBool allFit = PR_TRUE;
--- a/layout/reftests/box-properties/reftest.list
+++ b/layout/reftests/box-properties/reftest.list
@@ -1,10 +1,8 @@
-== column-gap-percent-1.html column-gap-percent-1-ref.html
-!= column-gap-percent-2.html column-gap-percent-2-ref.html
 == outline-radius-percent-1.html outline-radius-percent-1-ref.html
 == min-width-1.html min-width-1-ref.html
 == min-height-1.html min-height-1-ref.html
 == max-width-1.html max-width-1-ref.html
 == max-height-1.html max-height-1-ref.html
 == width-special-values-block.html width-special-values-block-ref.html
 == width-special-values-float.html width-special-values-block-ref.html
 == width-special-values-image-block.html width-special-values-image-block-ref.html
new file mode 100644
--- /dev/null
+++ b/layout/reftests/columns/basic-1.html
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <style>
+  div { -moz-column-count:2; -moz-column-gap:0; }
+  </style>
+</head>
+<body>
+  <div>
+    Hello<br>
+    Kitty
+  </div>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/columns/basic-ref.html
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <style>
+  table { border-spacing:0 }
+  td { vertical-align: baseline; padding:0; }
+  </style>
+</head>
+<body>
+  <table border="0" width="100%">
+    <tr>
+      <td width="50%">Hello</td>
+      <td width="50%">Kitty</td>
+    </tr>
+  </table>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/columns/min-width-1-ref.html
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <style>
+  div { border:2px solid black; width:100px; height:200px; }
+  </style>
+</head>
+<body>
+  <table width="1"><tr><td><div></div></tr></td></table>
+  <table width="1"><tr><td><div></div></tr></td></table>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/columns/min-width-1a.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <style>
+  div { -moz-column-count:2; -moz-column-gap:0; border:2px solid black; height:200px; }
+  div.gap { -moz-column-gap:80px; }
+  span { display:inline-block; width:50px; }
+  </style>
+</head>
+<body>
+  <table width="1"><tr><td><div>
+    <span></span>
+  </div></tr></td></table>
+  <table width="1"><tr><td><div class="gap">
+    <span></span>
+  </div></tr></td></table>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/columns/min-width-1b.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <style>
+  div { -moz-column-width:200px; -moz-column-count:2; -moz-column-gap:0; border:2px solid black; height:200px; }
+  div.gap { -moz-column-gap:80px; }
+  span { display:inline-block; width:100px; }
+  </style>
+</head>
+<body>
+  <table width="1"><tr><td><div>
+    <span></span>
+  </div></tr></td></table>
+  <table width="1"><tr><td><div class="gap">
+    <span></span>
+  </div></tr></td></table>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/columns/min-width-1c.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <style>
+  div { -moz-column-width:100px; -moz-column-count:2; -moz-column-gap:0; border:2px solid black; height:200px; }
+  div.gap { -moz-column-gap:80px; }
+  span { display:inline-block; width:200px; }
+  </style>
+</head>
+<body>
+  <table width="1"><tr><td><div>
+    <span></span>
+  </div></tr></td></table>
+  <table width="1"><tr><td><div class="gap">
+    <span></span>
+  </div></tr></td></table>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/columns/pref-width-1-ref.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <style>
+  div { float:left; border:2px solid black; height:200px; }
+  div.clear { clear:both; }
+  span { display:inline-block; width:100px; }
+  span.gap { width:80px; }
+  </style>
+</head>
+<body>
+  <div>
+    <span></span><span></span>
+  </div>
+  <div class="clear">
+    <span></span><span class="gap"></span><span></span>
+  </div>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/columns/pref-width-1a.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <style>
+  div { -moz-column-count:2; -moz-column-gap:0; float:left; border:2px solid black; height:200px; }
+  div.clear { clear:both; }
+  div.gap { -moz-column-gap:80px; }
+  span { display:inline-block; width:100px; }
+  </style>
+</head>
+<body>
+  <div>
+    <span></span><br><span></span>
+  </div>
+  <div class="gap clear">
+    <span></span><br><span></span>
+  </div>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/columns/pref-width-1b.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <style>
+  div { -moz-column-count:2; -moz-column-width:100px; -moz-column-gap:0; float:left; border:2px solid black; height:200px; }
+  div.clear { clear:both; }
+  div.gap { -moz-column-gap:80px; }
+  span { display:inline-block; width:50px; }
+  </style>
+</head>
+<body>
+  <div>
+    <span></span><br><span></span>
+  </div>
+  <div class="gap clear">
+    <span></span><br><span></span>
+  </div>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/columns/pref-width-1c.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <style>
+  div { -moz-column-count:2; -moz-column-width:100px; -moz-column-gap:0; float:left; border:2px solid black; height:200px; }
+  div.clear { clear:both; }
+  div.gap { -moz-column-gap:80px; }
+  span { display:inline-block; width:200px; }
+  </style>
+</head>
+<body>
+  <div>
+    <span></span><br><span></span>
+  </div>
+  <div class="gap clear">
+    <span></span><br><span></span>
+  </div>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/columns/reftest.list
@@ -0,0 +1,7 @@
+== basic-1.html basic-ref.html
+== pref-width-1a.html pref-width-1-ref.html
+== pref-width-1b.html pref-width-1-ref.html
+== pref-width-1c.html pref-width-1-ref.html
+== min-width-1a.html min-width-1-ref.html
+== min-width-1b.html min-width-1-ref.html
+== min-width-1c.html min-width-1-ref.html
--- a/layout/reftests/reftest.list
+++ b/layout/reftests/reftest.list
@@ -50,8 +50,11 @@ include text-transform/reftest.list
 # native-theme/
 include native-theme/reftest.list
 
 # bidi
 include bidi/reftest.list
 
 # z-index/
 include z-index/reftest.list
+ 
+# columns/
+include columns/reftest.list
--- a/layout/style/nsCSSParser.cpp
+++ b/layout/style/nsCSSParser.cpp
@@ -4668,17 +4668,17 @@ PRBool CSSParserImpl::ParseSingleValuePr
   case eCSSProperty__moz_border_radius_bottomRight:
   case eCSSProperty__moz_border_radius_bottomLeft:
     return ParsePositiveVariant(aErrorCode, aValue, VARIANT_HLP, nsnull);
   case eCSSProperty__moz_column_count:
     return ParsePositiveVariant(aErrorCode, aValue, VARIANT_AHI, nsnull);
   case eCSSProperty__moz_column_width:
     return ParseVariant(aErrorCode, aValue, VARIANT_AHL, nsnull);
   case eCSSProperty__moz_column_gap:
-    return ParseVariant(aErrorCode, aValue, VARIANT_HLP | VARIANT_NORMAL, nsnull);
+    return ParseVariant(aErrorCode, aValue, VARIANT_HL | VARIANT_NORMAL, nsnull);
   case eCSSProperty__moz_outline_radius_topLeft:
   case eCSSProperty__moz_outline_radius_topRight:
   case eCSSProperty__moz_outline_radius_bottomRight:
   case eCSSProperty__moz_outline_radius_bottomLeft:
     return ParsePositiveVariant(aErrorCode, aValue, VARIANT_HLP, nsnull);
   case eCSSProperty_bottom:
   case eCSSProperty_top:
   case eCSSProperty_left:
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -586,18 +586,17 @@ nsComputedDOMStyle::GetColumnGap(nsIDOMC
 {
   nsROCSSPrimitiveValue *val = GetROCSSPrimitiveValue();
   NS_ENSURE_TRUE(val, NS_ERROR_OUT_OF_MEMORY);
 
   const nsStyleColumn* column = GetStyleColumn();
   if (column->mColumnGap.GetUnit() == eStyleUnit_Normal) {
     val->SetAppUnits(GetStyleFont()->mFont.size);
   } else {
-    SetValueToCoord(val, GetStyleColumn()->mColumnGap,
-                    &nsComputedDOMStyle::GetFrameContentWidth);
+    SetValueToCoord(val, GetStyleColumn()->mColumnGap);
   }
 
   return CallQueryInterface(val, aValue);
 }
 
 nsresult
 nsComputedDOMStyle::GetCounterIncrement(nsIDOMCSSValue** aValue)
 {
@@ -2818,29 +2817,16 @@ nsComputedDOMStyle::StyleCoordToNSCoord(
     default:
       break;
   }
       
   return aDefaultValue;
 }
 
 PRBool
-nsComputedDOMStyle::GetFrameContentWidth(nscoord& aWidth)
-{
-  if (!mFrame) {
-    return PR_FALSE;
-  }
-
-  FlushPendingReflows();
-
-  aWidth = mFrame->GetContentRect().width;
-  return PR_TRUE;
-}
-
-PRBool
 nsComputedDOMStyle::GetCBContentWidth(nscoord& aWidth)
 {
   if (!mFrame) {
     return PR_FALSE;
   }
 
   nsIFrame* container = GetContainingBlockFor(mFrame);
   if (!container) {
--- a/layout/style/nsComputedDOMStyle.h
+++ b/layout/style/nsComputedDOMStyle.h
@@ -302,17 +302,16 @@ private:
    * eStyleUnit_Percent, attempts to resolve the percentage base and returns
    * the resulting nscoord.  If it's some other unit or a percentge base can't
    * be determined, returns aDefaultValue.
    */
   nscoord StyleCoordToNSCoord(const nsStyleCoord& aCoord,
                               PercentageBaseGetter aPercentageBaseGetter,
                               nscoord aDefaultValue);
 
-  PRBool GetFrameContentWidth(nscoord& aWidth);
   PRBool GetCBContentWidth(nscoord& aWidth);
   PRBool GetCBContentHeight(nscoord& aWidth);
   PRBool GetFrameBorderRectWidth(nscoord& aWidth);
 
   struct ComputedStyleMapEntry
   {
     // Create a pointer-to-member-function type.
     typedef nsresult (nsComputedDOMStyle::*ComputeMethod)(nsIDOMCSSValue**);
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -1208,17 +1208,17 @@ struct nsStyleColumn : public nsStyleStr
 
   nsChangeHint CalcDifference(const nsStyleColumn& aOther) const;
 #ifdef DEBUG
   static nsChangeHint MaxDifference();
 #endif
 
   PRUint32     mColumnCount; // [reset] see nsStyleConsts.h
   nsStyleCoord mColumnWidth; // [reset]
-  nsStyleCoord mColumnGap;   // [reset]
+  nsStyleCoord mColumnGap;   // [reset] coord
 };
 
 #ifdef MOZ_SVG
 enum nsStyleSVGPaintType {
   eStyleSVGPaintType_None = 0,
   eStyleSVGPaintType_Color,
   eStyleSVGPaintType_Server
 };
--- a/layout/style/test/property_database.js
+++ b/layout/style/test/property_database.js
@@ -311,18 +311,18 @@ var gCSSProperties = {
 			"3px"
 		]
 	},
 	"-moz-column-gap": {
 		domProp: "MozColumnGap",
 		inherited: false,
 		type: CSS_TYPE_LONGHAND,
 		initial_values: [ "normal", "1em" ],
-		other_values: [ "2px", "4em", "3%" ],
-		invalid_values: []
+		other_values: [ "2px", "4em" ],
+		invalid_values: [ "3%" ]
 	},
 	"-moz-column-width": {
 		domProp: "MozColumnWidth",
 		inherited: false,
 		type: CSS_TYPE_LONGHAND,
 		initial_values: [ "auto" ],
 		other_values: [ "15px", "50%" ],
 		invalid_values: [ "20" ]
--- a/layout/style/test/test_bug365932.html
+++ b/layout/style/test/test_bug365932.html
@@ -34,18 +34,16 @@ https://bugzilla.mozilla.org/show_bug.cg
       border-width: 0 80px;
     }
   </style>
 </head>
 <body>
 <a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=365932">Mozilla Bug 365932</a>
 <p id="display"></p>
 <div id="content">
-  <div id="column1" style="-moz-column-gap: 200px;"></div>
-  <div id="column2" style="-moz-column-gap: 50%;"></div>
   <div id="indent1" style="text-indent: 400px"></div>
   <div id="indent2" style="text-indent: 50%"></div>
 
   <div id="widthheight-1" class="auto"></div>
 
   <div id="minwidth1-1" style="min-width: 200px"></div>
   <div id="minwidth1-2" style="min-width: 25%"></div>
   <div id="minwidth2-1" style="min-width: 600px"></div>
@@ -149,18 +147,16 @@ https://bugzilla.mozilla.org/show_bug.cg
        style="min-height: 75%; max-height: 40%"></div>
 
   <div id="radius1" style="-moz-border-radius: 80px"></div>
   <div id="radius2" style="-moz-border-radius: 10%"></div>
   <div id="outlineradius1" style="-moz-outline-radius: 160px"></div>
   <div id="outlineradius2" style="-moz-outline-radius: 20%"></div>
 </div>
 <div id="content2" style="display: none">
-  <div id="column3" style="-moz-column-gap: 200px;"></div>
-  <div id="column4" style="-moz-column-gap: 50%;"></div>
   <div id="indent3" style="text-indent: 400px"></div>
   <div id="indent4" style="text-indent: 50%"></div>
 
   <div id="minwidth1-3" style="min-width: 200px"></div>
   <div id="minwidth1-4" style="min-width: 25%"></div>
   <div id="minwidth2-3" style="min-width: 600px"></div>
   <div id="minwidth2-4" style="min-width: 75%"></div>
 
@@ -204,17 +200,16 @@ https://bugzilla.mozilla.org/show_bug.cg
 </div>
 <pre id="test">
 <script class="testbody" type="text/javascript">
 
 /** Test for Bug 365932 **/
 
 document.body.offsetWidth;
 
-doATest("-moz-column-gap", "column", 200, 50);
 doATest("text-indent", "indent", 400, 50);
 doATest("-moz-border-radius-topleft", "radius", 80, 10);
 doATest("-moz-outline-radius-topleft", "outlineradius", 160, 20);
 
 doATest("width", "widthheight-", 440, 0);
 doATest("height", "widthheight-", 0, 0);
 
 doATest("min-width", "minwidth1-", 200, 25);