Make visibility:collapse work again after the display list change. Implement overflow handling and view synchronization for collapsed frames. bug 326551 r/sr=roc

layout/tables/nsTableCellFrame.cpp

@@ -208,9 +208,8 @@ nsTableCellFrame::AttributeChanged(PRInt32         aNameSpaceID,
                                    PRInt32         aModType)
 {
   // let the table frame decide what to do
-  nsTableFrame* tableFrame = nsnull; 
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsresult rv = nsTableFrame::GetTableFrame(this, tableFrame);
+  if (tableFrame) {
-  if ((NS_SUCCEEDED(rv)) && (tableFrame)) {
     tableFrame->AttributeChangedFor(this, mContent, aAttribute); 
   }
   return NS_OK;
@@ -404,9 +403,7 @@ nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
   // take account of 'empty-cells'
   if (GetStyleVisibility()->IsVisible() &&
       (NS_STYLE_TABLE_EMPTY_CELLS_HIDE != emptyCellStyle)) {
-    nsTableFrame* tableFrame;
+    nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-    nsTableFrame::GetTableFrame(this, tableFrame);
-    NS_ASSERTION(tableFrame, "null table frame");
 
     // display background if we need to. Note that we don't try to display
     // a background item to catch events; our anonymous inner block will catch
@@ -446,13 +443,11 @@ nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
   nsresult rv = DisplayOutline(aBuilder, aLists);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  nsPoint offset;
-  GetCollapseOffset(offset);
   PRBool quirkyClip = HasPctOverHeight() &&
     eCompatibility_NavQuirks == GetPresContext()->CompatibilityMode();
   nsIFrame* kid = mFrames.FirstChild();
   NS_ASSERTION(kid && !kid->GetNextSibling(), "Table cells should have just one child");
-  if (0 == offset.x && 0 == offset.y && !quirkyClip) {
+  if (!quirkyClip) {
     // The child's background will go in our BorderBackground() list.
     // This isn't a problem since it won't have a real background except for
     // event handling. We do not call BuildDisplayListForNonBlockChildren
@@ -471,22 +466,6 @@ nsTableCellFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
   if (quirkyClip) {
     clip = nsRect(nsPoint(0, 0), GetSize());
   }
-  if (offset.x < 0) {
-    // Clip off content to the left of the x=0 line. This is bogus really,
-    // but the whole handling of collapsed-offset cells is bogus.
-    if (clip.x < 0) {
-      clip.width = PR_MAX(0, clip.XMost());
-      clip.x = 0;
-    }
-  }
-  if (offset.y < 0) {
-    // Clip off content above the y=0 line. This is bogus really,
-    // but the whole handling of collapsed-offset cells is bogus.
-    if (clip.y < 0) {
-      clip.height = PR_MAX(0, clip.YMost());
-      clip.y = 0;
-    }
-  }
   return OverflowClip(aBuilder, set, aLists, clip + aBuilder->ToReferenceFrame(this));
 }
 
@@ -778,8 +757,9 @@ NS_METHOD nsTableCellFrame::Reflow(nsPresContext*          aPresContext,
   PRBool noBorderBeforeReflow = GetContentEmpty() &&
     GetStyleTableBorder()->mEmptyCells != NS_STYLE_TABLE_EMPTY_CELLS_SHOW;
   /* XXX: remove tableFrame when border-collapse inherits */
-  nsTableFrame* tableFrame = nsnull;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  rv = nsTableFrame::GetTableFrame(this, tableFrame); if (!tableFrame) ABORT1(NS_ERROR_NULL_POINTER);
+  if (!tableFrame)
+    ABORT1(NS_ERROR_NULL_POINTER);
 
   nsMargin borderPadding = aReflowState.mComputedPadding;
   nsMargin border;
@@ -877,10 +857,7 @@ NS_METHOD nsTableCellFrame::Reflow(nsPresContext*          aPresContext,
     }
     kidOrigin = firstKid->GetPosition();
   }
-  nsPoint collapsedOffset;
+  
-  GetCollapseOffset(collapsedOffset);
-  kidOrigin += collapsedOffset;
-
 #if defined DEBUG_TABLE_REFLOW_TIMING
   nsTableFrame::DebugReflow(firstKid, (nsHTMLReflowState&)kidReflowState);
 #endif
@@ -922,7 +899,6 @@ NS_METHOD nsTableCellFrame::Reflow(nsPresContext*          aPresContext,
       topInset    += border.top;
       bottomInset += border.bottom;
       kidOrigin.MoveTo(leftInset, topInset);
-      kidOrigin += collapsedOffset;
     }
   }
 
@@ -1133,10 +1109,9 @@ nsTableCellFrame::GetPreviousCellInColumn(nsITableCellLayout **aCellLayout)
   if (!aCellLayout) return NS_ERROR_NULL_POINTER;
   *aCellLayout = nsnull;
 
-  nsTableFrame* tableFrame = nsnull; 
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsresult rv = nsTableFrame::GetTableFrame(this, tableFrame);
+  if (!tableFrame)
-  if (NS_FAILED(rv)) return rv;
+    return NS_ERROR_FAILURE;
-  if (!tableFrame) return NS_ERROR_FAILURE;
 
   // Get current cell location
   PRInt32 rowIndex, colIndex;
@@ -1157,10 +1132,9 @@ nsTableCellFrame::GetNextCellInColumn(nsITableCellLayout **aCellLayout)
   if (!aCellLayout) return NS_ERROR_NULL_POINTER;
   *aCellLayout = nsnull;
 
-  nsTableFrame* tableFrame = nsnull; 
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsresult rv = nsTableFrame::GetTableFrame(this, tableFrame);
+  if (!tableFrame)
-  if (NS_FAILED(rv)) return rv;
+    return NS_ERROR_FAILURE;
-  if (!tableFrame) return NS_ERROR_FAILURE;
 
   // Get current cell location
   PRInt32 rowIndex, colIndex;
@@ -1204,51 +1178,6 @@ nsTableCellFrame::GetFrameName(nsAString& aResult) const
 }
 #endif
 
-void nsTableCellFrame::UpdateChildOffset(nsPoint aDelta)
-{
-  nsIFrame* kid = mFrames.FirstChild();
-  NS_ASSERTION(kid, "Table cells must have one kid");
-  
-  // XXX this is obscene! It doesn't move views or adjust overflow areas.
-  // On the other hand, the old approach of adjusting the rendering
-  // context translation was utterly wrong too.
-  kid->SetRect(kid->GetRect() + aDelta);
-}
-
-void nsTableCellFrame::SetCollapseOffsetX(nscoord aXOffset)
-{
-  // Get the frame property (creating a point struct if necessary)
-  nsPoint* offset = (nsPoint*)nsTableFrame::GetProperty(this, nsLayoutAtoms::collapseOffsetProperty, aXOffset != 0);
-
-  if (offset) {
-    UpdateChildOffset(nsPoint(aXOffset, 0) - *offset);
-    offset->x = aXOffset;
-  }
-}
-
-void nsTableCellFrame::SetCollapseOffsetY(nscoord aYOffset)
-{
-  // Get the property (creating a point struct if necessary)
-  nsPoint* offset = (nsPoint*)nsTableFrame::GetProperty(this, nsLayoutAtoms::collapseOffsetProperty, aYOffset != 0);
-
-  if (offset) {
-    UpdateChildOffset(nsPoint(0, aYOffset) - *offset);
-    offset->y = aYOffset;
-  }
-}
-
-void nsTableCellFrame::GetCollapseOffset(nsPoint& aOffset)
-{
-  // See if the property is set
-  nsPoint* offset = (nsPoint*)nsTableFrame::GetProperty(this, nsLayoutAtoms::collapseOffsetProperty);
-
-  if (offset) {
-    aOffset = *offset;
-  } else {
-    aOffset.MoveTo(0, 0);
-  }
-}
-
 // nsBCTableCellFrame
 
 nsBCTableCellFrame::nsBCTableCellFrame()

layout/tables/nsTableCellFrame.h

@@ -250,19 +250,6 @@ public:
   nscoord GetLastBlockHeight();
   void    SetLastBlockHeight(nscoord aValue);
 
-  // The collapse offset is (0,0) except for cells originating in a row/col which is collapsed
-  void    SetCollapseOffsetX(nscoord aXOffset);
-  void    SetCollapseOffsetY(nscoord aYOffset);
-  void    GetCollapseOffset(nsPoint& aOffset);
-  
-  /**
-   * This gets called when the collapse offset X or Y changes. Currently it's
-   * a disgusting hack (but not worse than the previous disgusting hack it
-   * replaces).
-   * XXX fix this when views have been removed.
-   */
-  void UpdateChildOffset(nsPoint aDelta);
-
   nsTableCellFrame* GetNextCell() const;
 
   virtual nsMargin* GetBorderWidth(float     aPixelsToTwips,

layout/tables/nsTableColFrame.cpp

@@ -140,10 +140,9 @@ NS_METHOD nsTableColFrame::Reflow(nsPresContext*          aPresContext,
   const nsStyleVisibility* colVis = GetStyleVisibility();
   PRBool collapseCol = (NS_STYLE_VISIBILITY_COLLAPSE == colVis->mVisible);
   if (collapseCol) {
-    nsTableFrame* tableFrame = nsnull;
+    nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-    nsTableFrame::GetTableFrame(this, tableFrame);
     if (tableFrame)  {
-      tableFrame->SetNeedToCollapseColumns(PR_TRUE);
+      tableFrame->SetNeedToCollapse(PR_TRUE);
     }    
   }
   if (aDesiredSize.mComputeMEW)

layout/tables/nsTableColGroupFrame.cpp

@@ -103,9 +103,9 @@ nsTableColGroupFrame::AddColsToTable(PRInt32          aFirstColIndex,
                                      nsIFrame*        aLastFrame)
 {
   nsresult rv = NS_OK;
-  nsTableFrame* tableFrame = nsnull;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  rv = nsTableFrame::GetTableFrame(this, tableFrame);
+  if (!tableFrame || !aFirstFrame)
-  if (!tableFrame || !aFirstFrame) return NS_ERROR_NULL_POINTER;
+    return NS_ERROR_NULL_POINTER;
 
   // set the col indices of the col frames and and add col info to the table
   PRInt32 colIndex = aFirstColIndex;
@@ -177,9 +177,9 @@ nsTableColGroupFrame::SetInitialChildList(nsPresContext* aPresContext,
                                           nsIAtom*        aListName,
                                           nsIFrame*       aChildList)
 {
-  nsTableFrame* tableFrame;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame(this, tableFrame);
+  if (!tableFrame)
-  if (!tableFrame) return NS_ERROR_NULL_POINTER;
+    return NS_ERROR_NULL_POINTER;
 
   if (!aChildList) {
     nsIFrame* firstChild;
@@ -249,9 +249,9 @@ nsTableColGroupFrame::InsertColsReflow(PRInt32         aColIndex,
 {
   AddColsToTable(aColIndex, PR_TRUE, aFirstFrame, aLastFrame);
 
-  nsTableFrame* tableFrame;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame(this, tableFrame);
+  if (!tableFrame)
-  if (!tableFrame) return;
+    return;
 
   // XXX this could be optimized with much effort
   tableFrame->SetNeedStrategyInit(PR_TRUE);
@@ -283,9 +283,9 @@ nsTableColGroupFrame::RemoveChild(nsTableColFrame& aChild,
       }
     }
   }
-  nsTableFrame* tableFrame;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame(this, tableFrame);
+  if (!tableFrame)
-  if (!tableFrame) return;
+    return;
 
   // XXX this could be optimized with much effort
   tableFrame->SetNeedStrategyInit(PR_TRUE);
@@ -304,9 +304,9 @@ nsTableColGroupFrame::RemoveFrame(nsIAtom*        aListName,
     PRInt32 colIndex = colFrame->GetColIndex();
     RemoveChild(*colFrame, PR_TRUE);
     
-    nsTableFrame* tableFrame;
+    nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-    nsTableFrame::GetTableFrame(this, tableFrame);
+    if (!tableFrame)
-    if (!tableFrame) return NS_ERROR_NULL_POINTER;
+      return NS_ERROR_NULL_POINTER;
 
     tableFrame->RemoveCol(this, colIndex, PR_TRUE, PR_TRUE);
 
@@ -348,10 +348,9 @@ NS_METHOD nsTableColGroupFrame::Reflow(nsPresContext*          aPresContext,
   const nsStyleVisibility* groupVis = GetStyleVisibility();
   PRBool collapseGroup = (NS_STYLE_VISIBILITY_COLLAPSE == groupVis->mVisible);
   if (collapseGroup) {
-    nsTableFrame* tableFrame = nsnull;
+    nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-    nsTableFrame::GetTableFrame(this, tableFrame);
     if (tableFrame)  {
-      tableFrame->SetNeedToCollapseColumns(PR_TRUE);
+      tableFrame->SetNeedToCollapse(PR_TRUE);;
     }
   }
   // for every content child that (is a column thingy and does not already have a frame)
@@ -438,15 +437,15 @@ NS_METHOD nsTableColGroupFrame::IR_StyleChanged(nsHTMLReflowMetrics&     aDesire
                                                 const nsHTMLReflowState& aReflowState,
                                                 nsReflowStatus&          aStatus)
 {
-  nsresult rv = NS_OK;
   // we presume that all the easy optimizations were done in the nsHTMLStyleSheet before we were called here
   // XXX: we can optimize this when we know which style attribute changed
-  nsTableFrame* tableFrame = nsnull;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  rv = nsTableFrame::GetTableFrame(this, tableFrame);
+  if (!tableFrame)
-  if (tableFrame)  {
+    return NS_ERROR_NULL_POINTER;
-    tableFrame->SetNeedStrategyInit(PR_TRUE);
+  
-  }
+  tableFrame->SetNeedStrategyInit(PR_TRUE);
-  return rv;
+  return NS_OK;
+  
 }
 
 NS_METHOD nsTableColGroupFrame::IR_TargetIsChild(nsHTMLReflowMetrics&     aDesiredSize,
@@ -467,16 +466,16 @@ NS_METHOD nsTableColGroupFrame::IR_TargetIsChild(nsHTMLReflowMetrics&     aDesir
   if (NS_FAILED(rv))
     return rv;
 
-  nsTableFrame *tableFrame=nsnull;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  rv = nsTableFrame::GetTableFrame(this, tableFrame);
+  if (!tableFrame)
-  if (tableFrame) {
+    return NS_ERROR_NULL_POINTER;
-    // compare the new col count to the old col count.  
+  
-    // If they are the same, we just need to rebalance column widths
+  // compare the new col count to the old col count.
-    // If they differ, we need to fix up other column groups and the column cache
+  // If they are the same, we just need to rebalance column widths
-    // XXX for now assume the worse
+  // If they differ, we need to fix up other column groups and the column cache
-    tableFrame->SetNeedStrategyInit(PR_TRUE);
+  // XXX for now assume the worse
-  }
+  tableFrame->SetNeedStrategyInit(PR_TRUE);
-  return rv;
+  return NS_OK;
 }
 
 nsTableColFrame * nsTableColGroupFrame::GetFirstColumn()
@@ -488,9 +487,13 @@ nsTableColFrame * nsTableColGroupFrame::GetNextColumn(nsIFrame *aChildFrame)
 {
   nsTableColFrame *result = nsnull;
   nsIFrame *childFrame = aChildFrame;
-  if (nsnull==childFrame)
+  if (!childFrame) {
     childFrame = mFrames.FirstChild();
-  while (nsnull!=childFrame)
+  }
+  else {
+    childFrame = childFrame->GetNextSibling();
+  }
+  while (childFrame)
   {
     if (NS_STYLE_DISPLAY_TABLE_COLUMN ==
         childFrame->GetStyleDisplay()->mDisplay)
@@ -542,8 +545,7 @@ void nsTableColGroupFrame::SetContinuousBCBorderWidth(PRUint8     aForSide,
 void nsTableColGroupFrame::GetContinuousBCBorderWidth(float     aPixelsToTwips,
                                                       nsMargin& aBorder)
 {
-  nsTableFrame* table;
+  nsTableFrame* table = nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame(this, table);
   nsTableColFrame* col = table->GetColFrame(mStartColIndex + mColCount - 1);
   col->GetContinuousBCBorderWidth(aPixelsToTwips, aBorder);
   aBorder.top = BC_BORDER_BOTTOM_HALF_COORD(aPixelsToTwips,

layout/tables/nsTableFrame.cpp

@@ -1804,8 +1804,7 @@ nsTableFrame::IsPrematureSpecialHeightReflow(const nsHTMLReflowState& aReflowSta
   PRBool premature = PR_FALSE; 
   if (aReflowState.mFlags.mSpecialHeightReflow) { 
     if (aNeedSpecialHeightReflow) { 
-      nsTableFrame* tableFrame; 
+      nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(aReflowState.frame);
-      nsTableFrame::GetTableFrame(aReflowState.frame, tableFrame); 
       if (tableFrame && (tableFrame != aReflowState.mPercentHeightReflowInitiator)) { 
         premature = PR_TRUE; 
       } 
@@ -2062,11 +2061,9 @@ NS_METHOD nsTableFrame::Reflow(nsPresContext*          aPresContext,
 
   nsMargin borderPadding = GetChildAreaOffset(&aReflowState);
   SetColumnDimensions(aDesiredSize.height, borderPadding);
-  if (NeedToCollapseRows()) {
+  if (NeedToCollapse() &&
-    AdjustForCollapsingRows(aDesiredSize);
+      (NS_UNCONSTRAINEDSIZE != aReflowState.availableWidth)) {
-  }
+    AdjustForCollapsingRowsCols(aDesiredSize, borderPadding);
-  if (NeedToCollapseColumns()) {
-    AdjustForCollapsingCols(aDesiredSize);
   }
 
   // See if we need to calc max elem and/or preferred widths. This isn't done on 
@@ -2301,206 +2298,77 @@ nsTableFrame::MoveOverflowToChildList(nsPresContext* aPresContext)
   return result;
 }
 
-NS_METHOD 
-nsTableFrame::CollapseRowGroupIfNecessary(nsIFrame* aRowGroupFrame,
-                                          const nscoord& aYTotalOffset,
-                                          nscoord& aYGroupOffset, PRInt32& aRowX)
-{
-  const nsStyleVisibility* groupVis = aRowGroupFrame->GetStyleVisibility();
-  
-  PRBool collapseGroup = (NS_STYLE_VISIBILITY_COLLAPSE == groupVis->mVisible);
-  if (collapseGroup) {
-    SetNeedToCollapseRows(PR_TRUE);
-  }
-  nsIFrame* rowFrame = aRowGroupFrame->GetFirstChild(nsnull);
 
-  while (nsnull != rowFrame) {
-    const nsStyleDisplay* rowDisplay = rowFrame->GetStyleDisplay();
-    if (NS_STYLE_DISPLAY_TABLE_ROW == rowDisplay->mDisplay) {
-      const nsStyleVisibility* rowVis = rowFrame->GetStyleVisibility();
-      PRBool collapseRow = (NS_STYLE_VISIBILITY_COLLAPSE == rowVis->mVisible);
-      if (collapseRow) {
-        SetNeedToCollapseRows(PR_TRUE);
-      }
-      nsRect rowRect = rowFrame->GetRect();
-      if (collapseGroup || collapseRow) {
-        aYGroupOffset += rowRect.height;
-        rowRect.height = 0;
-        rowFrame->SetRect(rowRect);
-        nsIFrame* cellFrame = rowFrame->GetFirstChild(nsnull);
-        while (nsnull != cellFrame) {
-          const nsStyleDisplay* cellDisplay = cellFrame->GetStyleDisplay();
-          if (NS_STYLE_DISPLAY_TABLE_CELL == cellDisplay->mDisplay) {
-            nsTableCellFrame* cFrame = (nsTableCellFrame*)cellFrame;
-            nsRect cRect = cFrame->GetRect();
-            cRect.height -= rowRect.height;
-            cFrame->SetCollapseOffsetY(-aYGroupOffset);
-            cFrame->SetRect(cRect);
-          }
-          cellFrame = cellFrame->GetNextSibling();
-        }
-        // check if a cell above spans into here
-        nsTableCellMap* cellMap = GetCellMap();
-        if (cellMap) {
-          PRInt32 numCols = cellMap->GetColCount();
-          nsTableCellFrame* lastCell = nsnull;
-          for (int colX = 0; colX < numCols; colX++) {
-            CellData* cellData = cellMap->GetDataAt(aRowX, colX);
-            if (cellData && cellData->IsRowSpan()) { // a cell above is spanning into here
-              // adjust the real cell's rect only once
-              nsTableCellFrame* realCell = cellMap->GetCellFrame(aRowX, colX, *cellData, PR_TRUE);
-              NS_ASSERTION(realCell, "row span without origin?");
-              if (realCell && (realCell != lastCell)) {
-                nsRect realRect = realCell->GetRect();
-                realRect.height -= rowRect.height;
-                realCell->SetRect(realRect);
-              }
-              lastCell = realCell;
-            }
-          }
-        }
-      } else { // row is not collapsed but needs to be adjusted by those that are
-        rowRect.y -= aYGroupOffset;
-        rowFrame->SetRect(rowRect);
-        // reset the collapse yoffset
-        nsIFrame* cellFrame = rowFrame->GetFirstChild(nsnull);
-        while (cellFrame) {
-          const nsStyleDisplay* cellDisplay = cellFrame->GetStyleDisplay();
-          if (NS_STYLE_DISPLAY_TABLE_CELL == cellDisplay->mDisplay) {
-            nsTableCellFrame* cFrame = (nsTableCellFrame*)cellFrame;
-            // reset the offset as this row is not collapsed
-            cFrame->SetCollapseOffsetY(0);
-          }
-          cellFrame = cellFrame->GetNextSibling();
-        }
-      }
-      aRowX++;
-    }
-    rowFrame = rowFrame->GetNextSibling();
-  } // end row frame while
-
-  nsRect groupRect = aRowGroupFrame->GetRect();
-  groupRect.height -= aYGroupOffset;
-  groupRect.y -= aYTotalOffset;
-  aRowGroupFrame->SetRect(groupRect);
-
-  return NS_OK;
-}
 
 // collapsing row groups, rows, col groups and cols are accounted for after both passes of
 // reflow so that it has no effect on the calculations of reflow.
-NS_METHOD nsTableFrame::AdjustForCollapsingRows(nsHTMLReflowMetrics& aDesiredSize)
+void
+nsTableFrame::AdjustForCollapsingRowsCols(nsHTMLReflowMetrics& aDesiredSize,
+                                          nsMargin             aBorderPadding)
 {
-  nscoord yGroupOffset = 0; // total offset among rows within a single row group
   nscoord yTotalOffset = 0; // total offset among all rows in all row groups
-  PRInt32 rowIndex = 0;
+
   // reset the bit, it will be set again if row/rowgroup is collapsed
-  SetNeedToCollapseRows(PR_FALSE); 
+  SetNeedToCollapse(PR_FALSE);
   
   // collapse the rows and/or row groups as necessary
   // Get the ordered children
   nsAutoVoidArray rowGroups;
   PRUint32 numRowGroups;
   OrderRowGroups(rowGroups, numRowGroups);
-  
+  nscoord width = GetCollapsedWidth(aBorderPadding);
+  nscoord rgWidth = width - 2 * GetCellSpacingX();
+  nsRect overflowArea(0, 0, 0, 0);
   // Walk the list of children
   for (PRUint32 childX = 0; childX < numRowGroups; childX++) {
     nsIFrame* childFrame = (nsIFrame*)rowGroups.ElementAt(childX);
     nsTableRowGroupFrame* rgFrame = GetRowGroupFrame(childFrame);
     if (!rgFrame) continue; // skip foreign frame types
-    CollapseRowGroupIfNecessary(rgFrame, yTotalOffset, yGroupOffset, rowIndex);
+    yTotalOffset += rgFrame->CollapseRowGroupIfNecessary(yTotalOffset, rgWidth);
-    yTotalOffset += yGroupOffset;
+    ConsiderChildOverflow(overflowArea, rgFrame);
-    yGroupOffset = 0;
   } 
 
   aDesiredSize.height -= yTotalOffset;
- 
+  aDesiredSize.width   = width;
-  return NS_OK;
+  overflowArea.UnionRect(nsRect(0, 0, aDesiredSize.width, aDesiredSize.height),
+                         overflowArea);
+  FinishAndStoreOverflow(&overflowArea,
+                         nsSize(aDesiredSize.width, aDesiredSize.height));
 }
 
-NS_METHOD nsTableFrame::AdjustForCollapsingCols(nsHTMLReflowMetrics& aDesiredSize)
+nscoord
+nsTableFrame::GetCollapsedWidth(nsMargin aBorderPadding)
 {
-  nsTableCellMap* cellMap = GetCellMap();
-  if (!cellMap) return NS_OK;
-   // reset the bit, it will be set again if col/colgroup is collapsed
-  SetNeedToCollapseColumns(PR_FALSE);
- 
-  PRInt32 numRows = cellMap->GetRowCount();
-  nsTableIterator groupIter(mColGroups, eTableDIR);
-  nsIFrame* groupFrame = groupIter.First(); 
   nscoord cellSpacingX = GetCellSpacingX();
-  nscoord xOffset = 0;
+  nscoord width = cellSpacingX;
-  PRInt32 colX = (groupIter.IsLeftToRight()) ? 0 : GetColCount() - 1; 
+  width += aBorderPadding.left + aBorderPadding.right;
-  PRInt32 direction = (groupIter.IsLeftToRight()) ? 1 : -1; 
+  for (nsIFrame* groupFrame = mColGroups.FirstChild(); groupFrame;
-  // iterate over the col groups
+         groupFrame = groupFrame->GetNextSibling()) {
-  while (nsnull != groupFrame) {
     const nsStyleVisibility* groupVis = groupFrame->GetStyleVisibility();
-    
     PRBool collapseGroup = (NS_STYLE_VISIBILITY_COLLAPSE == groupVis->mVisible);
-    if (collapseGroup) {
+    nsTableColGroupFrame* cgFrame = (nsTableColGroupFrame*)groupFrame;
-      SetNeedToCollapseColumns(PR_TRUE);
+    for (nsTableColFrame* colFrame = cgFrame->GetFirstColumn(); colFrame;
-    }
+         colFrame = colFrame->GetNextCol()) {
-    nsTableIterator colIter(*groupFrame, eTableDIR);
-    nsIFrame* colFrame = colIter.First();
-    // iterate over the cols in the col group
-    while (nsnull != colFrame) {
       const nsStyleDisplay* colDisplay = colFrame->GetStyleDisplay();
+      PRInt32 colX = colFrame->GetColIndex();
       if (NS_STYLE_DISPLAY_TABLE_COLUMN == colDisplay->mDisplay) {
         const nsStyleVisibility* colVis = colFrame->GetStyleVisibility();
         PRBool collapseCol = (NS_STYLE_VISIBILITY_COLLAPSE == colVis->mVisible);
-        if (collapseCol) {
-          SetNeedToCollapseColumns(PR_TRUE);
-        }
         PRInt32 colWidth = GetColumnWidth(colX);
-        if (collapseGroup || collapseCol) {
+        if (!collapseGroup && !collapseCol) {
-          xOffset += colWidth + cellSpacingX;
+          width += colWidth;
+          if (GetNumCellsOriginatingInCol(colX) > 0)
+            width += cellSpacingX;
         }
-        nsTableCellFrame* lastCell  = nsnull;
-        nsTableCellFrame* cellFrame = nsnull;
-        for (PRInt32 rowX = 0; rowX < numRows; rowX++) {
-          CellData* cellData = cellMap->GetDataAt(rowX, colX);
-          if (cellData) {
-            if (cellData->IsOrig()) { // the cell originates at (rowX, colX)
-              cellFrame = cellData->GetCellFrame();
-              // reset the collapse offsets since they may have been collapsed previously
-              cellFrame->SetCollapseOffsetX(0);
-              cellFrame->SetCollapseOffsetY(0);
-              nsRect cellRect = cellFrame->GetRect();
-              if (collapseGroup || collapseCol) {
-                if (lastCell != cellFrame) { // do it only once if there is a row span
-                  cellRect.width -= colWidth;
-                  cellFrame->SetCollapseOffsetX(-xOffset);
-                }
-              } else { // the cell is not in a collapsed col but needs to move
-                cellRect.x -= xOffset;
-              }
-              cellFrame->SetRect(cellRect);
-              // if the cell does not originate at (rowX, colX), adjust the real cells width
-            } else if (collapseGroup || collapseCol) { 
-              if (cellData->IsColSpan()) {
-                cellFrame = cellMap->GetCellFrame(rowX, colX, *cellData, PR_FALSE);
-              }
-              if ((cellFrame) && (lastCell != cellFrame)) {
-                nsRect cellRect = cellFrame->GetRect();
-                cellRect.width -= colWidth + cellSpacingX;
-                cellFrame->SetRect(cellRect);
-              }
-            }
-          }
-          lastCell = cellFrame;
-        }
-        colX += direction;
       }
-      colFrame = colIter.Next();
+    }
-    } // inner while
+  }
-    groupFrame = groupIter.Next();
+  return width;
-  } // outer while
-
-  aDesiredSize.width -= xOffset;
- 
-  return NS_OK;
 }
 
+
+
+
 NS_IMETHODIMP
 nsTableFrame::AppendFrames(nsIAtom*        aListName,
                            nsIFrame*       aFrameList)
@@ -4108,26 +3976,20 @@ NS_NewTableFrame(nsIPresShell* aPresShell)
   return new (aPresShell) nsTableFrame;
 }
 
-NS_METHOD 
+nsTableFrame*
-nsTableFrame::GetTableFrame(nsIFrame*      aSourceFrame, 
+nsTableFrame::GetTableFrame(nsIFrame* aSourceFrame)
-                            nsTableFrame*& aTableFrame)
 {
-  nsresult rv = NS_ERROR_UNEXPECTED;  // the value returned
-  aTableFrame = nsnull;               // initialize out-param
   if (aSourceFrame) {
     // "result" is the result of intermediate calls, not the result we return from this method
     for (nsIFrame* parentFrame = aSourceFrame->GetParent(); parentFrame;
          parentFrame = parentFrame->GetParent()) {
       if (nsLayoutAtoms::tableFrame == parentFrame->GetType()) {
-        aTableFrame = (nsTableFrame*)parentFrame;
+        return (nsTableFrame*)parentFrame;
-        rv = NS_OK; // only set if we found the table frame
-        break;
       }
     }
   }
-  NS_POSTCONDITION(nsnull!=aTableFrame, "unable to find table parent. aTableFrame null.");
+  NS_NOTREACHED("unable to find table parent");
-  NS_POSTCONDITION(NS_OK==rv, "unable to find table parent. result!=NS_OK");
+  return nsnull;
-  return rv;
 }
 
 PRBool 
@@ -4445,9 +4307,8 @@ void nsTableIterator::Init(nsIFrame*        aFirstChild,
     return;
   }
   if (eTableDIR == aType) {
-    nsTableFrame* table = nsnull;
+    nsTableFrame* table = nsTableFrame::GetTableFrame(mFirstChild);
-    nsresult rv = nsTableFrame::GetTableFrame(mFirstChild, table);
+    if (table) {
-    if (NS_SUCCEEDED(rv) && (table != nsnull)) {
       mLeftToRight = (NS_STYLE_DIRECTION_LTR ==
                       table->GetStyleVisibility()->mDirection);
     }
@@ -7738,7 +7599,7 @@ nsTableFrame::DumpTableFrames(nsIFrame* aFrame)
     tableFrame = NS_STATIC_CAST(nsTableFrame*, aFrame);
   }
   else {
-    nsTableFrame::GetTableFrame(aFrame, tableFrame);
+    tableFrame = nsTableFrame::GetTableFrame(aFrame);
   }
   tableFrame = NS_STATIC_CAST(nsTableFrame*, tableFrame->GetFirstInFlow());
   while (tableFrame) {

layout/tables/nsTableFrame.h

@@ -258,10 +258,7 @@ public:
   nsMargin GetContentAreaOffset(const nsHTMLReflowState* aReflowState) const;
 
   /** helper method to find the table parent of any table frame object */
-  // TODO: today, this depends on display types.  This should be changed to rely
+  static nsTableFrame* GetTableFrame(nsIFrame* aSourceFrame);
-  // on stronger criteria, like an inner table frame atom
-  static NS_METHOD GetTableFrame(nsIFrame*      aSourceFrame, 
-                                 nsTableFrame*& aTableFrame);
                                  
   /**
    * Helper method to handle display common to table frames, rowgroup frames
@@ -632,13 +629,21 @@ protected:
   nsresult RecoverState(nsTableReflowState& aReflowState,
                         nsIFrame*           aKidFrame);
 
-  NS_METHOD CollapseRowGroupIfNecessary(nsIFrame* aRowGroupFrame,
+  /** return the width of the table taking into account visibility collapse
-                                        const nscoord& aYTotalOffset,
+    * on columns and colgroups
-                                        nscoord& aYGroupOffset, PRInt32& aRowX);
+    * @param aBorderPadding  the border and padding of the table
+    */
+  nscoord GetCollapsedWidth(nsMargin aBorderPadding);
 
-  NS_METHOD AdjustForCollapsingRows(nsHTMLReflowMetrics&  aDesiredSize);
+  
+  /** Adjust the table for visibilty.collapse set on rowgroups, rows, colgroups
+    * and cols
+    * @param aDesiredSize    the metrics of the table
+    * @param aBorderPadding  the border and padding of the table
+    */
+  void AdjustForCollapsingRowsCols(nsHTMLReflowMetrics& aDesiredSize,
+                                   nsMargin             aBorderPadding);
 
-  NS_METHOD AdjustForCollapsingCols(nsHTMLReflowMetrics&  aDesiredSize);
   // end incremental reflow methods
 
 
@@ -752,12 +757,9 @@ public:
 
   PRBool NeedToCalcBCBorders() const;
   void SetNeedToCalcBCBorders(PRBool aValue);
-  
+
-  PRBool NeedToCollapseRows() const;
+  PRBool NeedToCollapse() const;
-  void SetNeedToCollapseRows(PRBool aValue);
+  void SetNeedToCollapse(PRBool aValue);
-  
-  PRBool NeedToCollapseColumns() const;
-  void SetNeedToCollapseColumns(PRBool aValue);
 
   /** Get the cell map for this table frame.  It is not always mCellMap.
     * Only the firstInFlow has a legit cell map
@@ -886,9 +888,8 @@ protected:
     PRUint32 mInitiatedSpecialReflow:1;
     PRUint32 mNeedToCalcBCBorders:1;
     PRUint32 mLeftContBCBorder:8;
-    PRUint32 mNeedToCollapseRows:1;    // rows that have visibility need to be collapse
+    PRUint32 mNeedToCollapse:1;    // rows, cols that have visibility:collapse need to be collapsed
-    PRUint32 mNeedToCollapseColumns:1; // colums that have visibility need to be collapsed
+    PRUint32 :10;                       // unused
-    PRUint32 :9;                       // unused
   } mBits;
 
   nsTableCellMap*         mCellMap;            // maintains the relationships between rows, cols, and cells
@@ -1006,24 +1007,14 @@ inline void nsTableFrame::SetRowInserted(PRBool aValue)
   mBits.mRowInserted = (unsigned)aValue;
 }
 
-inline void nsTableFrame::SetNeedToCollapseRows(PRBool aValue)
+inline void nsTableFrame::SetNeedToCollapse(PRBool aValue)
 {
-  mBits.mNeedToCollapseRows = (unsigned)aValue;
+  mBits.mNeedToCollapse = (unsigned)aValue;
 }
 
-inline PRBool nsTableFrame::NeedToCollapseRows() const
+inline PRBool nsTableFrame::NeedToCollapse() const
 {
-  return (PRBool)mBits.mNeedToCollapseRows;
+  return (PRBool)mBits.mNeedToCollapse;
-}
-
-inline void nsTableFrame::SetNeedToCollapseColumns(PRBool aValue)
-{
-  mBits.mNeedToCollapseColumns = (unsigned)aValue;
-}
-
-inline PRBool nsTableFrame::NeedToCollapseColumns() const
-{
-  return (PRBool)mBits.mNeedToCollapseColumns;
 }
 
 inline nsFrameList& nsTableFrame::GetColGroups()

layout/tables/nsTablePainter.cpp

@@ -409,27 +409,25 @@ TableBackgroundPainter::PaintTable(nsTableFrame* aTableFrame,
       PRBool cgDataOwnershipTaken = PR_FALSE;
       
       /*Loop over columns in this colgroup*/
-      if (cgData->IsVisible()) {
+      for (nsTableColFrame* col = cgFrame->GetFirstColumn(); col;
-        for (nsTableColFrame* col = cgFrame->GetFirstColumn(); col;
+           col = NS_STATIC_CAST(nsTableColFrame*, col->GetNextSibling())) {
-             col = NS_STATIC_CAST(nsTableColFrame*, col->GetNextSibling())) {
+        /*Create data struct for column*/
-          /*Create data struct for column*/
+        PRUint32 colIndex = col->GetColIndex();
-          PRUint32 colIndex = col->GetColIndex();
+        NS_ASSERTION(colIndex < mNumCols, "prevent array boundary violation");
-          NS_ASSERTION(colIndex < mNumCols, "prevent array boundary violation");
+        if (mNumCols <= colIndex)
-          if (mNumCols <= colIndex)
+          break;
-            break;
+        mCols[colIndex].mCol.SetFull(col);
-          mCols[colIndex].mCol.SetFull(col);
+        //Bring column mRect into table's coord system
-          //Bring column mRect into table's coord system
+        mCols[colIndex].mCol.mRect.MoveBy(cgData->mRect.x, cgData->mRect.y);
-          mCols[colIndex].mCol.mRect.MoveBy(cgData->mRect.x, cgData->mRect.y);
+        //link to parent colgroup's data
-          //link to parent colgroup's data
+        mCols[colIndex].mColGroup = cgData;
-          mCols[colIndex].mColGroup = cgData;
+        cgDataOwnershipTaken = PR_TRUE;
-          cgDataOwnershipTaken = PR_TRUE;
+        if (mIsBorderCollapse) {
-          if (mIsBorderCollapse) {
+          border.left = lastLeftBorder;
-            border.left = lastLeftBorder;
+          lastLeftBorder = col->GetContinuousBCBorderWidth(mP2t, border);
-            lastLeftBorder = col->GetContinuousBCBorderWidth(mP2t, border);
+          if (mCols[colIndex].mCol.ShouldSetBCBorder()) {
-            if (mCols[colIndex].mCol.ShouldSetBCBorder()) {
+            nsresult rv = mCols[colIndex].mCol.SetBCBorder(border, this);
-              nsresult rv = mCols[colIndex].mCol.SetBCBorder(border, this);
+            if (NS_FAILED(rv)) return rv;
-              if (NS_FAILED(rv)) return rv;
-            }
           }
         }
       }

layout/tables/nsTableRowFrame.cpp

@@ -217,8 +217,7 @@ nsTableRowFrame::AppendFrames(nsIAtom*        aListName,
   mFrames.AppendFrames(nsnull, aFrameList);
 
   // Add the new cell frames to the table
-  nsTableFrame *tableFrame = nsnull;
+  nsTableFrame *tableFrame =  nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame(this, tableFrame);
   for (nsIFrame* childFrame = aFrameList; childFrame;
        childFrame = childFrame->GetNextSibling()) {
     if (IS_TABLE_CELL(childFrame->GetType())) {
@@ -243,8 +242,7 @@ nsTableRowFrame::InsertFrames(nsIAtom*        aListName,
                               nsIFrame*       aFrameList)
 {
   // Get the table frame
-  nsTableFrame* tableFrame = nsnull;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame(this, tableFrame);
   
   // gather the new frames (only those which are cells) into an array
   nsIAtom* cellFrameType = (tableFrame->IsBorderCollapse()) ? nsLayoutAtoms::bcTableCellFrame : nsLayoutAtoms::tableCellFrame;
@@ -280,8 +278,7 @@ nsTableRowFrame::RemoveFrame(nsIAtom*        aListName,
                              nsIFrame*       aOldFrame)
 {
   // Get the table frame
-  nsTableFrame* tableFrame=nsnull;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame(this, tableFrame);
   if (tableFrame) {
     if (IS_TABLE_CELL(aOldFrame->GetType())) {
       nsTableCellFrame* cellFrame = (nsTableCellFrame*)aOldFrame;
@@ -347,9 +344,9 @@ void
 nsTableRowFrame::DidResize(const nsHTMLReflowState& aReflowState)
 {
   // Resize and re-align the cell frames based on our row height
-  nsTableFrame* tableFrame;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame(this, tableFrame);
+  if (!tableFrame)
-  if (!tableFrame) return;
+    return;
   
   nsTableIterator iter(*this, eTableDIR);
   nsIFrame* childFrame = iter.First();
@@ -517,9 +514,9 @@ nsTableRowFrame::UpdateHeight(nscoord           aHeight,
 nscoord
 nsTableRowFrame::CalcHeight(const nsHTMLReflowState& aReflowState)
 {
-  nsTableFrame* tableFrame = nsnull;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame(this, tableFrame);
+  if (!tableFrame)
-  if (!tableFrame) return 0;
+    return 0;
 
   nscoord computedHeight = (NS_UNCONSTRAINEDSIZE == aReflowState.mComputedHeight)
                             ? 0 : aReflowState.mComputedHeight;
@@ -589,9 +586,8 @@ private:
 void
 nsDisplayTableRowBackground::Paint(nsDisplayListBuilder* aBuilder,
     nsIRenderingContext* aCtx, const nsRect& aDirtyRect) {
-  nsTableFrame* tableFrame;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(mFrame);
-  nsTableFrame::GetTableFrame(mFrame, tableFrame);
+
-  NS_ASSERTION(tableFrame, "null table frame");
 
   nsPoint pt = aBuilder->ToReferenceFrame(mFrame);
   nsIRenderingContext::AutoPushTranslation translate(aCtx, pt.x, pt.y);
@@ -653,9 +649,9 @@ nsTableRowFrame::CalculateCellActualSize(nsIFrame* aCellFrame,
   // Get the height specified in the style information
   const nsStylePosition* position = aCellFrame->GetStylePosition();
 
-  nsTableFrame* tableFrame = nsnull;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame(this, tableFrame);
+  if (!tableFrame)
-  if (!tableFrame) return NS_ERROR_NULL_POINTER;
+    return NS_ERROR_NULL_POINTER;
   
   PRInt32 rowSpan = tableFrame->GetEffectiveRowSpan((nsTableCellFrame&)*aCellFrame);
   
@@ -752,14 +748,30 @@ GetSpaceBetween(PRInt32       aPrevColIndex,
                 PRInt32       aColSpan,
                 nsTableFrame& aTableFrame,
                 nscoord       aCellSpacingX,
-                PRBool        aIsLeftToRight)
+                PRBool        aIsLeftToRight,
+                PRBool        aCheckVisibility)
 {
   nscoord space = 0;
   PRInt32 colX;
   if (aIsLeftToRight) {
     for (colX = aPrevColIndex + 1; aColIndex > colX; colX++) {
-      space += aTableFrame.GetColumnWidth(colX);
+      PRBool isCollapsed = PR_FALSE;
-      if (aTableFrame.GetNumCellsOriginatingInCol(colX) > 0) {
+      if (!aCheckVisibility) {
+        space += aTableFrame.GetColumnWidth(colX);
+      }
+      else {
+        nsTableColFrame* colFrame = aTableFrame.GetColFrame(colX);
+        const nsStyleVisibility* colVis = colFrame->GetStyleVisibility();
+        PRBool collapseCol = (NS_STYLE_VISIBILITY_COLLAPSE == colVis->mVisible);
+        nsIFrame* cgFrame = colFrame->GetParent();
+        const nsStyleVisibility* groupVis = cgFrame->GetStyleVisibility();
+        PRBool collapseGroup = (NS_STYLE_VISIBILITY_COLLAPSE ==
+                                groupVis->mVisible);
+        isCollapsed = collapseCol || collapseGroup;
+        if (!isCollapsed)
+          space += aTableFrame.GetColumnWidth(colX);
+      }
+      if (!isCollapsed && (aTableFrame.GetNumCellsOriginatingInCol(colX) > 0)) {
         space += aCellSpacingX;
       }
     }
@@ -767,8 +779,23 @@ GetSpaceBetween(PRInt32       aPrevColIndex,
   else {
     PRInt32 lastCol = aColIndex + aColSpan - 1;
     for (colX = aPrevColIndex - 1; colX > lastCol; colX--) {
-      space += aTableFrame.GetColumnWidth(colX);
+      PRBool isCollapsed = PR_FALSE;
-      if (aTableFrame.GetNumCellsOriginatingInCol(colX) > 0) {
+      if (!aCheckVisibility) {
+        space += aTableFrame.GetColumnWidth(colX);
+      }
+      else {
+        nsTableColFrame* colFrame = aTableFrame.GetColFrame(colX);
+        const nsStyleVisibility* colVis = colFrame->GetStyleVisibility();
+        PRBool collapseCol = (NS_STYLE_VISIBILITY_COLLAPSE == colVis->mVisible);
+        nsIFrame* cgFrame = colFrame->GetParent();
+        const nsStyleVisibility* groupVis = cgFrame->GetStyleVisibility();
+        PRBool collapseGroup = (NS_STYLE_VISIBILITY_COLLAPSE ==
+                                groupVis->mVisible);
+        isCollapsed = collapseCol || collapseGroup;
+        if (!isCollapsed)
+          space += aTableFrame.GetColumnWidth(colX);
+      }
+      if (!isCollapsed && (aTableFrame.GetNumCellsOriginatingInCol(colX) > 0)) {
         space += aCellSpacingX;
       }
     }
@@ -898,7 +925,7 @@ nsTableRowFrame::ReflowChildren(nsPresContext*          aPresContext,
         if ((iter.IsLeftToRight() && (prevColIndex != (cellColIndex - 1))) ||
             (!iter.IsLeftToRight() && (prevColIndex != cellColIndex + cellColSpan))) {
           x += GetSpaceBetween(prevColIndex, cellColIndex, cellColSpan, aTableFrame, 
-                               cellSpacingX, iter.IsLeftToRight());
+                               cellSpacingX, iter.IsLeftToRight(), PR_FALSE);
         }
         // Calculate the available width for the table cell using the known column widths
         nscoord availColWidth, availCellWidth;
@@ -1046,7 +1073,7 @@ nsTableRowFrame::ReflowChildren(nsPresContext*          aPresContext,
         }
 
         FinishReflowChild(kidFrame, aPresContext, nsnull, desiredSize, x, 0, 0);
-        
+                
         x += desiredSize.width;  
       }
       else {// it's an unknown frame type, give it a generic reflow and ignore the results
@@ -1313,6 +1340,7 @@ nsTableRowFrame::IR_TargetIsChild(nsPresContext*          aPresContext,
 
     // Now place the child
     cellMet.width = colAvailWidth;
+
     FinishReflowChild(aNextFrame, aPresContext, nsnull, cellMet, cellOrigin.x, 0, 0);
 
     // Notify the table if the cell width changed so it can decide whether to rebalance
@@ -1396,14 +1424,14 @@ nsTableRowFrame::Reflow(nsPresContext*          aPresContext,
 #endif
   nsresult rv = NS_OK;
 
-  nsTableFrame* tableFrame = nsnull;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  rv = nsTableFrame::GetTableFrame(this, tableFrame);
+  if (!tableFrame)
-  if (!tableFrame) return NS_ERROR_NULL_POINTER;
+    return NS_ERROR_NULL_POINTER;
 
   const nsStyleVisibility* rowVis = GetStyleVisibility();
   PRBool collapseRow = (NS_STYLE_VISIBILITY_COLLAPSE == rowVis->mVisible);
   if (collapseRow) {
-    tableFrame->SetNeedToCollapseRows(PR_TRUE);
+    tableFrame->SetNeedToCollapse(PR_TRUE);
   }
 
   // see if a special height reflow needs to occur due to having a pct height
@@ -1456,8 +1484,9 @@ nsTableRowFrame::ReflowCellFrame(nsPresContext*          aPresContext,
                                  nscoord                  aAvailableHeight,
                                  nsReflowStatus&          aStatus)
 {
-  nsTableFrame* tableFrame = nsnull;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame(this, tableFrame); if (!tableFrame) ABORT1(0);
+  if (!tableFrame)
+    ABORT1(NS_ERROR_NULL_POINTER);
 
   // Reflow the cell frame with the specified height. Use the existing width
   nsSize cellSize = aCellFrame->GetSize();
@@ -1488,6 +1517,147 @@ nsTableRowFrame::ReflowCellFrame(nsPresContext*          aPresContext,
   return desiredSize.height;
 }
 
+nscoord
+nsTableRowFrame::CollapseRowIfNecessary(nscoord aRowOffset,
+                                        nscoord aWidth,
+                                        PRBool  aCollapseGroup,
+                                        PRBool& aDidCollapse)
+{
+  const nsStyleVisibility* rowVis = GetStyleVisibility();
+  PRBool collapseRow = (NS_STYLE_VISIBILITY_COLLAPSE == rowVis->mVisible);
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
+  if (!tableFrame)
+      return 0;
+  if (collapseRow) {
+    tableFrame->SetNeedToCollapse(PR_TRUE);
+  }
+
+  PRInt32 rowIndex = GetRowIndex();
+  nsRect rowRect = GetRect();
+  rowRect.y -= aRowOffset;
+  rowRect.width  = aWidth;
+  nsRect overflowArea(0, 0, 0, 0);
+  nscoord shift = 0;
+  nscoord cellSpacingX = tableFrame->GetCellSpacingX();
+  nscoord cellSpacingY = tableFrame->GetCellSpacingY();
+  nsTableCellFrame* cellFrame = GetFirstCell();
+  
+  if (aCollapseGroup || collapseRow) {
+    aDidCollapse = PR_TRUE;
+    shift = rowRect.height + cellSpacingY;
+    while (cellFrame) {
+      nsRect cRect = cellFrame->GetRect();
+      cRect.height = 0;
+      cellFrame->SetRect(cRect);
+      cellFrame = cellFrame->GetNextCell();
+    }
+    rowRect.height = 0;
+  }
+  else { // row is not collapsed
+    nsTableIterator iter(*this, eTableDIR);
+    // remember the col index of the previous cell to handle rowspans into this
+    // row
+    PRInt32 firstPrevColIndex = (iter.IsLeftToRight()) ? -1 :
+                                tableFrame->GetColCount();
+    PRInt32 prevColIndex  = firstPrevColIndex;
+    nscoord x = 0; // running total of children x offset
+
+    PRInt32 colIncrement = iter.IsLeftToRight() ? 1 : -1;
+
+    //nscoord x = cellSpacingX;
+
+    nsIFrame* kidFrame = iter.First();
+    while (kidFrame) {
+      nsIAtom* frameType = kidFrame->GetType();
+      if (IS_TABLE_CELL(frameType)) {
+        nsTableCellFrame* cellFrame = (nsTableCellFrame*)kidFrame;
+        PRInt32 cellColIndex;
+        cellFrame->GetColIndex(cellColIndex);
+        PRInt32 cellColSpan = tableFrame->GetEffectiveColSpan(*cellFrame);
+
+        // If the adjacent cell is in a prior row (because of a rowspan) add in
+        // the space
+        if ((iter.IsLeftToRight() && (prevColIndex != (cellColIndex - 1))) ||
+            (!iter.IsLeftToRight() &&
+             (prevColIndex != cellColIndex + cellColSpan))) {
+          x += GetSpaceBetween(prevColIndex, cellColIndex, cellColSpan,
+                               *tableFrame, cellSpacingX, iter.IsLeftToRight(),
+                               PR_TRUE);
+        }
+        nsRect cRect(x, 0, 0,rowRect.height);
+
+        // remember the rightmost (ltr) or leftmost (rtl) column this cell
+        // spans into
+        prevColIndex = (iter.IsLeftToRight()) ?
+                       cellColIndex + (cellColSpan - 1) : cellColIndex;
+        PRInt32 actualColSpan = cellColSpan;
+        PRBool isVisible = PR_FALSE;
+        for (PRInt32 colX = cellColIndex; actualColSpan > 0;
+             colX += colIncrement, actualColSpan--) {
+
+          nsTableColFrame* colFrame = tableFrame->GetColFrame(colX);
+          const nsStyleVisibility* colVis = colFrame->GetStyleVisibility();
+          PRBool collapseCol = (NS_STYLE_VISIBILITY_COLLAPSE ==
+                                colVis->mVisible);
+          nsIFrame* cgFrame = colFrame->GetParent();
+          const nsStyleVisibility* groupVis = cgFrame->GetStyleVisibility();
+          PRBool collapseGroup = (NS_STYLE_VISIBILITY_COLLAPSE ==
+                                  groupVis->mVisible);
+          PRBool isCollapsed = collapseCol || collapseGroup;
+          if (isCollapsed) {
+            tableFrame->SetNeedToCollapse(PR_TRUE);
+          }
+          else {
+            cRect.width += tableFrame->GetColumnWidth(colX);
+            isVisible = PR_TRUE;
+          }
+          if (!isCollapsed &&  (actualColSpan > 1)) {
+            nsTableColFrame* nextColFrame =
+              tableFrame->GetColFrame(colX + colIncrement);
+            const nsStyleVisibility* nextColVis =
+              nextColFrame->GetStyleVisibility();
+            if ( (NS_STYLE_VISIBILITY_COLLAPSE != nextColVis->mVisible) &&
+                (tableFrame->GetNumCellsOriginatingInCol(colX + colIncrement) > 0)) {
+              cRect.width += cellSpacingX;
+            }
+          }
+        }
+        x += cRect.width;
+        if (isVisible)
+          x += cellSpacingX;
+        PRInt32 actualRowSpan = tableFrame->GetEffectiveRowSpan(*cellFrame);
+        nsTableRowFrame* rowFrame = GetNextRow();
+        for (actualRowSpan--; actualRowSpan > 0 && rowFrame; actualRowSpan--) {
+          const nsStyleVisibility* nextRowVis = rowFrame->GetStyleVisibility();
+          PRBool collapseNextRow = (NS_STYLE_VISIBILITY_COLLAPSE ==
+                                    nextRowVis->mVisible);
+          if (!collapseNextRow) {
+            nsRect nextRect = rowFrame->GetRect();
+            cRect.height += nextRect.height + cellSpacingY;
+          }
+          rowFrame = rowFrame->GetNextRow();
+        }
+        cellFrame->SetRect(cRect);
+        nsRect cellOverflow = nsRect(0, 0, cRect.width, cRect.height);
+        cellFrame->FinishAndStoreOverflow(&cellOverflow, nsSize(cRect.width,
+                                              cRect.height));
+        nsTableFrame::RePositionViews(cellFrame);
+        ConsiderChildOverflow(overflowArea, cellFrame);
+        
+      }
+      kidFrame = iter.Next(); // Get the next child
+    }
+  }
+  SetRect(rowRect);
+  overflowArea.UnionRect(nsRect(0,0,rowRect.width, rowRect.height),
+                         overflowArea);
+  FinishAndStoreOverflow(&overflowArea, nsSize(rowRect.width,
+                                              rowRect.height));
+  nsTableFrame::RePositionViews(this);
+  return shift;
+}
+
+
 /**
  * These 3 functions are called by the row group frame's SplitRowGroup() code when
  * it creates a continuing cell frame and wants to insert it into the row's child list

layout/tables/nsTableRowFrame.h

@@ -164,6 +164,20 @@ public:
                           nsTableCellFrame*        aCellFrame,
                           nscoord                  aAvailableHeight,
                           nsReflowStatus&          aStatus);
+  /**
+    * Collapse the row if required, apply col and colgroup visibility: collapse
+    * info to the cells in the row.
+    * @return he amount to shift up all following rows
+    * @param aRowOffset     - shift the row up by this amount
+    * @param aWidth         - new width of the row
+    * @param aCollapseGroup - parent rowgroup is collapsed so this row needs
+    *                         to be collapsed
+    * @param aDidCollapse   - the row has been collapsed
+    */
+  nscoord CollapseRowIfNecessary(nscoord aRowOffset,
+                                 nscoord aWidth,
+                                 PRBool  aCollapseGroup,
+                                 PRBool& aDidCollapse);
 
   void InsertCellFrame(nsTableCellFrame* aFrame, 
                        nsTableCellFrame* aPrevSibling);

layout/tables/nsTableRowGroupFrame.cpp

@@ -143,8 +143,7 @@ PRInt32 nsTableRowGroupFrame::GetStartRowIndex()
   }
   // if the row group doesn't have any children, get it the hard way
   if (-1 == result) {
-    nsTableFrame* tableFrame;
+    nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-    nsTableFrame::GetTableFrame(this, tableFrame);
     if (tableFrame) {
       return tableFrame->GetStartRowIndex(*this);
     }
@@ -192,9 +191,7 @@ static void
 PaintRowGroupBackground(nsIFrame* aFrame, nsIRenderingContext* aCtx,
                         const nsRect& aDirtyRect, nsPoint aPt)
 {
-  nsTableFrame* tableFrame;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(aFrame);
-  nsTableFrame::GetTableFrame(aFrame, tableFrame);
-  NS_ASSERTION(tableFrame, "null table frame");
 
   nsIRenderingContext::AutoPushTranslation translate(aCtx, aPt.x, aPt.y);
   TableBackgroundPainter painter(tableFrame,
@@ -292,8 +289,11 @@ nsTableRowGroupFrame::ReflowChildren(nsPresContext*        aPresContext,
   if (aPageBreakBeforeEnd) 
     *aPageBreakBeforeEnd = PR_FALSE;
 
-  nsTableFrame* tableFrame = nsnull;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsresult rv = nsTableFrame::GetTableFrame(this, tableFrame); if (!tableFrame) ABORT1(rv);
+  if (!tableFrame)
+    ABORT1(NS_ERROR_NULL_POINTER);
+
+  nsresult rv = NS_OK;
 
   PRBool borderCollapse = tableFrame->IsBorderCollapse();
   GET_PIXELS_TO_TWIPS(aPresContext, p2t);
@@ -493,8 +493,7 @@ nsTableRowGroupFrame::CalculateRowHeights(nsPresContext*          aPresContext,
                                           const nsHTMLReflowState& aReflowState,
                                           nsTableRowFrame*         aStartRowFrameIn)
 {
-  nsTableFrame* tableFrame = nsnull;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame(this, tableFrame);
   if (!tableFrame) return;
 
   PRBool isPaginated = aPresContext->IsPaginated();
@@ -783,6 +782,48 @@ nsTableRowGroupFrame::CalculateRowHeights(nsPresContext*          aPresContext,
   delete [] rowInfo; // cleanup
 }
 
+nscoord
+nsTableRowGroupFrame::CollapseRowGroupIfNecessary(nscoord aYTotalOffset,
+                                                  nscoord aWidth)
+{
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
+
+  const nsStyleVisibility* groupVis = GetStyleVisibility();
+  PRBool collapseGroup = (NS_STYLE_VISIBILITY_COLLAPSE == groupVis->mVisible);
+  if (collapseGroup) {
+    tableFrame->SetNeedToCollapse(PR_TRUE);
+  }
+
+  nsRect overflowArea(0, 0, 0, 0);
+
+  nsTableRowFrame* rowFrame= GetFirstRow();
+  PRBool didCollapse = PR_FALSE;
+  nscoord yGroupOffset = 0;
+  while (rowFrame) {
+    yGroupOffset += rowFrame->CollapseRowIfNecessary(yGroupOffset,
+                                                     aWidth, collapseGroup,
+                                                     didCollapse);
+    ConsiderChildOverflow(overflowArea, rowFrame);
+    rowFrame = rowFrame->GetNextRow();
+  }
+
+  nsRect groupRect = GetRect();
+  groupRect.height -= yGroupOffset;
+  if (didCollapse) {
+    // add back the cellspacing between rowgroups
+    groupRect.height += tableFrame->GetCellSpacingY();
+  }
+
+  groupRect.y -= aYTotalOffset;
+  groupRect.width = aWidth;
+  SetRect(groupRect);
+  overflowArea.UnionRect(nsRect(0, 0, groupRect.width, groupRect.height),
+                         overflowArea);
+  FinishAndStoreOverflow(&overflowArea, nsSize(groupRect.width,
+                                              groupRect.height));
+  nsTableFrame::RePositionViews(this);
+  return yGroupOffset;
+}
 
 // Called by IR_TargetIsChild() to adjust the sibling frames that follow
 // after an incremental reflow of aKidFrame.
@@ -1189,8 +1230,7 @@ nsTableRowGroupFrame::Reflow(nsPresContext*          aPresContext,
         
   PRBool isPaginated = aPresContext->IsPaginated();
 
-  nsTableFrame* tableFrame = nsnull;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  rv = nsTableFrame::GetTableFrame(this, tableFrame);
   if (!tableFrame) return NS_ERROR_NULL_POINTER;
 
   // see if a special height reflow needs to occur due to having a pct height
@@ -1202,7 +1242,7 @@ nsTableRowGroupFrame::Reflow(nsPresContext*          aPresContext,
   const nsStyleVisibility* groupVis = GetStyleVisibility();
   PRBool collapseGroup = (NS_STYLE_VISIBILITY_COLLAPSE == groupVis->mVisible);
   if (collapseGroup) {
-    tableFrame->SetNeedToCollapseRows(PR_TRUE);
+    tableFrame->SetNeedToCollapse(PR_TRUE);
   }
 
   if (eReflowReason_Incremental == aReflowState.reason) {
@@ -1323,8 +1363,7 @@ nsTableRowGroupFrame::AppendFrames(nsIAtom*        aListName,
   mFrames.AppendFrames(nsnull, aFrameList);
 
   if (rows.Count() > 0) {
-    nsTableFrame* tableFrame = nsnull;
+    nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-    nsTableFrame::GetTableFrame(this, tableFrame);
     if (tableFrame) {
       tableFrame->AppendRows(*this, rowIndex, rows);
       // Reflow the new frames. They're already marked dirty, so generate a reflow
@@ -1354,12 +1393,10 @@ nsTableRowGroupFrame::InsertFrames(nsIAtom*        aListName,
                                    nsIFrame*       aPrevFrame,
                                    nsIFrame*       aFrameList)
 {
-  nsTableFrame* tableFrame = nsnull;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame(this, tableFrame);
+  if (!tableFrame)
-  if (!tableFrame) {
-    NS_ASSERTION(PR_FALSE, "no table frame");
     return NS_ERROR_NULL_POINTER;
-  }
+
   // collect the new row frames in an array
   nsVoidArray rows;
   PRBool gotFirstRow = PR_FALSE;
@@ -1410,8 +1447,7 @@ NS_IMETHODIMP
 nsTableRowGroupFrame::RemoveFrame(nsIAtom*        aListName,
                                   nsIFrame*       aOldFrame)
 {
-  nsTableFrame* tableFrame = nsnull;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame(this, tableFrame);
   if (tableFrame) {
     if (nsLayoutAtoms::tableRowFrame == aOldFrame->GetType()) {
       // remove the rows from the table (and flag a rebalance)
@@ -1474,8 +1510,7 @@ nscoord
 nsTableRowGroupFrame::GetHeightBasis(const nsHTMLReflowState& aReflowState)
 {
   nscoord result = 0;
-  nsTableFrame* tableFrame = nsnull;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame((nsIFrame*)this, tableFrame);
   if (tableFrame) {
     if ((aReflowState.mComputedHeight > 0) && (aReflowState.mComputedHeight < NS_UNCONSTRAINEDSIZE)) {
       nscoord cellSpacing = PR_MAX(0, GetRowCount() - 1) * tableFrame->GetCellSpacingY();
@@ -1500,8 +1535,7 @@ nsTableRowGroupFrame::GetHeightBasis(const nsHTMLReflowState& aReflowState)
 nscoord 
 nsTableRowGroupFrame::GetHeightOfRows()
 {
-  nsTableFrame* tableFrame = nsnull;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame(this, tableFrame);
   if (!tableFrame) return 0;
 
   nscoord height = 0;
@@ -1529,8 +1563,7 @@ nsresult
 nsTableRowGroupFrame::RecoverState(nsRowGroupReflowState& aReflowState,
                                    nsIFrame*              aKidFrame)
 {
-  nsTableFrame* tableFrame = nsnull;
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame(this, tableFrame);
   nscoord cellSpacingY = tableFrame->GetCellSpacingY();
 
   aReflowState.y = 0;
@@ -1596,8 +1629,9 @@ nsTableRowGroupFrame::IR_TargetIsChild(nsPresContext*        aPresContext,
 {
   nsresult rv;
   
-  nsTableFrame* tableFrame = nsnull; 
+  nsTableFrame* tableFrame = nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame(this, tableFrame); if (!tableFrame) ABORT1(NS_ERROR_NULL_POINTER);
+  if (!tableFrame)
+    ABORT1(NS_ERROR_NULL_POINTER);
   GET_PIXELS_TO_TWIPS(aPresContext, p2t);
 
   // Recover the state as if aNextFrame is about to be reflowed
@@ -1836,8 +1870,8 @@ nsTableRowGroupFrame::GetLine(PRInt32    aLineNumber,
   NS_ENSURE_ARG_POINTER(aNumFramesOnLine);
   NS_ENSURE_ARG_POINTER(aLineFlags);
 
-  nsTableFrame* parentFrame = nsnull; 
+  nsTableFrame* parentFrame = nsTableFrame::GetTableFrame(this);
-  if (NS_FAILED(nsTableFrame::GetTableFrame(this, parentFrame)))
+  if (!parentFrame)
     return NS_ERROR_FAILURE;
 
   nsTableCellMap* cellMap = parentFrame->GetCellMap();
@@ -1922,8 +1956,10 @@ nsTableRowGroupFrame::FindFrameAt(PRInt32    aLineNumber,
   CellData* cellData;
   nsIFrame* tempFrame = nsnull;
 
-  nsTableFrame* parentFrame = nsnull;
+  nsTableFrame* parentFrame = nsTableFrame::GetTableFrame(this);
-  nsTableFrame::GetTableFrame(this, parentFrame);
+  if (!parentFrame)
+    return NS_ERROR_FAILURE;
+
   nsTableCellMap* cellMap = parentFrame->GetCellMap();
   if (!cellMap)
      return NS_ERROR_FAILURE;
@@ -1992,8 +2028,9 @@ nsTableRowGroupFrame::GetNextSiblingOnLine(nsIFrame*& aFrame,
   if (NS_FAILED(result))
     return result;
 
-  nsTableFrame* parentFrame = nsnull;
+  nsTableFrame* parentFrame = nsTableFrame::GetTableFrame(this);
-  result = nsTableFrame::GetTableFrame(this, parentFrame);
+  if (!parentFrame)
+    return NS_ERROR_FAILURE;
   nsTableCellMap* cellMap = parentFrame->GetCellMap();
   if (!cellMap)
      return NS_ERROR_FAILURE;

layout/tables/nsTableRowGroupFrame.h

@@ -207,6 +207,17 @@ public:
    */
   void SetContinuousBCBorderWidth(PRUint8     aForSide,
                                   BCPixelSize aPixelValue);
+  /**
+    * Adjust to the effect of visibibility:collapse on the row group and
+    * its children
+    * @return              additional shift upward that should be applied to
+    *                      subsequent rowgroups due to rows and this rowgroup
+    *                      being collapsed
+    * @param aYTotalOffset the total amount that the rowgroup is shifted up
+    * @param aWidth        new width of the rowgroup
+    */
+  nscoord CollapseRowGroupIfNecessary(nscoord aYTotalOffset,
+                                      nscoord        aWidth);
 
 // nsILineIterator methods
 public: