improved colspan handling.

cells with colspans and specified widths divide their width provisionally between the columns they span. This provisional division is ammended by any cell in any of the effected columns that:
1) has a colspan=1, and
2) has a width specified
the widths of all other columns must be adjusted for cells like these.  Further complicating matters is that it seems <faith> that if multiple colspanning cells intersect a column and each has a width
specified such that the column would have a different width depending on which is used, only the first such cell is used </faith>.  I can neither confirm nor deny this reading the mozilla code.

I also taught the cell map how to deal with this situation:

 colspan=1 | colspan=2 | colspan=1
 colspan=1 | colspan=2 | colspan=1

This table "really" has 4 columns, but the middle cells are treated as if they have no colspan.
Trust me, it matters.  The original colspan attributes can't simply be thrown away because Mr. DOM
could come along at any time and add/remove a cell that would make them important.

layout/html/table/src/BasicTableLayoutStrategy.cpp

@@ -253,6 +253,7 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
   if (gsDebug==PR_TRUE) printf ("** %p: AssignPreliminaryColumnWidths **\n", mTableFrame);
   nsVoidArray *spanList=nsnull;
   nsVoidArray *colSpanList=nsnull;
+  PRInt32 numProvisionalFixedColumns = 0; // used for assigning width from cell with colspan to columns that are spanned
 
   PRBool hasColsAttribute = (PRBool)(NS_STYLE_TABLE_COLS_NONE!=mCols);
   
