Bug 975644: Add a hack to skip table row groups for sticky positioning. r?bz draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 01 Dec 2017 21:29:05 +0100
changeset 706642 ee1009c5b8ec09345871e4bdfd37306ec850eaad
parent 706635 679bc0250123c5e696c94109666b035a33b10a1b
child 706643 b0d8abf05b766f8246582a9d3f7d5658dd2e7c1a
push id91865
push userbmo:emilio@crisal.io
push dateSat, 02 Dec 2017 23:48:17 +0000
reviewersbz
bugs975644
milestone59.0a1
Bug 975644: Add a hack to skip table row groups for sticky positioning. r?bz This matches Blink's behavior. Just skipping table row groups from being containing blocks makes layout/reftests/table-overflow/table-cell-block-overflow.html render differently (and way more different than any other browser, actually...), so I avoided doing that. Though I'm not really proud of this one, better ideas welcome. Maybe I should just fix table layout so that we agree with WebKit / Blink... But that seemed harder, too. MozReview-Commit-ID: AkUB4MFzwZK
layout/generic/StickyScrollContainer.cpp
layout/generic/nsFrame.cpp
--- a/layout/generic/StickyScrollContainer.cpp
+++ b/layout/generic/StickyScrollContainer.cpp
@@ -172,16 +172,25 @@ StickyScrollContainer::ComputeStickyLimi
   nsIFrame* cbFrame = aFrame->GetContainingBlock();
   NS_ASSERTION(cbFrame == scrolledFrame ||
     nsLayoutUtils::IsProperAncestorFrame(scrolledFrame, cbFrame),
     "Scroll frame should be an ancestor of the containing block");
 
   nsRect rect =
     nsLayoutUtils::GetAllInFlowRectsUnion(aFrame, aFrame->GetParent());
 
+  // FIXME(bug 1421660): Table row groups aren't supposed to be containing
+  // blocks, but we treat them as such (maybe it's the right thing to do!).
+  // Anyway, not having this basically disables position: sticky on table cells,
+  // which would be really unfortunate, and doesn't match what other browsers
+  // do.
+  if (cbFrame != scrolledFrame && cbFrame->IsTableRowGroupFrame()) {
+    cbFrame = cbFrame->GetContainingBlock();
+  }
+
   // Containing block limits for the position of aFrame relative to its parent.
   // The margin box of the sticky element stays within the content box of the
   // contaning-block element.
   if (cbFrame != scrolledFrame) {
     *aContain = nsLayoutUtils::
       GetAllInFlowRectsUnion(cbFrame, aFrame->GetParent(),
                              nsLayoutUtils::RECTS_USE_CONTENT_BOX);
     nsRect marginRect = nsLayoutUtils::
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -7531,16 +7531,19 @@ static nsIFrame*
 GetNearestBlockContainer(nsIFrame* frame)
 {
   // The block wrappers we use to wrap blocks inside inlines aren't
   // described in the CSS spec.  We need to make them not be containing
   // blocks.
   // Since the parent of such a block is either a normal block or
   // another such pseudo, this shouldn't cause anything bad to happen.
   // Also the anonymous blocks inside table cells are not containing blocks.
+  //
+  // If we ever start skipping table row groups from being containing blocks,
+  // you need to remove the StickyScrollContainer hack referencing bug 1421660.
   while (frame->IsFrameOfType(nsIFrame::eLineParticipant) ||
          frame->IsBlockWrapper() ||
          // Table rows are not containing blocks either
          frame->IsTableRowFrame()) {
     frame = frame->GetParent();
     NS_ASSERTION(frame, "How come we got to the root frame without seeing a containing block?");
   }
   return frame;