Swap the order between the legend and area frames in a fieldset to fix selection and tabbing problems. b=236071 r+sr=bzbarsky

layout/base/nsCSSFrameConstructor.cpp

@@ -4867,14 +4867,18 @@ nsCSSFrameConstructor::ConstructFieldSetFrame(nsIPresShell*            aPresShel
   while (nsnull != child) {
     nsresult result = child->QueryInterface(kLegendFrameCID, (void**)&legendFrame);
     if (NS_SUCCEEDED(result) && legendFrame) {
+      // We want the legend to be the first frame in the fieldset child list.
+      // That way the EventStateManager will do the right thing when tabbing
+      // from a selection point within the legend (bug 236071), which is
+      // used for implementing legend access keys (bug 81481).
+      // GetAdjustedParentFrame() below depends on this frame order.
       if (nsnull != previous) {
         previous->SetNextSibling(legendFrame->GetNextSibling());
       } else {
         childItems.childList = legendFrame->GetNextSibling();
       }
-      areaFrame->SetNextSibling(legendFrame);
+      legendFrame->SetNextSibling(areaFrame);
       legendFrame->SetParent(newFrame);
-      legendFrame->SetNextSibling(nsnull);
       break;
     }
     previous = child;
@@ -4885,7 +4889,7 @@ nsCSSFrameConstructor::ConstructFieldSetFrame(nsIPresShell*            aPresShel
   areaFrame->SetInitialChildList(aPresContext, nsnull, childItems.childList);
 
   // Set the scroll frame's initial child list
-  newFrame->SetInitialChildList(aPresContext, nsnull, areaFrame);
+  newFrame->SetInitialChildList(aPresContext, nsnull, legendFrame ? legendFrame : areaFrame);
 
   // our new frame retured is the top frame which is the list frame. 
   aNewFrame = newFrame; 
@@ -8297,8 +8301,13 @@ GetAdjustedParentFrame(nsPresContext* aPresContext,
     // If the parent is a fieldSet, use the fieldSet's area frame as the
     // parent unless the new content is a legend. 
     nsCOMPtr<nsIDOMHTMLLegendElement> legendContent(do_QueryInterface(childContent));
-    if (!legendContent) 
+    if (!legendContent) {
+      // Depends on the fieldset child frame order - see ConstructFieldSetFrame() above.
       newParent = aParentFrame->GetFirstChild(nsnull);
+      if (newParent->GetNextSibling()) {
+        newParent = newParent->GetNextSibling();
+      }
+    }
   }
   return (newParent) ? newParent : aParentFrame;
 }

layout/forms/nsFieldSetFrame.cpp

@@ -166,10 +166,14 @@ nsFieldSetFrame::SetInitialChildList(nsPresContext* aPresContext,
                                      nsIAtom*        aListName,
                                      nsIFrame*       aChildList)
 {
- 
-  // get the content and legend frames.
+  // Get the content and legend frames.
+  if (aChildList->GetNextSibling()) {
+    mContentFrame = aChildList->GetNextSibling();
+    mLegendFrame  = aChildList;
+  } else {
     mContentFrame = aChildList;
-  mLegendFrame = mContentFrame->GetNextSibling();
+    mLegendFrame  = nsnull;
+  }
 
   // Queue up the frames for the content frame
   return nsHTMLContainerFrame::SetInitialChildList(aPresContext, nsnull, aChildList);
@@ -627,14 +631,9 @@ nsFieldSetFrame::RemoveFrame(nsPresContext* aPresContext,
   // XXX XXX
   // XXX temporary fix for bug 70648
   if (aOldFrame == mLegendFrame) {   
-    nsIFrame* sibling = mContentFrame->GetNextSibling();
-    NS_ASSERTION(sibling == mLegendFrame, "legendFrame is not next sibling");
     NS_ASSERTION(mLegendFrame->GetParent() == this, "Legend Parent has wrong parent");
-    nsIFrame* legendSibling = sibling->GetNextSibling();
-    // replace the legend, which is the next sibling, with any siblings of the legend (XXX always null?)
-    mContentFrame->SetNextSibling(legendSibling);
-    // OK, the legend is now removed from the sibling list, but who has ownership of it?
-    mLegendFrame->Destroy(aPresContext);
+    NS_ASSERTION(mLegendFrame->GetNextSibling() == mContentFrame, "mContentFrame is not next sibling");
+    mFrames.DestroyFrame(aPresContext, mLegendFrame);
     mLegendFrame = nsnull;
     return NS_OK;
   } else {