@@ -291,6 +292,8 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
     {
       haveColWidth = PR_TRUE;
       specifiedFixedColWidth = colPosition->mWidth.GetCoordValue();
+      if (nsTableColFrame::eWIDTH_SOURCE_CELL_WITH_SPAN==colFrame->GetWidthSource())
+        numProvisionalFixedColumns++;
     }
 
     /* Scan the column, simulatneously assigning column widths
@@ -341,7 +344,8 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
                 rowIndex, colSpan, cellMinSize.width, cellMinSize.height,
                 cellDesiredSize.width, cellDesiredSize.height);
 
-      if (PR_TRUE==haveColWidth)//  &&  PR_TRUE==cellGrantingWidth) 
+      if ((PR_TRUE==haveColWidth) && 
+          (nsTableColFrame::eWIDTH_SOURCE_CELL_WITH_SPAN!=colFrame->GetWidthSource()))
       {
         // This col has a specified fixed width so set the min and max width to the larger of 
         // (specified width, largest max_element_size of the cells in the column)
@@ -380,16 +384,6 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
           effectiveMinColumnWidth = cellMinWidth;
       }
 
-      //bookkeeping:  is this the cell that gave the column it's fixed width attribute?
-      // must be done after "haveColWidth && cellGrantingWidth" used above
-      if (PR_TRUE==cellGrantingWidth  && 1==colSpan)
-      {
-        const nsStylePosition* cellPosition;
-        cellFrame->GetStyleData(eStyleStruct_Position, (const nsStyleStruct *&)cellPosition);
-        if (eStyleUnit_Coord == cellPosition->mWidth.GetUnit())
-          cellGrantingWidth=PR_FALSE; //I've found the cell that gave the col it's width
-      }        
-
       if (1<colSpan)
       {
         // add the column to our list of post-process columns, if all of the intersected columns are auto
@@ -404,7 +398,8 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
             mTableFrame->GetColumnFrame(i+colIndex, cf);
             const nsStylePosition* colPos;
             cf->GetStyleData(eStyleStruct_Position, (nsStyleStruct*&)colPos);
-            if (colPos->mWidth.GetUnit() != eStyleUnit_Auto)
+            if (colPos->mWidth.GetUnit() != eStyleUnit_Auto &&
+               (nsTableColFrame::eWIDTH_SOURCE_CELL_WITH_SPAN!=cf->GetWidthSource()))
             {
               okToAdd = PR_FALSE;
               break;
@@ -412,20 +407,39 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
           }
         }
 
+        nscoord width = cellDesiredSize.width;  // used below as the cell's "natural" width
+        if (PR_TRUE == haveColWidth)
+          width = colSpan*specifiedFixedColWidth;
+
         if (PR_TRUE==okToAdd)
         {
-          ColSpanStruct *colSpanInfo = new ColSpanStruct(colSpan, colIndex, cellDesiredSize.width);
+          ColSpanStruct *colSpanInfo = new ColSpanStruct(colSpan, colIndex, width);
           if (nsnull==colSpanList)
             colSpanList = new nsVoidArray();
           colSpanList->AppendElement(colSpanInfo);
         }
 
-        // add the cell to our list of spanning cells
-        SpanInfo *spanInfo = new SpanInfo(colIndex, colSpan, cellMinWidth, cellDesiredWidth);
-        if (nsnull==spanList)
-          spanList = new nsVoidArray();
-        spanList->AppendElement(spanInfo);
+        if (PR_TRUE==cellGrantingWidth)
+        {
+          // add the cell to our list of spanning cells
+          SpanInfo *spanInfo = new SpanInfo(colIndex, colSpan, cellMinWidth, width);
+          if (nsnull==spanList)
+            spanList = new nsVoidArray();
+          spanList->AppendElement(spanInfo);
+        }
       }
+
+      //bookkeeping:  is this the cell that gave the column it's fixed width attribute?
+      // must be done after "haveColWidth && cellGrantingWidth" used above
+      if (PR_TRUE==cellGrantingWidth)
+      {
+        const nsStylePosition* cellPosition;
+        cellFrame->GetStyleData(eStyleStruct_Position, (const nsStyleStruct *&)cellPosition);
+        if (eStyleUnit_Coord == cellPosition->mWidth.GetUnit())
+          cellGrantingWidth=PR_FALSE; //I've found the cell that gave the col it's width
+      } 
+
+      // book 'em, Danno
       if (gsDebug) {
         printf ("    after cell %d, minColWidth = %d and maxColWidth = %d\n",
                 rowIndex, minColWidth, maxColWidth);
@@ -454,7 +468,8 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
     colFrame->SetEffectiveMaxColWidth(effectiveMaxColumnWidth);
     // this is the default, the real adjustment happens below where we deal with colspans
     colFrame->SetAdjustedMinColWidth(effectiveMinColumnWidth);  
-    if (PR_TRUE==haveColWidth)
+    if ((PR_TRUE==haveColWidth) && 
+        (nsTableColFrame::eWIDTH_SOURCE_CELL_WITH_SPAN!=colFrame->GetWidthSource()))
       mTableFrame->SetColumnWidth(colIndex, specifiedFixedColWidth);
     else
       mTableFrame->SetColumnWidth(colIndex, effectiveMaxColumnWidth);
@@ -467,8 +482,8 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
       maxColWidthArray[colIndex] = maxColWidth;
     }
     if (gsDebug==PR_TRUE) 
-      printf ("after col %d, minColWidth=%d effectiveMinColumnWidth=%d\n\tminTableWidth = %d and maxTableWidth = %d\n", 
-              colIndex, minColWidth, effectiveMinColumnWidth, mMinTableWidth, mMaxTableWidth);
+      printf ("after col %d, minColWidth=%d effectiveMinColumnWidth=%d\n\teffectiveMaxColumnWidth = %d\n", 
+              colIndex, minColWidth, effectiveMinColumnWidth, effectiveMaxColumnWidth);
   }
 
   // now, post-process the computed values based on the table attributes
@@ -479,10 +494,11 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
   if (nsnull!=spanList)
   {
     // we only want to do this if there are auto-cells involved
+    // or if we have columns that are provisionally fixed-width with colspans
     PRInt32 numAutoColumns=0;
     PRInt32 *autoColumns=nsnull;
     mTableFrame->GetColumnsByType(eStyleUnit_Auto, numAutoColumns, autoColumns);
-    if (0==numAutoColumns)
+    if (0==numAutoColumns && 0==numProvisionalFixedColumns)
     { //table fully specified, so no need to do any extra work here
       PRInt32 spanCount = spanList->Count();
       // go through the list backwards so we can delete easily
@@ -494,21 +510,21 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
       }
     }
     else
-    {
+    { // for every column, handle spanning cells that impact that column
       for (PRInt32 colIndex=0; colIndex<mNumCols; colIndex++)
       {
         if (gsDebug) printf("handling span for %d\n", colIndex);
         PRInt32 spanCount = spanList->Count();
         // go through the list backwards so we can delete easily
         for (PRInt32 spanIndex=spanCount-1; 0<=spanIndex; spanIndex--)
-        {
+        { // get each spanInfo struct and see if it impacts this column
           SpanInfo *spanInfo = (SpanInfo *)(spanList->ElementAt(spanIndex));
           // if the spanInfo is about a column before the current column, it effects 
           // the current column (otherwise it would have already been deleted.)
           if (spanInfo->initialColIndex <= colIndex)
           {
             if (0==spanInfo->effectiveMaxWidthOfSpannedCols)
-            {
+            { // if we have not yet computed effectiveMaxWidthOfSpannedCols, do it now
               for (PRInt32 span=0; span<spanInfo->initialColSpan; span++)
               {
                 nsTableColFrame *nextColFrame = mTableFrame->GetColFrame(colIndex+span);
@@ -550,10 +566,13 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
             nscoord spanCellMaxWidth;
             if (0!=spanInfo->effectiveMaxWidthOfSpannedCols)
             {
-              spanCellMaxWidth = (spanInfo->cellDesiredWidth * colFrame->GetEffectiveMaxColWidth()) /
-                                 (spanInfo->effectiveMaxWidthOfSpannedCols);
+              float percent = ((float)(colFrame->GetEffectiveMaxColWidth())) /
+                                 ((float)(spanInfo->effectiveMaxWidthOfSpannedCols));
+              spanCellMaxWidth = (nscoord)(((float)(spanInfo->cellDesiredWidth)) * percent);
               if (gsDebug==PR_TRUE) 
-                printf ("spanCellMaxWidth portion = %d\n", spanCellMaxWidth);
+                printf ("spanCellMaxWidth portion = %d with percent = %f from effMaxColW=%d and sum=%d\n", 
+                        spanCellMaxWidth, percent, colFrame->GetEffectiveMaxColWidth(),
+                        spanInfo->effectiveMaxWidthOfSpannedCols);
               if (colMaxWidth < spanCellMaxWidth)
               {
                 // make sure we're at least as big as our min
@@ -1561,7 +1580,7 @@ void BasicTableLayoutStrategy::DistributeExcessSpace(nscoord  aAvailWidth,
       nscoord excessForThisColumn = (nscoord)(excess*percent);
       nscoord colWidth = excessForThisColumn+oldColWidth;
       if (gsDebug==PR_TRUE) 
-        printf("  distribute excess to all columns:  column %d was %d, now set to %d from % = %f\n", 
+        printf("  distribute excess: column %d was %d, now %d from %=%f\n", 
                colIndex, oldColWidth, colWidth, percent);
       mTableFrame->SetColumnWidth(colIndex, colWidth);
     }

layout/html/table/src/nsCellMap.cpp

@@ -35,6 +35,7 @@ nsCellMap::nsCellMap(int aRows, int aColumns)
 {
   mCells = nsnull;
   mColFrames = nsnull;
+  mMinColSpans = nsnull;
   Reset(aRows, aColumns);
 }
 
@@ -57,10 +58,13 @@ nsCellMap::~nsCellMap()
     }
     delete [] mCells;
   }
-  if (nsnull!= mColFrames)
+  if (nsnull != mColFrames)
     delete mColFrames;
+  if (nsnull != mMinColSpans)
+    delete [] mMinColSpans;
   mCells = nsnull;
   mColFrames = nsnull;
+  mMinColSpans = nsnull;
 };
 
 
@@ -131,5 +135,32 @@ nsTableColFrame* nsCellMap::GetColumnFrame(PRInt32 aColIndex)
 }
 
 
+void nsCellMap::SetMinColSpan(PRInt32 aColIndex, PRBool aColSpan)
+{
+  NS_ASSERTION(aColIndex<mColCount, "bad aColIndex");
+  NS_ASSERTION(aColSpan>=0, "bad aColSpan");
+
+  // initialize the data structure if not already done
+  if (nsnull==mMinColSpans)
+  {
+    mMinColSpans = new PRInt32[mColCount];
+    for (PRInt32 i=0; i<mColCount; i++)
+      mMinColSpans[i]=1;
+  }
+
+  mMinColSpans[aColIndex] = aColSpan;
+}
+
+PRInt32 nsCellMap::GetMinColSpan(PRInt32 aColIndex)
+{
+  NS_ASSERTION(aColIndex<mColCount, "bad aColIndex");
+
+  PRInt32 result = 1; // default is 1, mMinColSpans need not be allocated for tables with no spans
+  if (nsnull!=mMinColSpans)
+    result = mMinColSpans[aColIndex];
+  return result;
+}
+
+
 
 

layout/html/table/src/nsCellMap.h

@@ -43,6 +43,9 @@ protected:
   /** storage for CellData pointers */
   PRInt32 *mCells;       ///XXX CellData *?
 
