Bug 496840. Make dynamic insertion and removal of legends work in the presence of multiple legends. r+sr=bzbarsky
authorRobert O'Callahan <robert@ocallahan.org>
Fri, 12 Jun 2009 14:01:05 +1200
changeset 29110 9d1dd0ef5ca655d519be1bb584fd18bed7aede86
parent 29109 4051ce823b55c37468e396c9a966e2a693c03114
child 29111 fd6f6d3df310fdad50d7ce9526f4e8355d09de66
push id7423
push userrocallahan@mozilla.com
push dateFri, 12 Jun 2009 02:16:07 +0000
treeherderautoland@b3c4e464fed7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs496840
milestone1.9.2a1pre
Bug 496840. Make dynamic insertion and removal of legends work in the presence of multiple legends. r+sr=bzbarsky
layout/base/nsCSSFrameConstructor.cpp
layout/forms/nsFieldSetFrame.cpp
layout/reftests/bugs/496840-1-ref.html
layout/reftests/bugs/496840-1.html
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -6477,22 +6477,34 @@ nsCSSFrameConstructor::ContentInserted(n
 
   PRBool isAppend;
   nsIFrame* prevSibling =
     GetInsertionPrevSibling(parentFrame, aContainer, aChild, aIndexInContainer,
                             &isAppend);
 
   nsIContent* container = parentFrame->GetContent();
 
-  if (parentFrame->GetType() == nsGkAtoms::frameSetFrame &&
+  nsIAtom* frameType = parentFrame->GetType();
+  if (frameType == nsGkAtoms::frameSetFrame &&
       IsSpecialFramesetChild(aChild)) {
     // Just reframe the parent, since framesets are weird like that.
     return RecreateFramesForContent(parentFrame->GetContent());
   }
-  
+
+  if (frameType == nsGkAtoms::fieldSetFrame &&
+      aChild->Tag() == nsGkAtoms::legend) {
+    // Just reframe the parent, since figuring out whether this
+    // should be the new legend and then handling it is too complex.
+    // We could do a little better here --- check if the fieldset already
+    // has a legend which occurs earlier in its child list than this node,
+    // and if so, proceed. But we'd have to extend nsFieldSetFrame
+    // to locate this legend in the inserted frames and extract it.
+    return RecreateFramesForContent(parentFrame->GetContent());
+  }
+
   // Don't construct kids of leaves
   if (parentFrame->IsLeaf()) {
     return NS_OK;
   }
 
 #ifdef MOZ_MATHML
   if (parentFrame->IsFrameOfType(nsIFrame::eMathML))
     return RecreateFramesForContent(parentFrame->GetContent());
@@ -8698,16 +8710,24 @@ nsCSSFrameConstructor::MaybeRecreateCont
       printf(" is special\n");
     }
 #endif
 
     *aResult = ReframeContainingBlock(aFrame);
     return PR_TRUE;
   }
 
