Bug 478956. Merge pseudo-frame handling for outer tables and non-table frames. r=bernd, sr=roc
authorBoris Zbarsky <bzbarsky@mit.edu>
Sun, 01 Mar 2009 10:16:29 -0500
changeset 25648 4acae1aa5b28ee3cf8aa4410b7e8018ca921cc3b
parent 25647 3bf433cc62cc81a9b982921d0b5eff98d20690cc
child 25649 159ff2e4c2ebd6c2aec52f5e6785e5a8a6f21b3a
push idunknown
push userunknown
push dateunknown
reviewersbernd, roc
bugs478956
milestone1.9.2a1pre
Bug 478956. Merge pseudo-frame handling for outer tables and non-table frames. r=bernd, sr=roc
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
layout/reftests/bugs/478956-1-ref.html
layout/reftests/bugs/478956-1a.html
layout/reftests/bugs/478956-1b.html
layout/reftests/bugs/reftest.list
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -2985,43 +2985,35 @@ nsCSSFrameConstructor::GetPseudoCellFram
       rv = CreatePseudoRowFrame(aNameSpaceID, aState);
     }
     rv = CreatePseudoCellFrame(aNameSpaceID, aState);
   }
   return rv;
 }
 
 nsresult 
-nsCSSFrameConstructor::GetParentFrame(PRInt32                  aNameSpaceID,
-                                      nsIFrame&                aParentFrameIn, 
-                                      nsIAtom*                 aChildFrameType, 
-                                      nsFrameConstructorState& aState, 
-                                      nsIFrame*&               aParentFrame,
-                                      PRBool&                  aIsPseudoParent)
+nsCSSFrameConstructor::CreateRequiredPseudoFrames(PRInt32                  aNameSpaceID,
+                                                  nsIFrame&                aParentFrameIn,
+                                                  nsIAtom*                 aChildFrameType,
+                                                  nsFrameConstructorState& aState,
+                                                  nsIFrame*&               aParentFrame,
+                                                  PRBool&                  aIsPseudoParent)
 {
   nsresult rv = NS_OK;
 
   nsIAtom* parentFrameType = aParentFrameIn.GetType();
   nsIFrame* pseudoParentFrame = nsnull;
   nsPseudoFrames& pseudoFrames = aState.mPseudoFrames;
   aParentFrame = &aParentFrameIn;
   aIsPseudoParent = PR_FALSE;
 
   nsFrameState savedStateBits  = aState.mAdditionalStateBits;
   aState.mAdditionalStateBits &= ~NS_FRAME_GENERATED_CONTENT;
 
-  if (nsGkAtoms::tableOuterFrame == aChildFrameType) { // table child
-    if (IsTableRelated(parentFrameType, PR_TRUE) &&
-        (nsGkAtoms::tableCaptionFrame != parentFrameType) ) { // need pseudo cell parent
-      rv = GetPseudoCellFrame(aNameSpaceID, aState, aParentFrameIn);
-      if (NS_FAILED(rv)) return rv;
-      pseudoParentFrame = pseudoFrames.mCellInner.mFrame;
-    }
-  } 
-  else if (nsGkAtoms::tableCaptionFrame == aChildFrameType) { // caption child
+  if (nsGkAtoms::tableCaptionFrame == aChildFrameType) { // caption child
     if (nsGkAtoms::tableOuterFrame != parentFrameType) { // need pseudo table parent
       rv = GetPseudoTableFrame(aNameSpaceID, aState, aParentFrameIn);
       if (NS_FAILED(rv)) return rv;
       pseudoParentFrame = pseudoFrames.mTableOuter.mFrame;
     }
   }
   else if (nsGkAtoms::tableColGroupFrame == aChildFrameType) { // col group child
     if (nsGkAtoms::tableFrame != parentFrameType) { // need pseudo table parent
@@ -3055,26 +3047,21 @@ nsCSSFrameConstructor::GetParentFrame(PR
   }
   else if (IS_TABLE_CELL(aChildFrameType)) { // cell child
     if (nsGkAtoms::tableRowFrame != parentFrameType) { // need pseudo row parent
       rv = GetPseudoRowFrame(aNameSpaceID, aState, aParentFrameIn);
       if (NS_FAILED(rv)) return rv;
       pseudoParentFrame = pseudoFrames.mRow.mFrame;
     }
   }
-  else if (nsGkAtoms::tableFrame == aChildFrameType) { // invalid
-    NS_ASSERTION(PR_FALSE, "GetParentFrame called on nsGkAtoms::tableFrame child");
-  }
-  else { // foreign frame
-    if (IsTableRelated(parentFrameType, PR_FALSE)) { // need pseudo cell parent
-      rv = GetPseudoCellFrame(aNameSpaceID, aState, aParentFrameIn);
-      if (NS_FAILED(rv)) return rv;
-      pseudoParentFrame = pseudoFrames.mCellInner.mFrame;
-    }
-  }
+#ifdef DEBUG
+  else {
+    NS_ERROR("Unexpected frame type in CreateRequiredPseudoFrames");
+  }
+#endif
   
   if (pseudoParentFrame) {
     aParentFrame = pseudoParentFrame;
     aIsPseudoParent = PR_TRUE;
   }
 
   aState.mAdditionalStateBits = savedStateBits;
   return rv;
@@ -3134,16 +3121,25 @@ nsCSSFrameConstructor::AdjustParentFrame
                  "Must have inner cell frame now!");
 
     aParentFrame = aState.mPseudoFrames.mCellInner.mFrame;
     aFrameItems = &aState.mPseudoFrames.mCellInner.mChildList;
     // We pushed an anonymous table cell.  The inner block of this
     // needs to become the float containing block.
     aState.PushFloatContainingBlock(aParentFrame, aSaveState);
     aCreatedPseudo = PR_TRUE;
+
+    // Now it might be that we had existing pseudo-frames and in particular an
+    // existing pseudo-cell (so that the pseudo cell we just got is not the
+    // lowest pseudo-frame).  If that's the case, we need to process everythign
+    // below that cell, so that our later siblings don't see those
+    // pseudo-frames.
+    if (aState.mPseudoFrames.mTableOuter.mFrame) {
+      ProcessPseudoFrames(aState, nsGkAtoms::tableOuterFrame);
+    }
   }
   return NS_OK;
 }
 
 // Pull all the captions present in aItems out  into aCaptions
 static void
 PullOutCaptionFrames(nsFrameItems& aItems, nsFrameItems& aCaptions)
 {
@@ -3189,40 +3185,23 @@ nsCSSFrameConstructor::ConstructTableFra
 #ifdef MOZ_MATHML
   if (kNameSpaceID_MathML == aNameSpaceID)
     aNewOuterFrame = NS_NewMathMLmtableOuterFrame(mPresShell,
                                                   outerStyleContext);
   else
 #endif
     aNewOuterFrame = NS_NewTableOuterFrame(mPresShell, outerStyleContext);
 
-  nsIFrame* parentFrame = aContentParent;
-  nsFrameItems* frameItems = &aChildItems;
-  // We may need to push a float containing block
-  nsFrameConstructorSaveState floatSaveState;
-  if (!aIsPseudo) {
-    // this frame may have a pseudo parent
-    PRBool hasPseudoParent = PR_FALSE;
-    GetParentFrame(aNameSpaceID,*parentFrame, nsGkAtoms::tableOuterFrame,
-                   aState, parentFrame, hasPseudoParent);
-    if (!hasPseudoParent && !aState.mPseudoFrames.IsEmpty()) {
-      ProcessPseudoFrames(aState, aChildItems);
-    }
-    if (hasPseudoParent) {
-      aState.PushFloatContainingBlock(parentFrame, floatSaveState);
-      frameItems = &aState.mPseudoFrames.mCellInner.mChildList;
-      if (aState.mPseudoFrames.mTableOuter.mFrame) {
-        ProcessPseudoFrames(aState, nsGkAtoms::tableOuterFrame);
-      }
-    }
-  }
+  NS_ASSERTION(!IsTableRelated(aContentParent->GetType(), PR_TRUE) ||
+               aContentParent->GetType() == nsGkAtoms::tableCaptionFrame,
+               "Unexpected parent frame for table");
 
   nsIFrame* geometricParent = aState.GetGeometricParent
                                 (outerStyleContext->GetStyleDisplay(),
-                                 parentFrame);
+                                 aContentParent);
 
   // Init the table outer frame and see if we need to create a view, e.g.
   // the frame is absolutely positioned  
   InitAndRestoreFrame(aState, aContent, geometricParent, nsnull, aNewOuterFrame);  
   nsHTMLContainerFrame::CreateViewForFrame(aNewOuterFrame, PR_FALSE);
 
   // Create the inner table frame
 #ifdef MOZ_MATHML
@@ -3234,18 +3213,18 @@ nsCSSFrameConstructor::ConstructTableFra
  
   InitAndRestoreFrame(aState, aContent, aNewOuterFrame, nsnull,
                       aNewInnerFrame);
 
   if (!aIsPseudo) {
     // Put the newly created frames into the right child list
     aNewOuterFrame->SetInitialChildList(nsnull, aNewInnerFrame);
 
-    rv = aState.AddChild(aNewOuterFrame, *frameItems, aContent,
-                         aStyleContext, parentFrame);
+    rv = aState.AddChild(aNewOuterFrame, aChildItems, aContent,
+                         aStyleContext, aContentParent);
     if (NS_FAILED(rv)) {
       return rv;
     }
 
     if (!mRootElementFrame) {
       // The frame we're constructing will be the root element frame.
       // Set mRootElementFrame before processing children.
       mRootElementFrame = aNewOuterFrame;
@@ -3284,19 +3263,19 @@ nsCSSFrameConstructor::ConstructTableCap
                                                   PRBool*                  aHasPseudoParent)
 
 {
   if (!aParentFrameIn) return NS_ERROR_UNEXPECTED;
 
   nsIFrame* parentFrame = aParentFrameIn;
   *aHasPseudoParent = PR_FALSE;
   // this frame may have a pseudo parent
-  GetParentFrame(aNameSpaceID, *aParentFrameIn, 
-                 nsGkAtoms::tableCaptionFrame, aState, parentFrame,
-                 *aHasPseudoParent);
+  CreateRequiredPseudoFrames(aNameSpaceID, *aParentFrameIn,
+                             nsGkAtoms::tableCaptionFrame, aState, parentFrame,
+                             *aHasPseudoParent);
   if (!*aHasPseudoParent && !aState.mPseudoFrames.IsEmpty()) {
     ProcessPseudoFrames(aState, aChildItems);
   }
 
   aNewFrame = NS_NewTableCaptionFrame(mPresShell, aStyleContext);
   if (NS_UNLIKELY(!aNewFrame)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
@@ -3328,19 +3307,19 @@ nsCSSFrameConstructor::ConstructTableRow
                                                    PRBool*                  aHasPseudoParent)
 {
   if (!aParentFrameIn) return NS_ERROR_UNEXPECTED;
 
   nsIFrame* parentFrame = aParentFrameIn;
   *aHasPseudoParent = PR_FALSE;
   if (!aIsPseudo) {
     // this frame may have a pseudo parent
-    GetParentFrame(aNameSpaceID, *aParentFrameIn, 
-                   nsGkAtoms::tableRowGroupFrame, aState, parentFrame,
-                   *aHasPseudoParent);
+    CreateRequiredPseudoFrames(aNameSpaceID, *aParentFrameIn,
+                               nsGkAtoms::tableRowGroupFrame, aState,
+                               parentFrame, *aHasPseudoParent);
     if (!*aHasPseudoParent && !aState.mPseudoFrames.IsEmpty()) {
       ProcessPseudoFrames(aState, aChildItems);
     }
     if (!aIsPseudo && *aHasPseudoParent &&
         aState.mPseudoFrames.mRowGroup.mFrame) {
       ProcessPseudoFrames(aState, nsGkAtoms::tableRowGroupFrame);
     }
   }
@@ -3398,19 +3377,19 @@ nsCSSFrameConstructor::ConstructTableCol
                                                    PRBool*                  aHasPseudoParent)
 {
   if (!aParentFrameIn) return NS_ERROR_UNEXPECTED;
 
   nsIFrame* parentFrame = aParentFrameIn;
   *aHasPseudoParent = PR_FALSE;
   if (!aIsPseudo) {
     // this frame may have a pseudo parent
-    GetParentFrame(aNameSpaceID, *aParentFrameIn,
-                   nsGkAtoms::tableColGroupFrame, aState, parentFrame,
-                   *aHasPseudoParent);
+    CreateRequiredPseudoFrames(aNameSpaceID, *aParentFrameIn,
+                               nsGkAtoms::tableColGroupFrame, aState,
+                               parentFrame, *aHasPseudoParent);
     if (!*aHasPseudoParent && !aState.mPseudoFrames.IsEmpty()) {
       ProcessPseudoFrames(aState, aChildItems);
     }
     if (!aIsPseudo && *aHasPseudoParent &&
         aState.mPseudoFrames.mColGroup.mFrame) {
       ProcessPseudoFrames(aState, nsGkAtoms::tableColGroupFrame);
     }
   }
@@ -3447,19 +3426,19 @@ nsCSSFrameConstructor::ConstructTableRow
                                               PRBool*                  aHasPseudoParent)
 {
   if (!aParentFrameIn) return NS_ERROR_UNEXPECTED;
 
   nsIFrame* parentFrame = aParentFrameIn;
   *aHasPseudoParent = PR_FALSE;
   if (!aIsPseudo) {
     // this frame may have a pseudo parent
-    GetParentFrame(aNameSpaceID, *aParentFrameIn,
-                   nsGkAtoms::tableRowFrame, aState, parentFrame,
-                   *aHasPseudoParent);
+    CreateRequiredPseudoFrames(aNameSpaceID, *aParentFrameIn,
+                               nsGkAtoms::tableRowFrame, aState, parentFrame,
+                               *aHasPseudoParent);
     if (!*aHasPseudoParent && !aState.mPseudoFrames.IsEmpty()) {
       ProcessPseudoFrames(aState, aChildItems);
     }
     if (!aIsPseudo && *aHasPseudoParent && aState.mPseudoFrames.mRow.mFrame) {
       ProcessPseudoFrames(aState, nsGkAtoms::tableRowFrame);
     }
   }
 
@@ -3502,19 +3481,19 @@ nsCSSFrameConstructor::ConstructTableCol
                                               PRBool*                  aHasPseudoParent)
 {
   if (!aParentFrameIn || !aStyleContext) return NS_ERROR_UNEXPECTED;
 
   nsIFrame* parentFrame = aParentFrameIn;
   *aHasPseudoParent = PR_FALSE;
   if (!aIsPseudo) {
     // this frame may have a pseudo parent
-    GetParentFrame(aNameSpaceID, *aParentFrameIn,
-                   nsGkAtoms::tableColFrame, aState, parentFrame,
-                   *aHasPseudoParent);
+    CreateRequiredPseudoFrames(aNameSpaceID, *aParentFrameIn,
+                               nsGkAtoms::tableColFrame, aState, parentFrame,
+                               *aHasPseudoParent);
     if (!*aHasPseudoParent && !aState.mPseudoFrames.IsEmpty()) {
       ProcessPseudoFrames(aState, aChildItems);
     }
   }
 
   nsTableColFrame* colFrame = NS_NewTableColFrame(mPresShell, aStyleContext);
   aNewFrame = colFrame;
   if (NS_UNLIKELY(!aNewFrame)) {
@@ -3563,19 +3542,19 @@ nsCSSFrameConstructor::ConstructTableCel
 {
   if (!aParentFrameIn) return NS_ERROR_UNEXPECTED;
 
   nsIFrame* parentFrame = aParentFrameIn;
   *aHasPseudoParent = PR_FALSE;
   if (!aIsPseudo) {
     // this frame may have a pseudo parent
     // use nsGkAtoms::tableCellFrame which will match if it is really nsGkAtoms::bcTableCellFrame
-    GetParentFrame(aNameSpaceID, *aParentFrameIn,
-                   nsGkAtoms::tableCellFrame, aState, parentFrame,
-                   *aHasPseudoParent);
+    CreateRequiredPseudoFrames(aNameSpaceID, *aParentFrameIn,
+                               nsGkAtoms::tableCellFrame, aState, parentFrame,
+                               *aHasPseudoParent);
     if (!*aHasPseudoParent && !aState.mPseudoFrames.IsEmpty()) {
       ProcessPseudoFrames(aState, aChildItems);
     }
     if (!aIsPseudo && *aHasPseudoParent && aState.mPseudoFrames.mCellOuter.mFrame) {
       ProcessPseudoFrames(aState, nsGkAtoms::tableCellFrame);
     }
   }
 #ifdef MOZ_MATHML
@@ -5876,18 +5855,17 @@ nsCSSFrameConstructor::FindDisplayData(c
     return &sInlineData;
   }
 
   NS_ASSERTION(IsTableRelated(aDisplay->mDisplay), "Unexpected display type");
 
   if (NS_STYLE_DISPLAY_TABLE == aDisplay->mDisplay ||
       NS_STYLE_DISPLAY_INLINE_TABLE == aDisplay->mDisplay) {
     static const FrameConstructionData sTableData =
-      FULL_CTOR_FCDATA(FCDATA_IS_TABLE_PART,
-                       &nsCSSFrameConstructor::ConstructTable);
+      FULL_CTOR_FCDATA(0, &nsCSSFrameConstructor::ConstructTable);
     return &sTableData;
   }
 
   static const FrameConstructionData sTablePartData =
     FULL_CTOR_FCDATA(FCDATA_IS_TABLE_PART | FCDATA_SKIP_FRAMEMAP,
                      &nsCSSFrameConstructor::ConstructTablePart);
   return &sTablePartData;
   
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -534,22 +534,22 @@ private:
   nsresult GetPseudoRowFrame(PRInt32                  aNameSpaceID,
                              nsFrameConstructorState& aState, 
                              nsIFrame&                aParentFrameIn);
 
   nsresult GetPseudoCellFrame(PRInt32                  aNameSpaceID,
                               nsFrameConstructorState& aState, 
                               nsIFrame&                aParentFrameIn);
 
-  nsresult GetParentFrame(PRInt32                  aNameSpaceID,
-                          nsIFrame&                aParentFrameIn, 
-                          nsIAtom*                 aChildFrameType, 
-                          nsFrameConstructorState& aState, 
-                          nsIFrame*&               aParentFrame,
-                          PRBool&                  aIsPseudoParent);
+  nsresult CreateRequiredPseudoFrames(PRInt32                  aNameSpaceID,
+                                      nsIFrame&                aParentFrameIn,
+                                      nsIAtom*                 aChildFrameType,
+                                      nsFrameConstructorState& aState,
+                                      nsIFrame*&               aParentFrame,
+                                      PRBool&                  aIsPseudoParent);
 
 private:
   /* A constructor function that just creates an nsIFrame object.  The caller
      is responsible for initializing the object, adding it to frame lists,
      constructing frames for the children, etc.
 
      @param nsIPresShell the presshell whose arena should be used to allocate
                          the frame.
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/478956-1-ref.html
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+  <body>
+    <table cellpadding="0" cellspacing="0">
+      <tr>
+        <td>Long long cell</td>
+        <td>cell</td>
+      </tr>
+    </table>
+    <table cellpadding="0" cellspacing="0">
+      <tr>
+        <td>cell</td>
+        <td>cell</td>
+      </tr>
+    </table>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/478956-1a.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+  <body>
+    <div style="display: table-row">
+      <div style="display: table-row">
+        <div style="display: table-cell">Long long cell</div>
+        <div style="display: table-cell">cell</div>
+      </div>
+      <span style="display: table"></span>
+      <div style="display: table-row">
+        <div style="display: table-cell">cell</div>
+        <div style="display: table-cell">cell</div>
+      </div>
+    </div>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/478956-1b.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+  <body>
+    <div style="display: table-row">
+      <div style="display: table-row">
+        <div style="display: table-cell">Long long cell</div>
+        <div style="display: table-cell">cell</div>
+      </div>
+      <span style="display: block"></span>
+      <div style="display: table-row">
+        <div style="display: table-cell">cell</div>
+        <div style="display: table-cell">cell</div>
+      </div>
+    </div>
+  </body>
+</html>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -1087,8 +1087,10 @@ fails == 461512-1.html 461512-1-ref.html
 == 476598-1b.html 476598-1-ref.html
 == 476598-1b.html 476598-1-ref2.html
 != 476598-1-ref.html about:blank
 == 478377-1.xul 478377-1-ref.xul
 == 478811-1.html 478811-1-ref.html
 == 478811-2.html 478811-2-ref.html
 == 478811-3.html 478811-3-ref.html
 == 478811-4.html 478811-4-ref.html
+== 478956-1a.html 478956-1-ref.html
+== 478956-1b.html 478956-1-ref.html