+  /** storage for CellData pointers */
+  PRInt32 *mMinColSpans;
+
   /** a cache of the column frames, by col index */
   nsVoidArray * mColFrames;
 
@@ -81,6 +84,9 @@ public:
 
   nsTableColFrame * GetColumnFrame(PRInt32 aColIndex);
 
+  void    SetMinColSpan(PRInt32 aColIndex, PRBool  aColSpan);
+  PRInt32 GetMinColSpan(PRInt32 aColIndex);
+
   void AppendColumnFrame(nsTableColFrame *aColFrame);
 
   PRBool RowImpactedBySpanningCell(PRInt32 aRowIndex);

layout/html/table/src/nsTableFrame.cpp

@@ -469,13 +469,24 @@ PRInt32 nsTableFrame::GetEffectiveColSpan (PRInt32 aColIndex, nsTableCellFrame *
   NS_PRECONDITION (nsnull!=cellMap, "bad call, cellMap not yet allocated.");
   NS_PRECONDITION (0<=aColIndex && aColIndex<GetColCount(), "bad col index arg");
 
+  PRInt32 result;
   if (cellMap->GetRowCount()==1)
     return 1;
   PRInt32 colSpan = aCell->GetColSpan();
   PRInt32 colCount = GetColCount();
   if (colCount < (aColIndex + colSpan))
-    return (colCount - aColIndex);
-  return colSpan;
+    result =  colCount - aColIndex;
+  else
+  {
+    result = colSpan;
+    // check for case where all cells in a column have a colspan
+    PRInt32 initialColIndex = aCell->GetColIndex();
+    PRInt32 minColSpanForCol = cellMap->GetMinColSpan(initialColIndex);
+    result -= (minColSpanForCol - 1); // minColSpanForCol is always at least 1
+                                      // and we want to treat default as 0 (no effect)
+  }
+
+  return result;
 }
 
 PRInt32 nsTableFrame::GetEffectiveCOLSAttribute()
