Fix bug 404666 by making spanned colframes be continuations of the col that spans them. r=bernd, sr=roc, a=schrep
authorbzbarsky@mit.edu
Sun, 02 Dec 2007 23:45:06 -0800
changeset 8570 e1f7d1a21089a9b1714a15a135ba04e63fd286f2
parent 8569 87a1599ae1754fb1a687c0fc2e41e7ba52295f07
child 8571 165a2eeb98f5084d8dd2fb5d83e4f66011c1085a
push idunknown
push userunknown
push dateunknown
reviewersbernd, roc, schrep
bugs404666
milestone1.9b2pre
Fix bug 404666 by making spanned colframes be continuations of the col that spans them. r=bernd, sr=roc, a=schrep
layout/base/nsCSSFrameConstructor.cpp
layout/reftests/bugs/404666-1-ref.html
layout/reftests/bugs/404666-1.html
layout/reftests/bugs/404666-2-ref.html
layout/reftests/bugs/404666-2.html
layout/reftests/bugs/reftest.list
layout/tables/nsTableColFrame.cpp
layout/tables/nsTableColFrame.h
layout/tables/nsTableColGroupFrame.cpp
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -3979,18 +3979,20 @@ nsCSSFrameConstructor::ConstructTableCol
     // The same content node should always resolve to the same style context.
     if (1 == spanX)
       styleContext = aNewFrame->GetStyleContext();
     nsTableColFrame* newCol = NS_NewTableColFrame(mPresShell, styleContext);
     if (NS_UNLIKELY(!newCol)) {
       return NS_ERROR_OUT_OF_MEMORY;
     }
     InitAndRestoreFrame(aState, aContent, parentFrame, nsnull, newCol, PR_FALSE);
+    lastCol->SetNextSibling(newCol);
+    lastCol->SetNextContinuation(newCol);
+    newCol->SetPrevContinuation(lastCol);
     newCol->SetColType(eColAnonymousCol);
-    lastCol->SetNextSibling(newCol);
     lastCol = newCol;
   }
 
   if (!aIsPseudo && aIsPseudoParent) {
       aState.mPseudoFrames.mColGroup.mChildList.AddChild(aNewFrame);
   }
   
   return rv;
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/404666-1-ref.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+table { background: white }
+col { background: green }
+td { color: white }
+</style>
+</head>
+<body>
+<table>
+  <col id="col" span="2">
+  <tr>
+   <td>One</td>
+   <td>Two</td>
+   <td>Three</td>
+  </tr>
+</table>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/404666-1.html
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+table { background: white }
+td { color: white }
+</style>
+<script>
+function runTest()
+{
+  document.body.offsetHeight;
+  document.getElementById("col").style.backgroundColor = "green";
+}
+</script>
+</head>
+<body onload="runTest()">
+<table>
+  <col id="col" span="2">
+  <tr>
+   <td>One</td>
+   <td>Two</td>
+   <td>Three</td>
+  </tr>
+</table>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/404666-2-ref.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+table {background-color:green}
+col {background-color:red}
+td { color: white }
+</style>
+</head>
+<body>
+<table>
+  <col id="col" span="2" style="visibility: collapse">
+  <tr>
+   <td>One</td>
+   <td>Two</td>
+   <td>Three</td>
+  </tr>
+</table>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/404666-2.html
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+table {background-color:green}
+col {background-color:red}
+td { color: white }
+</style>
+<script>
+function runTest()
+{
+  document.body.offsetHeight;
+  document.getElementById("col").style.visibility = "collapse";
+}
+</script>
+</head>
+<body onload="runTest()">
+<table>
+  <col id="col" span="2">
+  <tr>
+   <td>One</td>
+   <td>Two</td>
+   <td>Three</td>
+  </tr>
+</table>
+</html>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -482,11 +482,13 @@ random == 403134-1.html 403134-1-ref.htm
 #== 403657-1.html 403657-1-ref.html  # Fails depending on the fonts...
 == 403733-1.html 403733-1-ref.html
 == 403962-1.xhtml 403962-1-ref.xhtml
 == 404030-1.html 404030-1-ref.html
 != 404030-1-notref.html 404030-1.html
 != 404030-1-notref2.html 404030-1.html
 == 404301-1.html 404301-1-ref.html
 != data:application/xml,<foo/> data:text/plain, # bug 404419
+== 404666-1.html 404666-1-ref.html
+== 404666-2.html 404666-2-ref.html
 == 405186-1.xhtml about:blank
 == 405305-1.html 405305-1-ref.html
 == 405584-1.html 405584-1-ref.html
--- a/layout/tables/nsTableColFrame.cpp
+++ b/layout/tables/nsTableColFrame.cpp
@@ -47,17 +47,18 @@
 #include "nsIDOMHTMLTableColElement.h"
 
 #define COL_TYPE_BITS                 0xF0000000 // uses bits 29-32 from mState
 #define COL_TYPE_OFFSET               28
 
 #define COL_CONSTRAINT_BITS           0x07000000 // uses bits 25-27
 #define COL_CONSTRAINT_OFFSET         24
 
-nsTableColFrame::nsTableColFrame(nsStyleContext* aContext) : nsFrame(aContext)
+nsTableColFrame::nsTableColFrame(nsStyleContext* aContext) :
+  nsSplittableFrame(aContext)
 {
   SetColType(eColContent);
   ResetIntrinsics();
   ResetSpanIntrinsics();
   ResetFinalWidth();
 }
 
 nsTableColFrame::~nsTableColFrame()
