Bug 242829 - Remove table caption list. r=dshin

This fixes the issue by moving the table captions to the principal child
list. It also paves the way to fix bug 144517, but it doesn't fix it as
that requires extra layout changes to nsTableWrapper frame (mostly to
reserve the multiple caption block sizes).

Test courtesy of Masayuki in D246407 (with minor tweaks).

Differential Revision: https://phabricator.services.mozilla.com/D246468

layout/base/nsCSSFrameConstructor.cpp

@@ -2138,8 +2138,7 @@ nsIFrame* nsCSSFrameConstructor::ConstructTable(nsFrameConstructorState& aState,
   // Set the table wrapper frame's secondary childlist lists
   if (captionList.NotEmpty()) {
     captionList.ApplySetParent(newFrame);
-    newFrame->SetInitialChildList(FrameChildListID::Caption,
+    newFrame->AppendFrames(FrameChildListID::Principal, std::move(captionList));
-                                  std::move(captionList));
   }
 
   return newFrame;
@@ -6650,7 +6649,8 @@ void nsCSSFrameConstructor::ContentAppended(nsIContent* aFirstNewContent,
     NS_ASSERTION(LayoutFrameType::Table == frameType, "how did that happen?");
     nsContainerFrame* outerTable = parentFrame->GetParent();
     captionList.ApplySetParent(outerTable);
-    AppendFrames(outerTable, FrameChildListID::Caption, std::move(captionList));
+    AppendFrames(outerTable, FrameChildListID::Principal,
+                 std::move(captionList));
   }
 
   LAYOUT_PHASE_TEMP_EXIT();
@@ -7153,17 +7153,18 @@ void nsCSSFrameConstructor::ContentRangeInserted(nsIContent* aStartChild,
 
     // If the parent of our current prevSibling is different from the frame
     // we'll actually use as the parent, then the calculated insertion
-    // point is now invalid (bug 341382).
+    // point is now invalid (bug 341382). Insert right after the table frame
-    if (captionPrevSibling && captionPrevSibling->GetParent() != outerTable) {
+    // instead.
-      captionPrevSibling = nullptr;
+    if (!captionPrevSibling || captionPrevSibling->GetParent() != outerTable) {
+      captionPrevSibling = outerTable->PrincipalChildList().FirstChild();
     }
 
     captionList.ApplySetParent(outerTable);
     if (captionIsAppend) {
-      AppendFrames(outerTable, FrameChildListID::Caption,
+      AppendFrames(outerTable, FrameChildListID::Principal,
                    std::move(captionList));
     } else {
-      InsertFrames(outerTable, FrameChildListID::Caption, captionPrevSibling,
+      InsertFrames(outerTable, FrameChildListID::Principal, captionPrevSibling,
                    std::move(captionList));
     }
   }
@@ -7219,9 +7220,13 @@ static bool IsSyntheticColGroup(const nsIFrame* aFrame) {
          static_cast<const nsTableColGroupFrame*>(aFrame)->IsSynthetic();
 }
 
-static bool IsOnlyNonWhitespaceFrameInList(const nsFrameList& aFrameList,
+static bool IsOnlyNonWhitespaceFrameInList(
-                                           const nsIFrame* aFrame) {
+    const nsFrameList& aFrameList, const nsIFrame* aFrame,
+    const nsIFrame* aIgnoreFrame = nullptr) {
   for (const nsIFrame* f : aFrameList) {
+    if (f == aIgnoreFrame) {
+      continue;
+    }
     if (f == aFrame) {
       // If we have continuations, ignore them too.
       aFrame = aFrame->GetNextContinuation();
@@ -7272,7 +7277,7 @@ static bool IsOnlyMeaningfulChildOfWrapperPseudo(nsIFrame* aFrame,
     // We can't remove the table if there are any captions present (captions are
     // never anonymous themselves), because table wrapper always relies on
     // having a table frame.
-    if (!wrapper->GetChildList(FrameChildListID::Caption).IsEmpty()) {
+    if (!wrapper->PrincipalChildList().OnlyChild()) {
       return false;
     }
     // Similarly we can't remove the table if there's still a non-anonymous col
@@ -7293,14 +7298,15 @@ static bool IsOnlyMeaningfulChildOfWrapperPseudo(nsIFrame* aFrame,
   }
   if (aFrame->IsTableCaption()) {
     MOZ_ASSERT(aParent->IsTableWrapperFrame());
-    MOZ_ASSERT(aParent->PrincipalChildList().OnlyChild());
+    auto* table = aParent->PrincipalChildList().FirstChild();
-    MOZ_ASSERT(aParent->PrincipalChildList().OnlyChild()->IsTableFrame());
+    MOZ_ASSERT(table);
-    return IsOnlyNonWhitespaceFrameInList(
+    MOZ_ASSERT(table->IsTableFrame());
-               aParent->GetChildList(FrameChildListID::Caption), aFrame) &&
+    return IsOnlyNonWhitespaceFrameInList(aParent->PrincipalChildList(), aFrame,
+                                          /* aIgnoreFrame = */ table) &&
            // This checks for both colgroups and the principal list of the table
            // frame.
            AllChildListsAreEffectivelyEmpty(
-               aParent->PrincipalChildList().OnlyChild());
+               aParent->PrincipalChildList().FirstChild());
   }
   MOZ_ASSERT(!aFrame->IsTableColGroupFrame());
   return IsOnlyNonWhitespaceFrameInList(aParent->PrincipalChildList(), aFrame);
@@ -7855,18 +7861,16 @@ nsIFrame* nsCSSFrameConstructor::CreateContinuingOuterTableFrame(
 
   newFrame->Init(aContent, aParentFrame, aFrame);
 
-  // Create a continuing inner table frame, and if there's a caption then
+  // Create a continuing inner table frame. Note we don't replicate the
-  // replicate the caption
+  // captions: a comment used to hint at that, but the code dealing with that
+  // never worked and was removed in bug 309322.
   nsFrameList newChildFrames;
 
-  nsIFrame* childFrame = aFrame->PrincipalChildList().FirstChild();
+  if (nsIFrame* childFrame = aFrame->PrincipalChildList().FirstChild()) {
-  if (childFrame) {
+    MOZ_ASSERT(childFrame->IsTableFrame());
     nsIFrame* continuingTableFrame =
         CreateContinuingFrame(childFrame, newFrame);
     newChildFrames.AppendFrame(nullptr, continuingTableFrame);
-
-    NS_ASSERTION(!childFrame->GetNextSibling(),
-                 "there can be only one inner table frame");
   }
 
   // Set the table wrapper's initial child list

layout/base/nsLayoutUtils.cpp

@@ -817,8 +817,6 @@ FrameChildListID nsLayoutUtils::GetChildListNameFor(nsIFrame* aChildFrame) {
     LayoutFrameType childType = aChildFrame->Type();
     if (LayoutFrameType::TableColGroup == childType) {
       id = FrameChildListID::ColGroup;
-    } else if (aChildFrame->IsTableCaption()) {
-      id = FrameChildListID::Caption;
     } else {
       id = FrameChildListID::Principal;
     }
@@ -3370,12 +3368,10 @@ void nsLayoutUtils::AddBoxesForFrame(nsIFrame* aFrame,
   auto pseudoType = aFrame->Style()->GetPseudoType();
 
   if (pseudoType == PseudoStyleType::tableWrapper) {
-    AddBoxesForFrame(aFrame->PrincipalChildList().FirstChild(), aCallback);
+    for (nsIFrame* kid : aFrame->PrincipalChildList()) {
-    if (aCallback->mIncludeCaptionBoxForTable) {
-      nsIFrame* kid =
-          aFrame->GetChildList(FrameChildListID::Caption).FirstChild();
-      if (kid) {
       AddBoxesForFrame(kid, aCallback);
+      if (!aCallback->mIncludeCaptionBoxForTable) {
+        break;
       }
     }
   } else if (pseudoType == PseudoStyleType::mozBlockInsideInlineWrapper ||
@@ -3401,26 +3397,11 @@ void nsLayoutUtils::GetAllInFlowBoxes(nsIFrame* aFrame,
 nsIFrame* nsLayoutUtils::GetFirstNonAnonymousFrame(nsIFrame* aFrame) {
   while (aFrame) {
     auto pseudoType = aFrame->Style()->GetPseudoType();
-
+    if (pseudoType == PseudoStyleType::tableWrapper ||
-    if (pseudoType == PseudoStyleType::tableWrapper) {
+        pseudoType == PseudoStyleType::mozBlockInsideInlineWrapper ||
-      nsIFrame* f =
-          GetFirstNonAnonymousFrame(aFrame->PrincipalChildList().FirstChild());
-      if (f) {
-        return f;
-      }
-      nsIFrame* kid =
-          aFrame->GetChildList(FrameChildListID::Caption).FirstChild();
-      if (kid) {
-        f = GetFirstNonAnonymousFrame(kid);
-        if (f) {
-          return f;
-        }
-      }
-    } else if (pseudoType == PseudoStyleType::mozBlockInsideInlineWrapper ||
         pseudoType == PseudoStyleType::mozMathMLAnonymousBlock) {
       for (nsIFrame* kid : aFrame->PrincipalChildList()) {
-        nsIFrame* f = GetFirstNonAnonymousFrame(kid);
+        if (nsIFrame* f = GetFirstNonAnonymousFrame(kid)) {
-        if (f) {
           return f;
         }
       }

layout/generic/nsContainerFrame.cpp

@@ -1865,13 +1865,7 @@ nsIFrame* nsContainerFrame::GetFirstNonAnonBoxInSubtree(nsIFrame* aFrame) {
     // column, we'll always return the column. This is fine; we're really just
     // looking for a handle to *anything* with a meaningful content node inside
     // the table, for use in DOM comparisons to things outside of the table.)
-    if (MOZ_UNLIKELY(aFrame->IsTableWrapperFrame())) {
+    if (MOZ_UNLIKELY(aFrame->IsTableFrame())) {
-      nsIFrame* captionDescendant = GetFirstNonAnonBoxInSubtree(
-          aFrame->GetChildList(FrameChildListID::Caption).FirstChild());
-      if (captionDescendant) {
-        return captionDescendant;
-      }
-    } else if (MOZ_UNLIKELY(aFrame->IsTableFrame())) {
       nsIFrame* colgroupDescendant = GetFirstNonAnonBoxInSubtree(
           aFrame->GetChildList(FrameChildListID::ColGroup).FirstChild());
       if (colgroupDescendant) {

layout/generic/nsFrameList.cpp

@@ -455,8 +455,6 @@ const char* ChildListName(FrameChildListID aListID) {
   switch (aListID) {
     case FrameChildListID::Principal:
       return "";
-    case FrameChildListID::Caption:
-      return "CaptionList";
     case FrameChildListID::ColGroup:
       return "ColGroupList";
     case FrameChildListID::Absolute:

layout/generic/nsFrameList.h

@@ -34,7 +34,6 @@ class FrameChildList;
 enum class FrameChildListID {
   // The individual concrete child lists.
   Principal,
-  Caption,
   ColGroup,
   Absolute,
   Fixed,

layout/tables/nsTableWrapperFrame.cpp

@@ -85,80 +85,32 @@ a11y::AccType nsTableWrapperFrame::AccessibleType() {
 
 void nsTableWrapperFrame::Destroy(DestroyContext& aContext) {
   DestroyAbsoluteFrames(aContext);
-  mCaptionFrames.DestroyFrames(aContext);
   nsContainerFrame::Destroy(aContext);
 }
 
-const nsFrameList& nsTableWrapperFrame::GetChildList(
-    ChildListID aListID) const {
-  if (aListID == FrameChildListID::Caption) {
-    return mCaptionFrames;
-  }
-
-  return nsContainerFrame::GetChildList(aListID);
-}
-
-void nsTableWrapperFrame::GetChildLists(nsTArray<ChildList>* aLists) const {
-  nsContainerFrame::GetChildLists(aLists);
-  mCaptionFrames.AppendIfNonempty(aLists, FrameChildListID::Caption);
-}
-
-void nsTableWrapperFrame::SetInitialChildList(ChildListID aListID,
-                                              nsFrameList&& aChildList) {
-  if (FrameChildListID::Caption == aListID) {
-#ifdef DEBUG
-    nsIFrame::VerifyDirtyBitSet(aChildList);
-    for (nsIFrame* f : aChildList) {
-      MOZ_ASSERT(f->GetParent() == this, "Unexpected parent");
-    }
-#endif
-    // the frame constructor already checked for table-caption display type
-    MOZ_ASSERT(mCaptionFrames.IsEmpty(),
-               "already have child frames in CaptionList");
-    mCaptionFrames = std::move(aChildList);
-  } else {
-    MOZ_ASSERT(FrameChildListID::Principal != aListID ||
-                   (aChildList.FirstChild() &&
-                    aChildList.FirstChild() == aChildList.LastChild() &&
-                    aChildList.FirstChild()->IsTableFrame()),
-               "expected a single table frame in principal child list");
-    nsContainerFrame::SetInitialChildList(aListID, std::move(aChildList));
-  }
-}
-
 void nsTableWrapperFrame::AppendFrames(ChildListID aListID,
                                        nsFrameList&& aFrameList) {
   // We only have two child frames: the inner table and a caption frame.
   // The inner frame is provided when we're initialized, and it cannot change
-  MOZ_ASSERT(FrameChildListID::Caption == aListID, "unexpected child list");
+  MOZ_ASSERT(FrameChildListID::Principal == aListID, "unexpected child list");
   MOZ_ASSERT(aFrameList.IsEmpty() || aFrameList.FirstChild()->IsTableCaption(),
              "appending non-caption frame to captionList");
-  mCaptionFrames.AppendFrames(nullptr, std::move(aFrameList));
+  nsContainerFrame::AppendFrames(aListID, std::move(aFrameList));
-
+  // The presence of caption frames makes us sort our display list differently,
-  // Reflow the new caption frame. It's already marked dirty, so
+  // so mark us as changed for the new ordering.
-  // just tell the pres shell.
-  PresShell()->FrameNeedsReflow(this, IntrinsicDirty::FrameAndAncestors,
-                                NS_FRAME_HAS_DIRTY_CHILDREN);
-  // The presence of caption frames makes us sort our display
-  // list differently, so mark us as changed for the new
-  // ordering.
   MarkNeedsDisplayItemRebuild();
 }
 
 void nsTableWrapperFrame::InsertFrames(
     ChildListID aListID, nsIFrame* aPrevFrame,
     const nsLineList::iterator* aPrevFrameLine, nsFrameList&& aFrameList) {
-  MOZ_ASSERT(FrameChildListID::Caption == aListID, "unexpected child list");
+  MOZ_ASSERT(FrameChildListID::Principal == aListID, "unexpected child list");
   MOZ_ASSERT(aFrameList.IsEmpty() || aFrameList.FirstChild()->IsTableCaption(),
-             "inserting non-caption frame into captionList");
+             "inserting non-caption frame");
   MOZ_ASSERT(!aPrevFrame || aPrevFrame->GetParent() == this,
              "inserting after sibling frame with different parent");
-  mCaptionFrames.InsertFrames(nullptr, aPrevFrame, std::move(aFrameList));
+  nsContainerFrame::InsertFrames(aListID, aPrevFrame, aPrevFrameLine,
-
+                                 std::move(aFrameList));
-  // Reflow the new caption frame. It's already marked dirty, so
-  // just tell the pres shell.
-  PresShell()->FrameNeedsReflow(this, IntrinsicDirty::FrameAndAncestors,
-                                NS_FRAME_HAS_DIRTY_CHILDREN);
   MarkNeedsDisplayItemRebuild();
 }
 
@@ -167,13 +119,8 @@ void nsTableWrapperFrame::RemoveFrame(DestroyContext& aContext,
                                       nsIFrame* aOldFrame) {
   // We only have two child frames: the inner table and one caption frame.
   // The inner frame can't be removed so this should be the caption
-  MOZ_ASSERT(FrameChildListID::Caption == aListID, "can't remove inner frame");
+  MOZ_ASSERT(aOldFrame->IsTableCaption(), "can't remove inner frame");
-
+  nsContainerFrame::RemoveFrame(aContext, aListID, aOldFrame);
-  // Remove the frame and destroy it
-  mCaptionFrames.DestroyFrame(aContext, aOldFrame);
-
-  PresShell()->FrameNeedsReflow(this, IntrinsicDirty::FrameAndAncestors,
-                                NS_FRAME_HAS_DIRTY_CHILDREN);
   MarkNeedsDisplayItemRebuild();
 }
 
@@ -184,17 +131,27 @@ void nsTableWrapperFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
 
   // If there's no caption, take a short cut to avoid having to create
   // the special display list set and then sort it.
-  if (mCaptionFrames.IsEmpty()) {
+  if (nsIFrame* inner = mFrames.OnlyChild()) {
-    BuildDisplayListForInnerTable(aBuilder, aLists);
+    BuildDisplayListForChild(aBuilder, inner, aLists);
     DisplayOutline(aBuilder, aLists);
     return;
   }
 
-  nsDisplayListCollection set(aBuilder);
+  MOZ_ASSERT(mFrames.FirstChild());
-  BuildDisplayListForInnerTable(aBuilder, set);
+  MOZ_ASSERT(mFrames.FirstChild()->IsTableFrame());
 
+  nsDisplayListCollection set(aBuilder);
   nsDisplayListSet captionSet(set, set.BlockBorderBackgrounds());
-  BuildDisplayListForChild(aBuilder, mCaptionFrames.FirstChild(), captionSet);
+  for (auto* frame : mFrames) {
+    const bool isTable = frame->IsTableFrame();
+    auto& setForFrame = isTable ? set : captionSet;
+    BuildDisplayListForChild(aBuilder, frame, setForFrame);
+    if (!isTable) {
+      // FIXME(emilio, bug 144517): Historically we haven't displayed / laid
+      // out multiple captions. This preserves that behavior.
+      break;
+    }
+  }
 
   // Now we have to sort everything by content order, since the caption
   // may be somewhere inside the table.
@@ -211,18 +168,6 @@ void nsTableWrapperFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
   DisplayOutline(aBuilder, aLists);
 }
 
-void nsTableWrapperFrame::BuildDisplayListForInnerTable(
-    nsDisplayListBuilder* aBuilder, const nsDisplayListSet& aLists) {
-  // Just paint the regular children, but the children's background is our
-  // true background (there should only be one, the real table)
-  nsIFrame* kid = mFrames.FirstChild();
-  // The children should be in content order
-  while (kid) {
-    BuildDisplayListForChild(aBuilder, kid, aLists);
-    kid = kid->GetNextSibling();
-  }
-}
-
 ComputedStyle* nsTableWrapperFrame::GetParentComputedStyle(
     nsIFrame** aProviderFrame) const {
   // The table wrapper frame and the (inner) table frame split the style
@@ -261,12 +206,11 @@ nscoord nsTableWrapperFrame::IntrinsicISize(const IntrinsicSizeInput& aInput,
     iSize = std::max(iSize, innerTableMinISize);
   }
 
-  if (mCaptionFrames.NotEmpty()) {
+  if (nsIFrame* caption = GetCaption()) {
     // The table wrapper's intrinsic inline size should be as least as large as
     // caption's min inline size.
     const nscoord capMinISize = nsLayoutUtils::IntrinsicForContainer(
-        aInput.mContext, mCaptionFrames.FirstChild(),
+        aInput.mContext, caption, IntrinsicISizeType::MinISize);
-        IntrinsicISizeType::MinISize);
     iSize = std::max(iSize, capMinISize);
   }
   return iSize;
@@ -321,7 +265,7 @@ LogicalSize nsTableWrapperFrame::CaptionShrinkWrapSize(
     gfxContext* aRenderingContext, nsIFrame* aCaptionFrame, WritingMode aWM,
     const LogicalSize& aCBSize, nscoord aAvailableISize,
     ComputeSizeFlags aFlags) const {
-  MOZ_ASSERT(aCaptionFrame == mCaptionFrames.FirstChild());
+  MOZ_ASSERT(aCaptionFrame != mFrames.FirstChild());
 
   AutoMaybeDisableFontInflation an(aCaptionFrame);
 
@@ -444,8 +388,8 @@ LogicalSize nsTableWrapperFrame::ComputeAutoSize(
     return innerTableSize;
   }
   const LogicalSize captionSize =
-      CaptionShrinkWrapSize(aRenderingContext, mCaptionFrames.FirstChild(), aWM,
+      CaptionShrinkWrapSize(aRenderingContext, GetCaption(), aWM, aCBSize,
-                            aCBSize, innerTableSize.ISize(aWM), flags);
+                            innerTableSize.ISize(aWM), flags);
   const nscoord iSize =
       std::max(innerTableSize.ISize(aWM), captionSize.ISize(aWM));
   nscoord bSize = NS_UNCONSTRAINEDSIZE;
@@ -457,14 +401,14 @@ LogicalSize nsTableWrapperFrame::ComputeAutoSize(
 }
 
 Maybe<StyleCaptionSide> nsTableWrapperFrame::GetCaptionSide() const {
-  if (mCaptionFrames.IsEmpty()) {
+  if (!HasCaption()) {
     return Nothing();
   }
-  return Some(mCaptionFrames.FirstChild()->StyleTableBorder()->mCaptionSide);
+  return Some(GetCaption()->StyleTableBorder()->mCaptionSide);
 }
 
 StyleVerticalAlignKeyword nsTableWrapperFrame::GetCaptionVerticalAlign() const {
-  const auto& va = mCaptionFrames.FirstChild()->StyleDisplay()->mVerticalAlign;
+  const auto& va = GetCaption()->StyleDisplay()->mVerticalAlign;
   return va.IsKeyword() ? va.AsKeyword() : StyleVerticalAlignKeyword::Top;
 }
 
@@ -490,7 +434,7 @@ void nsTableWrapperFrame::GetCaptionOrigin(StyleCaptionSide aCaptionSide,
       (NS_UNCONSTRAINEDSIZE == aCaptionSize.BSize(aWM))) {
     return;
   }
-  if (mCaptionFrames.IsEmpty()) {
+  if (!HasCaption()) {
     return;
   }
 
@@ -630,7 +574,7 @@ void nsTableWrapperFrame::CreateReflowInputForCaption(
     nsPresContext* aPresContext, nsIFrame* aCaptionFrame,
     const ReflowInput& aOuterRI, Maybe<ReflowInput>& aChildRI,
     const nscoord aAvailISize) const {
-  MOZ_ASSERT(aCaptionFrame == mCaptionFrames.FirstChild());
+  MOZ_ASSERT(aCaptionFrame == GetCaption());
 
   const WritingMode wm = aCaptionFrame->GetWritingMode();
 
@@ -680,9 +624,8 @@ void nsTableWrapperFrame::ReflowChild(nsPresContext* aPresContext,
 
 void nsTableWrapperFrame::UpdateOverflowAreas(ReflowOutput& aMet) {
   aMet.SetOverflowAreasToDesiredBounds();
-  ConsiderChildOverflow(aMet.mOverflowAreas, InnerTableFrame());
+  for (auto* frame : mFrames) {
-  if (mCaptionFrames.NotEmpty()) {
+    ConsiderChildOverflow(aMet.mOverflowAreas, frame);
-    ConsiderChildOverflow(aMet.mOverflowAreas, mCaptionFrames.FirstChild());
   }
 }
 
@@ -709,11 +652,10 @@ void nsTableWrapperFrame::Reflow(nsPresContext* aPresContext,
   nsRect origCaptionRect;
   nsRect origCaptionInkOverflow;
   bool captionFirstReflow = false;
-  if (mCaptionFrames.NotEmpty()) {
+  if (nsIFrame* caption = GetCaption()) {
-    origCaptionRect = mCaptionFrames.FirstChild()->GetRect();
+    origCaptionRect = caption->GetRect();
-    origCaptionInkOverflow = mCaptionFrames.FirstChild()->InkOverflowRect();
+    origCaptionInkOverflow = caption->InkOverflowRect();
-    captionFirstReflow =
+    captionFirstReflow = caption->HasAnyStateBits(NS_FRAME_FIRST_REFLOW);
-        mCaptionFrames.FirstChild()->HasAnyStateBits(NS_FRAME_FIRST_REFLOW);
   }
 
   // ComputeAutoSize has to match this logic.
@@ -721,7 +663,7 @@ void nsTableWrapperFrame::Reflow(nsPresContext* aPresContext,
   Maybe<StyleCaptionSide> captionSide = GetCaptionSide();
   const nscoord contentBoxISize = aOuterRI.ComputedSize(wm).ISize(wm);
 
-  MOZ_ASSERT(mCaptionFrames.NotEmpty() == captionSide.isSome());
+  MOZ_ASSERT(HasCaption() == captionSide.isSome());
 
   // Compute the table's size first, and then prevent the caption from
   // being larger in the inline dir unless it has to be.
@@ -745,14 +687,13 @@ void nsTableWrapperFrame::Reflow(nsPresContext* aPresContext,
     // advantage of that later when we call GetCaptionOrigin, though.)
     nscoord innerBorderISize =
         innerRI->ComputedSizeWithBorderPadding(wm).ISize(wm);
-    CreateReflowInputForCaption(aPresContext, mCaptionFrames.FirstChild(),
+    CreateReflowInputForCaption(aPresContext, GetCaption(), aOuterRI, captionRI,
-                                aOuterRI, captionRI, innerBorderISize);
+                                innerBorderISize);
 
     // We intentionally don't merge capStatus into aStatus, since we currently
     // can't handle caption continuations, but we probably should.
     nsReflowStatus capStatus;
-    ReflowChild(aPresContext, mCaptionFrames.FirstChild(), *captionRI,
+    ReflowChild(aPresContext, GetCaption(), *captionRI, captionMet, capStatus);
-                captionMet, capStatus);
     captionSize = captionMet.Size(wm);
     captionMargin = captionRI->ComputedLogicalMargin(wm);
     nscoord bSizeOccupiedByCaption =
@@ -787,13 +728,13 @@ void nsTableWrapperFrame::Reflow(nsPresContext* aPresContext,
   aDesiredSize.SetSize(wm, desiredSize);
   nsSize containerSize = aDesiredSize.PhysicalSize();
 
-  MOZ_ASSERT(mCaptionFrames.NotEmpty() == captionSide.isSome());
+  MOZ_ASSERT(HasCaption() == captionSide.isSome());
-  if (mCaptionFrames.NotEmpty()) {
+  if (nsIFrame* caption = GetCaption()) {
     LogicalPoint captionOrigin(wm);
     GetCaptionOrigin(*captionSide, innerSize, captionSize, captionMargin,
                      captionOrigin, wm);
-    FinishReflowChild(mCaptionFrames.FirstChild(), aPresContext, captionMet,
+    FinishReflowChild(caption, aPresContext, captionMet, captionRI.ptr(), wm,
-                      captionRI.ptr(), wm, captionOrigin, containerSize,
+                      captionOrigin, containerSize,
                       ReflowChildFlags::ApplyRelativePositioning);
     captionRI.reset();
   }
@@ -808,9 +749,9 @@ void nsTableWrapperFrame::Reflow(nsPresContext* aPresContext,
                     wm, innerOrigin, containerSize, ReflowChildFlags::Default);
   innerRI.reset();
 
-  if (mCaptionFrames.NotEmpty()) {
+  if (HasCaption()) {
-    nsTableFrame::InvalidateTableFrame(mCaptionFrames.FirstChild(),
+    nsTableFrame::InvalidateTableFrame(GetCaption(), origCaptionRect,
-                                       origCaptionRect, origCaptionInkOverflow,
+                                       origCaptionInkOverflow,
                                        captionFirstReflow);
   }
 

layout/tables/nsTableWrapperFrame.h

@@ -39,11 +39,6 @@ class nsTableWrapperFrame : public nsContainerFrame {
 
   void Destroy(DestroyContext&) override;
 
-  const nsFrameList& GetChildList(ChildListID aListID) const override;
-  void GetChildLists(nsTArray<ChildList>* aLists) const override;
-
-  void SetInitialChildList(ChildListID aListID,
-                           nsFrameList&& aChildList) override;
   void AppendFrames(ChildListID aListID, nsFrameList&& aFrameList) override;
   void InsertFrames(ChildListID aListID, nsIFrame* aPrevFrame,
                     const nsLineList::iterator* aPrevFrameLine,
@@ -61,9 +56,6 @@ class nsTableWrapperFrame : public nsContainerFrame {
   void BuildDisplayList(nsDisplayListBuilder* aBuilder,
                         const nsDisplayListSet& aLists) override;
 
-  void BuildDisplayListForInnerTable(nsDisplayListBuilder* aBuilder,
-                                     const nsDisplayListSet& aLists);
-
   nscoord SynthesizeFallbackBaseline(
       mozilla::WritingMode aWM,
       BaselineSharingGroup aBaselineGroup) const override;
@@ -171,9 +163,13 @@ class nsTableWrapperFrame : public nsContainerFrame {
     return map->GetEffectiveRowSpan(aRowIdx, aColIdx);
   }
 
+  bool HasCaption() const { return !mFrames.OnlyChild(); }
+  nsIFrame* GetCaption() const {
+    return HasCaption() ? mFrames.FirstChild()->GetNextSibling() : nullptr;
+  }
+
  protected:
-  explicit nsTableWrapperFrame(ComputedStyle* aStyle,
+  nsTableWrapperFrame(ComputedStyle* aStyle, nsPresContext* aPresContext,
-                               nsPresContext* aPresContext,
                       ClassID aID = kClassID);
   virtual ~nsTableWrapperFrame();
 
@@ -276,9 +272,6 @@ class nsTableWrapperFrame : public nsContainerFrame {
       const mozilla::StyleSizeOverrides& aWrapperSizeOverrides,
       const mozilla::LogicalSize& aBorderPadding,
       nscoord aBSizeOccupiedByCaption) const;
-
- private:
-  nsFrameList mCaptionFrames;
 };
 
 #endif

testing/web-platform/tests/html/interaction/focus/tab-table-caption.html

@@ -0,0 +1,57 @@
+<!doctype html>
+<meta charset="utf-8">
+<meta name="variant" content="?caption-side=top">
+<meta name="variant" content="?caption-side=bottom">
+<title>Tab navigation around table with caption</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/resources/testdriver.js"></script>
+<script src="/resources/testdriver-vendor.js"></script>
+<script src="/resources/testdriver-actions.js"></script>
+<script>
+  "use strict";
+
+  const searchParams = new URLSearchParams(document.location.search);
+  const captionSide = searchParams.get("caption-side");
+
+  addEventListener("DOMContentLoaded", () => {
+    document.querySelector("table").style.captionSide = captionSide;
+    const tabKey = "\uE004";
+    const shiftKey = "\uE008";
+    const firstTabbable = document.querySelector("body > span");
+    const lastTabbable = document.querySelector("table ~ span");
+    const tabbableInCaption = document.querySelector("caption > span");
+    const tabbableInCell = document.querySelector("td > span");
+    for (const data of [
+      {init: firstTabbable, prev: null, next: tabbableInCell },
+      {init: tabbableInCaption, prev: tabbableInCell, next: lastTabbable },
+      {init: tabbableInCell, prev: firstTabbable, next: tabbableInCaption },
+      {init: lastTabbable, prev: tabbableInCaption, next: null},
+    ]) {
+      if (data.prev) {
+        promise_test(async () => {
+          data.init.focus();
+          await new test_driver.Actions().keyDown(shiftKey).keyDown(tabKey).keyUp(tabKey).keyUp(shiftKey).send();
+          assert_equals(document.activeElement, data.prev);
+        }, `Shift+Tab on ${data.init.outerHTML} should move focus to ${data.prev.outerHTML}`);
+      }
+      if (data.next) {
+        promise_test(async () => {
+          data.init.focus();
+          await new test_driver.Actions().keyDown(tabKey).keyUp(tabKey).send();
+          assert_equals(document.activeElement, data.next);
+        }, `Tab on ${data.init.outerHTML} should move focus to ${data.next.outerHTML}`);
+      }
+    }
+  });
+</script>
+<span tabindex="0">First tabbable span</span>
+<table>
+  <tbody>
+    <tr>
+      <td><span tabindex="0">Tabbable in cell<span></td>
+    </tr>
+  </tbody>
+  <caption><span tabindex="0">Tabbable in caption</span></caption>
+</table>
+<span tabindex="0">Last tabbable span</span>