+  if (aFrame->GetType() == nsGkAtoms::legendFrame &&
+      aFrame->GetParent()->GetType() == nsGkAtoms::fieldSetFrame) {
+    // When we remove the legend for a fieldset, we should reframe
+    // the fieldset to ensure another legend is used, if there is one
+    *aResult = RecreateFramesForContent(aFrame->GetParent()->GetContent());
+    return PR_TRUE;
+  }
+
   // Now check for possibly needing to reconstruct due to a pseudo parent
   nsIFrame* inFlowFrame =
     (aFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW) ?
       mPresShell->FrameManager()->GetPlaceholderFrameFor(aFrame) : aFrame;
   NS_ASSERTION(inFlowFrame, "How did that happen?");
   nsIFrame* parent = inFlowFrame->GetParent();
   if (IsTablePseudo(parent)) {
     if (FindFirstNonWhitespaceChild(parent) == inFlowFrame ||
--- a/layout/forms/nsFieldSetFrame.cpp
+++ b/layout/forms/nsFieldSetFrame.cpp
@@ -113,17 +113,16 @@ public:
   NS_IMETHOD GetFrameName(nsAString& aResult) const {
     return MakeFrameName(NS_LITERAL_STRING("FieldSet"), aResult);
   }
 #endif
 
 protected:
 
   virtual PRIntn GetSkipSides() const;
-  nsIFrame* MaybeSetLegend(nsIFrame* aFrameList, nsIAtom* aListName);
   void ReParentFrameList(nsIFrame* aFrameList);
 
   nsIFrame* mLegendFrame;
   nsIFrame* mContentFrame;
   nsRect    mLegendRect;
   nscoord   mLegendSpace;
 };
 
@@ -610,94 +609,70 @@ nsFieldSetFrame::GetSkipSides() const
 {
   return 0;
 }
 
 NS_IMETHODIMP
 nsFieldSetFrame::AppendFrames(nsIAtom*       aListName,
                               nsIFrame*      aFrameList)
 {
-  aFrameList = MaybeSetLegend(aFrameList, aListName);
   if (aFrameList) {
+    // aFrameList is not allowed to contain "the legend" for this fieldset
     ReParentFrameList(aFrameList);
     return mContentFrame->AppendFrames(aListName, aFrameList);
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsFieldSetFrame::InsertFrames(nsIAtom*       aListName,
                               nsIFrame*      aPrevFrame,
                               nsIFrame*      aFrameList)
 {
   NS_ASSERTION(!aPrevFrame || aPrevFrame->GetParent() == this ||
                aPrevFrame->GetParent() == mContentFrame,
                "inserting after sibling frame with different parent");
 
-  aFrameList = MaybeSetLegend(aFrameList, aListName);
   if (aFrameList) {
+    // aFrameList is not allowed to contain "the legend" for this fieldset
     ReParentFrameList(aFrameList);
     if (NS_UNLIKELY(aPrevFrame == mLegendFrame)) {
       aPrevFrame = nsnull;
     }
     return mContentFrame->InsertFrames(aListName, aPrevFrame, aFrameList);
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsFieldSetFrame::RemoveFrame(nsIAtom*       aListName,
                              nsIFrame*      aOldFrame)
 {
   // For reference, see bug 70648, bug 276104 and bug 236071.
-  if (aOldFrame == mLegendFrame) {
-    NS_ASSERTION(!aListName, "Unexpected frame list when removing legend frame");
-    NS_ASSERTION(mLegendFrame->GetParent() == this, "Legend Parent has wrong parent");
-    NS_ASSERTION(mLegendFrame->GetNextSibling() == mContentFrame, "mContentFrame is not next sibling");
-
-    mFrames.DestroyFrame(mLegendFrame);
-    mLegendFrame = nsnull;
-    PresContext()->PresShell()->
-      FrameNeedsReflow(this, nsIPresShell::eTreeChange, NS_FRAME_IS_DIRTY);
-    return NS_OK;
-  }
+  NS_ASSERTION(aOldFrame != mLegendFrame, "Cannot remove mLegendFrame here");
   return mContentFrame->RemoveFrame(aListName, aOldFrame);
 }
 
 #ifdef ACCESSIBILITY
 NS_IMETHODIMP nsFieldSetFrame::GetAccessible(nsIAccessible** aAccessible)
 {
   nsCOMPtr<nsIAccessibilityService> accService = do_GetService("@mozilla.org/accessibilityService;1");
 
   if (accService) {
     return accService->CreateHTMLGroupboxAccessible(static_cast<nsIFrame*>(this), aAccessible);
   }
 
   return NS_ERROR_FAILURE;
 }
 #endif
 
-nsIFrame*
-nsFieldSetFrame::MaybeSetLegend(nsIFrame* aFrameList, nsIAtom* aListName)
-{
-  if (!mLegendFrame && aFrameList->GetType() == nsGkAtoms::legendFrame) {
-    NS_ASSERTION(!aListName, "Unexpected frame list when adding legend frame");
-    mLegendFrame = aFrameList;
-    aFrameList = mLegendFrame->GetNextSibling();
-    mLegendFrame->SetNextSibling(mContentFrame);
-    mFrames.SetFrames(mLegendFrame);
-    PresContext()->PresShell()->
-      FrameNeedsReflow(this, nsIPresShell::eTreeChange,
-                       NS_FRAME_HAS_DIRTY_CHILDREN);
-  }
-  return aFrameList;
-}
-
 void
 nsFieldSetFrame::ReParentFrameList(nsIFrame* aFrameList)
 {
   nsFrameManager* frameManager = PresContext()->FrameManager();
   for (nsIFrame* frame = aFrameList; frame; frame = frame->GetNextSibling()) {
+    NS_ASSERTION(mLegendFrame || frame->GetType() != nsGkAtoms::legendFrame,
+                 "The fieldset's legend is not allowed in this list");
     frame->SetParent(mContentFrame);
     frameManager->ReParentStyleContext(frame);
   }
   mContentFrame->AddStateBits(GetStateBits() & NS_FRAME_HAS_CHILD_WITH_VIEW);
 }
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/496840-1-ref.html
@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<html>
+<body>
+<fieldset id="fs1"><legend>#1</legend><legend>#2</legend></fieldset>
+<fieldset id="fs2"><legend>#2</legend></fieldset>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/496840-1.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<body>
+<fieldset id="fs1"><legend>#2</legend></fieldset>
+<fieldset id="fs2"><legend>#1</legend><legend>#2</legend></fieldset>
+<script>
+document.body.offsetHeight;
+
+var fs1 = document.getElementById("fs1");
+var l = document.createElement("legend");
+l.textContent = "#1";
+fs1.insertBefore(l, fs1.firstChild);
+
+var fs2 = document.getElementById("fs2");
+fs2.removeChild(fs2.firstChild);
+</script>
+</body>
+</html>