@@ -691,8 +702,6 @@ void nsTableFrame::BuildCellMap ()
       PRInt32 colIndex = 0;
       nsIFrame* cellFrame;
       rowFrame->FirstChild(cellFrame);
-      if (gsDebug==PR_TRUE) 
-        DumpCellMap();
 
       while ((nsnull != cellFrame) && (colIndex < mColCount))
       {
@@ -745,6 +754,26 @@ void nsTableFrame::BuildCellMap ()
   }
   if (gsDebug==PR_TRUE)
     DumpCellMap ();
+  // iterate through the columns setting the min col span if necessary
+  // it would be more efficient if we could do this in the above loop
+  for (PRInt32 colIndex=0; colIndex<mColCount; colIndex++)
+  {
+    PRInt32 minColSpan;
+    for (PRInt32 rowIndex=0; rowIndex<rowCount; rowIndex++)
+    {
+      nsTableCellFrame *cellFrame = mCellMap->GetCellFrameAt(rowIndex, colIndex);
+      if (nsnull!=cellFrame)
+      {
+        PRInt32 colSpan = cellFrame->GetColSpan();
+        if (0==rowIndex)
+          minColSpan = colSpan;
+        else
+          minColSpan = PR_MIN(minColSpan, colSpan);
+      }
+    }
+    if (1!=minColSpan)
+      mCellMap->SetMinColSpan(colIndex, minColSpan);
+  }
 }
 
 /**
@@ -2529,27 +2558,31 @@ nsTableFrame::SetColumnStyleFromCell(nsIPresContext  * aPresContext,
         GetColumnFrame(i+aCellFrame->GetColIndex(), colFrame);
         if (nsTableColFrame::eWIDTH_SOURCE_CELL != colFrame->GetWidthSource()) 
         {
-          // get the column style and set the width attribute
-          nsIStyleContext *colSC;
-          colFrame->GetStyleContext(aPresContext, colSC);
-          nsStylePosition* colPosition = (nsStylePosition*) colSC->GetMutableStyleData(eStyleStruct_Position);
-          NS_RELEASE(colSC);
-          // set the column width attribute
-          if (eStyleUnit_Coord == cellPosition->mWidth.GetUnit())
+          if ((1==colSpan) ||
+              (nsTableColFrame::eWIDTH_SOURCE_CELL_WITH_SPAN != colFrame->GetWidthSource()))
           {
-            nscoord width = cellPosition->mWidth.GetCoordValue();
-            colPosition->mWidth.SetCoordValue(width/colSpan);
+            // get the column style and set the width attribute
+            nsIStyleContext *colSC;
+            colFrame->GetStyleContext(aPresContext, colSC);
+            nsStylePosition* colPosition = (nsStylePosition*) colSC->GetMutableStyleData(eStyleStruct_Position);
+            NS_RELEASE(colSC);
+            // set the column width attribute
+            if (eStyleUnit_Coord == cellPosition->mWidth.GetUnit())
+            {
+              nscoord width = cellPosition->mWidth.GetCoordValue();
+              colPosition->mWidth.SetCoordValue(width/colSpan);
+            }
+            else
+            {
+              float width = cellPosition->mWidth.GetPercentValue();
+              colPosition->mWidth.SetPercentValue(width/colSpan);
+            }
+            // set the column width-set-type
+            if (1==colSpan)
+              colFrame->SetWidthSource(nsTableColFrame::eWIDTH_SOURCE_CELL);
+            else
+              colFrame->SetWidthSource(nsTableColFrame::eWIDTH_SOURCE_CELL_WITH_SPAN);
           }
-          else
-          {
-            float width = cellPosition->mWidth.GetPercentValue();
-            colPosition->mWidth.SetPercentValue(width/colSpan);
-          }
-          // set the column width-set-type
-          if (1==colSpan)
-            colFrame->SetWidthSource(nsTableColFrame::eWIDTH_SOURCE_CELL);
-          else
-            colFrame->SetWidthSource(nsTableColFrame::eWIDTH_SOURCE_CELL_WITH_SPAN);
         }
       }
     }

layout/tables/BasicTableLayoutStrategy.cpp

@@ -253,6 +253,7 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
   if (gsDebug==PR_TRUE) printf ("** %p: AssignPreliminaryColumnWidths **\n", mTableFrame);
   nsVoidArray *spanList=nsnull;
   nsVoidArray *colSpanList=nsnull;
+  PRInt32 numProvisionalFixedColumns = 0; // used for assigning width from cell with colspan to columns that are spanned
 
   PRBool hasColsAttribute = (PRBool)(NS_STYLE_TABLE_COLS_NONE!=mCols);
   
@@ -291,6 +292,8 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
     {
       haveColWidth = PR_TRUE;
       specifiedFixedColWidth = colPosition->mWidth.GetCoordValue();
+      if (nsTableColFrame::eWIDTH_SOURCE_CELL_WITH_SPAN==colFrame->GetWidthSource())
+        numProvisionalFixedColumns++;
     }
 
     /* Scan the column, simulatneously assigning column widths
@@ -341,7 +344,8 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
                 rowIndex, colSpan, cellMinSize.width, cellMinSize.height,
                 cellDesiredSize.width, cellDesiredSize.height);
 
-      if (PR_TRUE==haveColWidth)//  &&  PR_TRUE==cellGrantingWidth) 
+      if ((PR_TRUE==haveColWidth) && 
+          (nsTableColFrame::eWIDTH_SOURCE_CELL_WITH_SPAN!=colFrame->GetWidthSource()))
       {
         // This col has a specified fixed width so set the min and max width to the larger of 
         // (specified width, largest max_element_size of the cells in the column)
@@ -380,16 +384,6 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
           effectiveMinColumnWidth = cellMinWidth;
       }
 
-      //bookkeeping:  is this the cell that gave the column it's fixed width attribute?
-      // must be done after "haveColWidth && cellGrantingWidth" used above
-      if (PR_TRUE==cellGrantingWidth  && 1==colSpan)
-      {
-        const nsStylePosition* cellPosition;
-        cellFrame->GetStyleData(eStyleStruct_Position, (const nsStyleStruct *&)cellPosition);
-        if (eStyleUnit_Coord == cellPosition->mWidth.GetUnit())
-          cellGrantingWidth=PR_FALSE; //I've found the cell that gave the col it's width
-      }        
-
       if (1<colSpan)
       {
         // add the column to our list of post-process columns, if all of the intersected columns are auto
@@ -404,7 +398,8 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
             mTableFrame->GetColumnFrame(i+colIndex, cf);
             const nsStylePosition* colPos;
             cf->GetStyleData(eStyleStruct_Position, (nsStyleStruct*&)colPos);
-            if (colPos->mWidth.GetUnit() != eStyleUnit_Auto)
+            if (colPos->mWidth.GetUnit() != eStyleUnit_Auto &&
+               (nsTableColFrame::eWIDTH_SOURCE_CELL_WITH_SPAN!=cf->GetWidthSource()))
             {
               okToAdd = PR_FALSE;
               break;
@@ -412,20 +407,39 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
           }
         }
 
+        nscoord width = cellDesiredSize.width;  // used below as the cell's "natural" width
+        if (PR_TRUE == haveColWidth)
+          width = colSpan*specifiedFixedColWidth;
+
         if (PR_TRUE==okToAdd)
         {
-          ColSpanStruct *colSpanInfo = new ColSpanStruct(colSpan, colIndex, cellDesiredSize.width);
+          ColSpanStruct *colSpanInfo = new ColSpanStruct(colSpan, colIndex, width);
           if (nsnull==colSpanList)
             colSpanList = new nsVoidArray();
           colSpanList->AppendElement(colSpanInfo);
         }
 
-        // add the cell to our list of spanning cells
-        SpanInfo *spanInfo = new SpanInfo(colIndex, colSpan, cellMinWidth, cellDesiredWidth);
-        if (nsnull==spanList)
-          spanList = new nsVoidArray();
-        spanList->AppendElement(spanInfo);
+        if (PR_TRUE==cellGrantingWidth)
+        {
+          // add the cell to our list of spanning cells
+          SpanInfo *spanInfo = new SpanInfo(colIndex, colSpan, cellMinWidth, width);
+          if (nsnull==spanList)
+            spanList = new nsVoidArray();
+          spanList->AppendElement(spanInfo);
+        }
       }
+
+      //bookkeeping:  is this the cell that gave the column it's fixed width attribute?
+      // must be done after "haveColWidth && cellGrantingWidth" used above
+      if (PR_TRUE==cellGrantingWidth)
+      {
+        const nsStylePosition* cellPosition;
+        cellFrame->GetStyleData(eStyleStruct_Position, (const nsStyleStruct *&)cellPosition);
+        if (eStyleUnit_Coord == cellPosition->mWidth.GetUnit())
+          cellGrantingWidth=PR_FALSE; //I've found the cell that gave the col it's width
+      } 
+
+      // book 'em, Danno
       if (gsDebug) {
         printf ("    after cell %d, minColWidth = %d and maxColWidth = %d\n",
                 rowIndex, minColWidth, maxColWidth);
@@ -454,7 +468,8 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
     colFrame->SetEffectiveMaxColWidth(effectiveMaxColumnWidth);
     // this is the default, the real adjustment happens below where we deal with colspans
     colFrame->SetAdjustedMinColWidth(effectiveMinColumnWidth);  
-    if (PR_TRUE==haveColWidth)
+    if ((PR_TRUE==haveColWidth) && 
+        (nsTableColFrame::eWIDTH_SOURCE_CELL_WITH_SPAN!=colFrame->GetWidthSource()))
       mTableFrame->SetColumnWidth(colIndex, specifiedFixedColWidth);
     else
       mTableFrame->SetColumnWidth(colIndex, effectiveMaxColumnWidth);
@@ -467,8 +482,8 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
       maxColWidthArray[colIndex] = maxColWidth;
     }
     if (gsDebug==PR_TRUE) 
-      printf ("after col %d, minColWidth=%d effectiveMinColumnWidth=%d\n\tminTableWidth = %d and maxTableWidth = %d\n", 
-              colIndex, minColWidth, effectiveMinColumnWidth, mMinTableWidth, mMaxTableWidth);
+      printf ("after col %d, minColWidth=%d effectiveMinColumnWidth=%d\n\teffectiveMaxColumnWidth = %d\n", 
+              colIndex, minColWidth, effectiveMinColumnWidth, effectiveMaxColumnWidth);
   }
 
   // now, post-process the computed values based on the table attributes
@@ -479,10 +494,11 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
   if (nsnull!=spanList)
   {
     // we only want to do this if there are auto-cells involved
+    // or if we have columns that are provisionally fixed-width with colspans
     PRInt32 numAutoColumns=0;
     PRInt32 *autoColumns=nsnull;
     mTableFrame->GetColumnsByType(eStyleUnit_Auto, numAutoColumns, autoColumns);
-    if (0==numAutoColumns)
+    if (0==numAutoColumns && 0==numProvisionalFixedColumns)
     { //table fully specified, so no need to do any extra work here
       PRInt32 spanCount = spanList->Count();
       // go through the list backwards so we can delete easily
@@ -494,21 +510,21 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
       }
     }
     else
-    {
+    { // for every column, handle spanning cells that impact that column
       for (PRInt32 colIndex=0; colIndex<mNumCols; colIndex++)
       {
         if (gsDebug) printf("handling span for %d\n", colIndex);
         PRInt32 spanCount = spanList->Count();
         // go through the list backwards so we can delete easily
         for (PRInt32 spanIndex=spanCount-1; 0<=spanIndex; spanIndex--)
-        {
+        { // get each spanInfo struct and see if it impacts this column
           SpanInfo *spanInfo = (SpanInfo *)(spanList->ElementAt(spanIndex));
           // if the spanInfo is about a column before the current column, it effects 
           // the current column (otherwise it would have already been deleted.)
           if (spanInfo->initialColIndex <= colIndex)
           {
             if (0==spanInfo->effectiveMaxWidthOfSpannedCols)
-            {
+            { // if we have not yet computed effectiveMaxWidthOfSpannedCols, do it now
               for (PRInt32 span=0; span<spanInfo->initialColSpan; span++)
               {
                 nsTableColFrame *nextColFrame = mTableFrame->GetColFrame(colIndex+span);
@@ -550,10 +566,13 @@ PRBool BasicTableLayoutStrategy::AssignPreliminaryColumnWidths()
             nscoord spanCellMaxWidth;
             if (0!=spanInfo->effectiveMaxWidthOfSpannedCols)
             {
-              spanCellMaxWidth = (spanInfo->cellDesiredWidth * colFrame->GetEffectiveMaxColWidth()) /
-                                 (spanInfo->effectiveMaxWidthOfSpannedCols);
+              float percent = ((float)(colFrame->GetEffectiveMaxColWidth())) /
+                                 ((float)(spanInfo->effectiveMaxWidthOfSpannedCols));
+              spanCellMaxWidth = (nscoord)(((float)(spanInfo->cellDesiredWidth)) * percent);
               if (gsDebug==PR_TRUE) 
-                printf ("spanCellMaxWidth portion = %d\n", spanCellMaxWidth);
+                printf ("spanCellMaxWidth portion = %d with percent = %f from effMaxColW=%d and sum=%d\n", 
+                        spanCellMaxWidth, percent, colFrame->GetEffectiveMaxColWidth(),
+                        spanInfo->effectiveMaxWidthOfSpannedCols);
               if (colMaxWidth < spanCellMaxWidth)
               {
                 // make sure we're at least as big as our min
@@ -1561,7 +1580,7 @@ void BasicTableLayoutStrategy::DistributeExcessSpace(nscoord  aAvailWidth,
       nscoord excessForThisColumn = (nscoord)(excess*percent);
       nscoord colWidth = excessForThisColumn+oldColWidth;
       if (gsDebug==PR_TRUE) 
-        printf("  distribute excess to all columns:  column %d was %d, now set to %d from % = %f\n", 
+        printf("  distribute excess: column %d was %d, now %d from %=%f\n", 
                colIndex, oldColWidth, colWidth, percent);
       mTableFrame->SetColumnWidth(colIndex, colWidth);
     }

layout/tables/nsCellMap.cpp

@@ -35,6 +35,7 @@ nsCellMap::nsCellMap(int aRows, int aColumns)
 {
   mCells = nsnull;
   mColFrames = nsnull;
+  mMinColSpans = nsnull;
   Reset(aRows, aColumns);
 }
 
@@ -57,10 +58,13 @@ nsCellMap::~nsCellMap()
     }
     delete [] mCells;
   }
-  if (nsnull!= mColFrames)
+  if (nsnull != mColFrames)
     delete mColFrames;
+  if (nsnull != mMinColSpans)
+    delete [] mMinColSpans;
   mCells = nsnull;
   mColFrames = nsnull;
+  mMinColSpans = nsnull;
 };
 
 
@@ -131,5 +135,32 @@ nsTableColFrame* nsCellMap::GetColumnFrame(PRInt32 aColIndex)
 }
 
 
+void nsCellMap::SetMinColSpan(PRInt32 aColIndex, PRBool aColSpan)
+{
+  NS_ASSERTION(aColIndex<mColCount, "bad aColIndex");
+  NS_ASSERTION(aColSpan>=0, "bad aColSpan");
+
+  // initialize the data structure if not already done
+  if (nsnull==mMinColSpans)
+  {
+    mMinColSpans = new PRInt32[mColCount];
+    for (PRInt32 i=0; i<mColCount; i++)
+      mMinColSpans[i]=1;
+  }
+
+  mMinColSpans[aColIndex] = aColSpan;
+}
+
+PRInt32 nsCellMap::GetMinColSpan(PRInt32 aColIndex)
+{
+  NS_ASSERTION(aColIndex<mColCount, "bad aColIndex");
+
+  PRInt32 result = 1; // default is 1, mMinColSpans need not be allocated for tables with no spans
+  if (nsnull!=mMinColSpans)
+    result = mMinColSpans[aColIndex];
+  return result;
+}
+
+
 
 

layout/tables/nsCellMap.h

@@ -43,6 +43,9 @@ protected:
   /** storage for CellData pointers */
   PRInt32 *mCells;       ///XXX CellData *?
 
