PDT+ bug 28341 - Destroy caption frame when outer table frame is destroyed. Add caption to outer frame rather than inner frame. r=troy, a=rickg

layout/base/nsCSSFrameConstructor.cpp

@@ -1402,12 +1402,6 @@ nsCSSFrameConstructor::ConstructTableCaptionFrame(nsIPresShell*            aPres
                                           nsLayoutAtoms::floaterList,
                                           aState.mFloatedItems.childList);
   }
-
-  if (outerFrame) {
-    outerFrame->SetInitialChildList(aPresContext, nsLayoutAtoms::captionList,
-                                    aNewCaptionFrame);
-  }
-
   return rv;
 }
 
@@ -5096,7 +5090,19 @@ nsCSSFrameConstructor::ConstructFrameByDisplayType(nsIPresShell* aPresShell,
     // the next 5 cases are only relevant if the parent is not a table, ConstructTableFrame handles children
     case NS_STYLE_DISPLAY_TABLE_CAPTION:
     {
-      rv = ConstructTableCaptionFrame(aPresShell, aPresContext, aState, aContent, aParentFrame,
+      // aParentFrame may be an inner table frame rather than an outer frame 
+      // In this case we need to get the outer frame.
+      nsIFrame* parentFrame = aParentFrame;
+      nsIFrame* outerFrame = nsnull;
+      aParentFrame->GetParent(&outerFrame);
+      nsCOMPtr<nsIAtom> frameType;
+      if (outerFrame) {
+        outerFrame->GetFrameType(getter_AddRefs(frameType));
+        if (nsLayoutAtoms::tableOuterFrame == frameType.get()) {
+          parentFrame = outerFrame;
+        }
+      }
+      rv = ConstructTableCaptionFrame(aPresShell, aPresContext, aState, aContent, parentFrame,
                                       aStyleContext, newFrame, ignore, tableCreator);
       aFrameItems.AddChild(newFrame);
       return rv;

layout/html/style/src/nsCSSFrameConstructor.cpp

@@ -1402,12 +1402,6 @@ nsCSSFrameConstructor::ConstructTableCaptionFrame(nsIPresShell*            aPres
                                           nsLayoutAtoms::floaterList,
                                           aState.mFloatedItems.childList);
   }
-
-  if (outerFrame) {
-    outerFrame->SetInitialChildList(aPresContext, nsLayoutAtoms::captionList,
-                                    aNewCaptionFrame);
-  }
-
   return rv;
 }
 
@@ -5096,7 +5090,19 @@ nsCSSFrameConstructor::ConstructFrameByDisplayType(nsIPresShell* aPresShell,
     // the next 5 cases are only relevant if the parent is not a table, ConstructTableFrame handles children
     case NS_STYLE_DISPLAY_TABLE_CAPTION:
     {
-      rv = ConstructTableCaptionFrame(aPresShell, aPresContext, aState, aContent, aParentFrame,
+      // aParentFrame may be an inner table frame rather than an outer frame 
+      // In this case we need to get the outer frame.
+      nsIFrame* parentFrame = aParentFrame;
+      nsIFrame* outerFrame = nsnull;
+      aParentFrame->GetParent(&outerFrame);
+      nsCOMPtr<nsIAtom> frameType;
+      if (outerFrame) {
+        outerFrame->GetFrameType(getter_AddRefs(frameType));
+        if (nsLayoutAtoms::tableOuterFrame == frameType.get()) {
+          parentFrame = outerFrame;
+        }
+      }
+      rv = ConstructTableCaptionFrame(aPresShell, aPresContext, aState, aContent, parentFrame,
                                       aStyleContext, newFrame, ignore, tableCreator);
       aFrameItems.AddChild(newFrame);
       return rv;

layout/html/table/src/nsTableOuterFrame.cpp

@@ -37,6 +37,8 @@
 #include "nsHTMLParts.h"
 #include "nsIPresShell.h"
 
+/* ----------- nsTableCaptionFrame ---------- */
+
 #define NS_TABLE_FRAME_CAPTION_LIST_INDEX 0
 
 // caption frame
@@ -46,6 +48,20 @@ nsTableCaptionFrame::nsTableCaptionFrame()
   SetFlags(NS_BLOCK_SPACE_MGR|NS_BLOCK_WRAP_SIZE);
 }
 
+nsTableCaptionFrame::~nsTableCaptionFrame()
+{
+}
+
+NS_IMETHODIMP
+nsTableOuterFrame::Destroy(nsIPresContext* aPresContext)
+{
+  if (mCaptionFrame) {
+    mCaptionFrame->Destroy(aPresContext);
+  }
+
+  return nsHTMLContainerFrame::Destroy(aPresContext);
+}
+
 NS_IMETHODIMP
 nsTableCaptionFrame::GetFrameType(nsIAtom** aType) const
 {
@@ -117,13 +133,19 @@ struct OuterTableReflowState {
   }
 };
 
-
-
 /* ----------- nsTableOuterFrame ---------- */
 
 NS_IMPL_ADDREF_INHERITED(nsTableOuterFrame, nsHTMLContainerFrame)
 NS_IMPL_RELEASE_INHERITED(nsTableOuterFrame, nsHTMLContainerFrame)
 
+nsTableOuterFrame::nsTableOuterFrame()
+{
+}
+
+nsTableOuterFrame::~nsTableOuterFrame()
+{
+}
+
 nsresult nsTableOuterFrame::QueryInterface(const nsIID& aIID, void** aInstancePtr)
 {
   if (NULL == aInstancePtr) {

layout/html/table/src/nsTableOuterFrame.h

@@ -32,10 +32,14 @@ struct nsStyleTable;
 
 class nsTableCaptionFrame : public nsBlockFrame
 {
-  public:
+public:
   // nsISupports
-  nsTableCaptionFrame();
   NS_IMETHOD  GetFrameType(nsIAtom** aType) const;
+  friend nsresult NS_NewTableCaptionFrame(nsIPresShell* aPresShell, nsIFrame** aNewFrame);
+
+protected:
+  nsTableCaptionFrame();
+  virtual ~nsTableCaptionFrame();
 };
 
 
@@ -60,8 +64,7 @@ public:
     *
     * @return  NS_OK if the frame was properly allocated, otherwise an error code
     */
-  friend nsresult 
+  friend nsresult NS_NewTableOuterFrame(nsIPresShell* aPresShell, nsIFrame** aResult);
-  NS_NewTableOuterFrame(nsIPresShell* aPresShell, nsIFrame** aResult);
 
   NS_IMETHOD Init(nsIPresContext*  aPresContext,
                   nsIContent*      aContent,
@@ -69,6 +72,8 @@ public:
                   nsIStyleContext* aContext,
                   nsIFrame*        aPrevInFlow);
 
+  NS_IMETHOD Destroy(nsIPresContext* aPresContext);
+
   NS_IMETHOD AdjustZeroWidth();
 
   /**  @see nsIFrame::SetInitialChildList */    
@@ -151,6 +156,9 @@ public:
 protected:
 
 
+  nsTableOuterFrame();
+  virtual ~nsTableOuterFrame();
+
   /** Always returns 0, since the outer table frame has no border of its own
     * The inner table frame can answer this question in a meaningful way.
     * @see nsHTMLContainerFrame::GetSkipSides */

layout/html/tests/table/bugs/bug28341.html

@@ -0,0 +1,8 @@
+The bug causes a crash when the page is destroyed
+<BR>
+<TABLE>
+<CAPTION>
+<IMG SRC="../images/animated.gif">
+</CAPTION>
+<TR><TD>foo</TD></TR>
+</TABLE>

layout/html/tests/table/bugs/file_list.txt

@@ -127,6 +127,7 @@ file:///s|/mozilla/layout/html/tests/table/bugs/bug2684.html
 file:///s|/mozilla/layout/html/tests/table/bugs/bug2757.html
 file:///s|/mozilla/layout/html/tests/table/bugs/bug2763.html
 file:///s|/mozilla/layout/html/tests/table/bugs/bug2773.html
+file:///s|/mozilla/layout/html/tests/table/bugs/bug28341.html
 file:///s|/mozilla/layout/html/tests/table/bugs/bug2886.html
 file:///s|/mozilla/layout/html/tests/table/bugs/bug2886-2.html
 file:///s|/mozilla/layout/html/tests/table/bugs/bug28928.html

layout/tables/nsTableOuterFrame.cpp

@@ -37,6 +37,8 @@
 #include "nsHTMLParts.h"
 #include "nsIPresShell.h"
 
+/* ----------- nsTableCaptionFrame ---------- */
+
 #define NS_TABLE_FRAME_CAPTION_LIST_INDEX 0
 
 // caption frame
@@ -46,6 +48,20 @@ nsTableCaptionFrame::nsTableCaptionFrame()
   SetFlags(NS_BLOCK_SPACE_MGR|NS_BLOCK_WRAP_SIZE);
 }
 
+nsTableCaptionFrame::~nsTableCaptionFrame()
+{
+}
+
+NS_IMETHODIMP
+nsTableOuterFrame::Destroy(nsIPresContext* aPresContext)
+{
+  if (mCaptionFrame) {
+    mCaptionFrame->Destroy(aPresContext);
+  }
+
+  return nsHTMLContainerFrame::Destroy(aPresContext);
+}
+
 NS_IMETHODIMP
 nsTableCaptionFrame::GetFrameType(nsIAtom** aType) const
 {
@@ -117,13 +133,19 @@ struct OuterTableReflowState {
   }
 };
 
-
-
 /* ----------- nsTableOuterFrame ---------- */
 
 NS_IMPL_ADDREF_INHERITED(nsTableOuterFrame, nsHTMLContainerFrame)
 NS_IMPL_RELEASE_INHERITED(nsTableOuterFrame, nsHTMLContainerFrame)
 
+nsTableOuterFrame::nsTableOuterFrame()
+{
+}
+
+nsTableOuterFrame::~nsTableOuterFrame()
+{
+}
+
 nsresult nsTableOuterFrame::QueryInterface(const nsIID& aIID, void** aInstancePtr)
 {
   if (NULL == aInstancePtr) {

layout/tables/nsTableOuterFrame.h

@@ -32,10 +32,14 @@ struct nsStyleTable;
 
 class nsTableCaptionFrame : public nsBlockFrame
 {
-  public:
+public:
   // nsISupports
-  nsTableCaptionFrame();
   NS_IMETHOD  GetFrameType(nsIAtom** aType) const;
+  friend nsresult NS_NewTableCaptionFrame(nsIPresShell* aPresShell, nsIFrame** aNewFrame);
+
+protected:
+  nsTableCaptionFrame();
+  virtual ~nsTableCaptionFrame();
 };
 
 
@@ -60,8 +64,7 @@ public:
     *
     * @return  NS_OK if the frame was properly allocated, otherwise an error code
     */
-  friend nsresult 
+  friend nsresult NS_NewTableOuterFrame(nsIPresShell* aPresShell, nsIFrame** aResult);
-  NS_NewTableOuterFrame(nsIPresShell* aPresShell, nsIFrame** aResult);
 
   NS_IMETHOD Init(nsIPresContext*  aPresContext,
                   nsIContent*      aContent,
@@ -69,6 +72,8 @@ public:
                   nsIStyleContext* aContext,
                   nsIFrame*        aPrevInFlow);
 
+  NS_IMETHOD Destroy(nsIPresContext* aPresContext);
+
   NS_IMETHOD AdjustZeroWidth();
 
   /**  @see nsIFrame::SetInitialChildList */    
@@ -151,6 +156,9 @@ public:
 protected:
 
 
+  nsTableOuterFrame();
+  virtual ~nsTableOuterFrame();
+
   /** Always returns 0, since the outer table frame has no border of its own
     * The inner table frame can answer this question in a meaningful way.
     * @see nsHTMLContainerFrame::GetSkipSides */