@@ -68,16 +69,21 @@ nsTableColType
 nsTableColFrame::GetColType() const 
 {
   return (nsTableColType)((mState & COL_TYPE_BITS) >> COL_TYPE_OFFSET);
 }
 
 void 
 nsTableColFrame::SetColType(nsTableColType aType) 
 {
+  NS_ASSERTION(aType != eColAnonymousCol ||
+               (GetPrevContinuation() &&
+                GetPrevContinuation()->GetNextContinuation() == this &&
+                GetPrevContinuation()->GetNextSibling() == this),
+               "spanned content cols must be continuations");
   PRUint32 type = aType - eColContent;
   mState |= (type << COL_TYPE_OFFSET);
 }
 
 void nsTableColFrame::SetContinuousBCBorderWidth(PRUint8     aForSide,
                                                  BCPixelSize aPixelValue)
 {
   switch (aForSide) {
@@ -167,17 +173,17 @@ NS_NewTableColFrame(nsIPresShell* aPresS
 }
 
 NS_IMETHODIMP
 nsTableColFrame::Init(nsIContent*      aContent,
                       nsIFrame*        aParent,
                       nsIFrame*        aPrevInFlow)
 {
   // Let the base class do its initialization
-  nsresult rv = nsFrame::Init(aContent, aParent, aPrevInFlow);
+  nsresult rv = nsSplittableFrame::Init(aContent, aParent, aPrevInFlow);
 
   // record that children that are ignorable whitespace should be excluded 
   mState |= NS_FRAME_EXCLUDE_IGNORABLE_WHITESPACE;
 
   return rv;
 }
 
 nsTableColFrame*  
@@ -201,8 +207,15 @@ nsTableColFrame::GetType() const
 
 #ifdef DEBUG
 NS_IMETHODIMP
 nsTableColFrame::GetFrameName(nsAString& aResult) const
 {
   return MakeFrameName(NS_LITERAL_STRING("TableCol"), aResult);
 }
 #endif
+
+nsSplittableType
+nsTableColFrame::GetSplittableType() const
+{
+  return NS_FRAME_NOT_SPLITTABLE;
+}
+
--- a/layout/tables/nsTableColFrame.h
+++ b/layout/tables/nsTableColFrame.h
@@ -46,17 +46,17 @@ class nsTableCellFrame;
 
 enum nsTableColType {
   eColContent            = 0, // there is real col content associated   
   eColAnonymousCol       = 1, // the result of a span on a col
   eColAnonymousColGroup  = 2, // the result of a span on a col group
   eColAnonymousCell      = 3  // the result of a cell alone
 };
 
-class nsTableColFrame : public nsFrame {
+class nsTableColFrame : public nsSplittableFrame {
 public:
 
   enum {eWIDTH_SOURCE_NONE          =0,   // no cell has contributed to the width style
         eWIDTH_SOURCE_CELL          =1,   // a cell specified a width
         eWIDTH_SOURCE_CELL_WITH_SPAN=2    // a cell implicitly specified a width via colspan
   };
 
   nsTableColType GetColType() const;
@@ -98,16 +98,18 @@ public:
    * @see nsGkAtoms::tableColFrame
    */
   virtual nsIAtom* GetType() const;
   
 #ifdef DEBUG
   NS_IMETHOD GetFrameName(nsAString& aResult) const;
 #endif
 
+  virtual nsSplittableType GetSplittableType() const;
+
   /** return the number of the columns the col represents.  always >= 1 */
   PRInt32 GetSpan();
 
   /** convenience method, calls into cellmap */
   nsVoidArray * GetCells();
 
   /** convenience method, calls into cellmap */
   PRInt32 Count() const;
--- a/layout/tables/nsTableColGroupFrame.cpp
+++ b/layout/tables/nsTableColGroupFrame.cpp
@@ -242,30 +242,27 @@ nsTableColGroupFrame::InsertFrames(nsIAt
 
   nsTableColFrame* col = GetFirstColumn();
   nsTableColFrame* nextCol;
   while (col && col->GetColType() == eColAnonymousColGroup) {
     // this colgroup spans one or more columns but now that there is a
     // real column below, spanned anonymous columns should be removed,
     // since the HTML spec says to ignore the span of a colgroup if it
     // has content columns in it.
+    NS_ASSERTION(col != aPrevFrame, "Bad aPrevFrame");
     nextCol = col->GetNextCol();
     RemoveFrame(nsnull, col);
     col = nextCol;
   }
 
-  if (aPrevFrame) {
-    col = GetNextColumn(aPrevFrame);
-    while (col && col->GetColType() == eColAnonymousCol) {
-      // This is a column frame from a <col span="N">.  We want to
-      // insert our new frame after the end of this span
-      aPrevFrame = col;
-      col = col->GetNextCol();
-    }
-  }
+  NS_ASSERTION(!aPrevFrame || aPrevFrame == aPrevFrame->GetLastContinuation(),
+               "Prev frame should be last in continuation chain");
+  NS_ASSERTION(!aPrevFrame || !GetNextColumn(aPrevFrame) ||
+               GetNextColumn(aPrevFrame)->GetColType() != eColAnonymousCol,
+               "Shouldn't be inserting before a spanned colframe");
 
   mFrames.InsertFrames(this, aPrevFrame, aFrameList);
   nsIFrame* prevFrame = nsTableFrame::GetFrameAtOrBefore(this, aPrevFrame,
                                                          nsGkAtoms::tableColFrame);
 
   PRInt32 colIndex = (prevFrame) ? ((nsTableColFrame*)prevFrame)->GetColIndex() + 1 : GetStartColumnIndex();
   InsertColsReflow(colIndex, aFrameList, lastFrame);