+  /** storage for CellData pointers */
+  PRInt32 *mMinColSpans;
+
   /** a cache of the column frames, by col index */
   nsVoidArray * mColFrames;
 
@@ -81,6 +84,9 @@ public:
 
   nsTableColFrame * GetColumnFrame(PRInt32 aColIndex);
 
+  void    SetMinColSpan(PRInt32 aColIndex, PRBool  aColSpan);
+  PRInt32 GetMinColSpan(PRInt32 aColIndex);
+
   void AppendColumnFrame(nsTableColFrame *aColFrame);
 
   PRBool RowImpactedBySpanningCell(PRInt32 aRowIndex);

layout/tables/nsTableFrame.cpp

@@ -469,13 +469,24 @@ PRInt32 nsTableFrame::GetEffectiveColSpan (PRInt32 aColIndex, nsTableCellFrame *
   NS_PRECONDITION (nsnull!=cellMap, "bad call, cellMap not yet allocated.");
   NS_PRECONDITION (0<=aColIndex && aColIndex<GetColCount(), "bad col index arg");
 
+  PRInt32 result;
   if (cellMap->GetRowCount()==1)
     return 1;
   PRInt32 colSpan = aCell->GetColSpan();
   PRInt32 colCount = GetColCount();
   if (colCount < (aColIndex + colSpan))
-    return (colCount - aColIndex);
-  return colSpan;
+    result =  colCount - aColIndex;
+  else
+  {
+    result = colSpan;
+    // check for case where all cells in a column have a colspan
+    PRInt32 initialColIndex = aCell->GetColIndex();
+    PRInt32 minColSpanForCol = cellMap->GetMinColSpan(initialColIndex);
+    result -= (minColSpanForCol - 1); // minColSpanForCol is always at least 1
+                                      // and we want to treat default as 0 (no effect)
+  }
+
+  return result;
 }
 
 PRInt32 nsTableFrame::GetEffectiveCOLSAttribute()
@@ -691,8 +702,6 @@ void nsTableFrame::BuildCellMap ()
       PRInt32 colIndex = 0;
       nsIFrame* cellFrame;
       rowFrame->FirstChild(cellFrame);
-      if (gsDebug==PR_TRUE) 
-        DumpCellMap();
 
       while ((nsnull != cellFrame) && (colIndex < mColCount))
       {
@@ -745,6 +754,26 @@ void nsTableFrame::BuildCellMap ()
   }
   if (gsDebug==PR_TRUE)
     DumpCellMap ();
+  // iterate through the columns setting the min col span if necessary
+  // it would be more efficient if we could do this in the above loop
+  for (PRInt32 colIndex=0; colIndex<mColCount; colIndex++)
+  {
+    PRInt32 minColSpan;
+    for (PRInt32 rowIndex=0; rowIndex<rowCount; rowIndex++)
+    {
+      nsTableCellFrame *cellFrame = mCellMap->GetCellFrameAt(rowIndex, colIndex);
+      if (nsnull!=cellFrame)
+      {
+        PRInt32 colSpan = cellFrame->GetColSpan();
+        if (0==rowIndex)
+          minColSpan = colSpan;
+        else
+          minColSpan = PR_MIN(minColSpan, colSpan);
+      }
+    }
+    if (1!=minColSpan)
+      mCellMap->SetMinColSpan(colIndex, minColSpan);
+  }
 }
 
 /**
@@ -2529,27 +2558,31 @@ nsTableFrame::SetColumnStyleFromCell(nsIPresContext  * aPresContext,
         GetColumnFrame(i+aCellFrame->GetColIndex(), colFrame);
         if (nsTableColFrame::eWIDTH_SOURCE_CELL != colFrame->GetWidthSource()) 
         {
-          // get the column style and set the width attribute
-          nsIStyleContext *colSC;
-          colFrame->GetStyleContext(aPresContext, colSC);
-          nsStylePosition* colPosition = (nsStylePosition*) colSC->GetMutableStyleData(eStyleStruct_Position);
-          NS_RELEASE(colSC);
-          // set the column width attribute
-          if (eStyleUnit_Coord == cellPosition->mWidth.GetUnit())
+          if ((1==colSpan) ||
+              (nsTableColFrame::eWIDTH_SOURCE_CELL_WITH_SPAN != colFrame->GetWidthSource()))
           {
-            nscoord width = cellPosition->mWidth.GetCoordValue();
-            colPosition->mWidth.SetCoordValue(width/colSpan);
+            // get the column style and set the width attribute
+            nsIStyleContext *colSC;
+            colFrame->GetStyleContext(aPresContext, colSC);
+            nsStylePosition* colPosition = (nsStylePosition*) colSC->GetMutableStyleData(eStyleStruct_Position);
+            NS_RELEASE(colSC);
+            // set the column width attribute
+            if (eStyleUnit_Coord == cellPosition->mWidth.GetUnit())
+            {
+              nscoord width = cellPosition->mWidth.GetCoordValue();
+              colPosition->mWidth.SetCoordValue(width/colSpan);
+            }
+            else
+            {
+              float width = cellPosition->mWidth.GetPercentValue();
+              colPosition->mWidth.SetPercentValue(width/colSpan);
+            }
+            // set the column width-set-type
+            if (1==colSpan)
+              colFrame->SetWidthSource(nsTableColFrame::eWIDTH_SOURCE_CELL);
+            else
+              colFrame->SetWidthSource(nsTableColFrame::eWIDTH_SOURCE_CELL_WITH_SPAN);
           }
-          else
-          {
-            float width = cellPosition->mWidth.GetPercentValue();
-            colPosition->mWidth.SetPercentValue(width/colSpan);
-          }
-          // set the column width-set-type
-          if (1==colSpan)
-            colFrame->SetWidthSource(nsTableColFrame::eWIDTH_SOURCE_CELL);
-          else
-            colFrame->SetWidthSource(nsTableColFrame::eWIDTH_SOURCE_CELL_WITH_SPAN);
         }
       }
     }