Bug 975644: Enable position sticky in table parts. r?bz draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 29 Nov 2017 16:11:25 +0100
changeset 706643 b0d8abf05b766f8246582a9d3f7d5658dd2e7c1a
parent 706642 ee1009c5b8ec09345871e4bdfd37306ec850eaad
child 706695 60268de43b6797fe9d9be895ce5e355aed14bed0
push id91865
push userbmo:emilio@crisal.io
push dateSat, 02 Dec 2017 23:48:17 +0000
reviewersbz
bugs975644
milestone59.0a1
Bug 975644: Enable position sticky in table parts. r?bz MozReview-Commit-ID: 85nCuChHdTa
layout/base/RestyleManager.cpp
layout/generic/StickyScrollContainer.cpp
layout/generic/nsFrame.cpp
layout/reftests/position-sticky/inner-table-1-ref.html
layout/reftests/position-sticky/inner-table-1.html
layout/reftests/position-sticky/reftest.list
layout/tables/nsTableRowFrame.cpp
testing/web-platform/meta/css/css-position/position-sticky-table-tfoot-bottom.html.ini
testing/web-platform/meta/css/css-position/position-sticky-table-th-bottom.html.ini
testing/web-platform/meta/css/css-position/position-sticky-table-th-left.html.ini
testing/web-platform/meta/css/css-position/position-sticky-table-th-right.html.ini
testing/web-platform/meta/css/css-position/position-sticky-table-th-top.html.ini
testing/web-platform/meta/css/css-position/position-sticky-table-thead-top.html.ini
testing/web-platform/meta/css/css-position/position-sticky-table-tr-bottom.html.ini
testing/web-platform/meta/css/css-position/position-sticky-table-tr-top.html.ini
--- a/layout/base/RestyleManager.cpp
+++ b/layout/base/RestyleManager.cpp
@@ -739,25 +739,16 @@ RecomputePosition(nsIFrame* aFrame)
   }
 
   aFrame->SchedulePaint();
 
   // For relative positioning, we can simply update the frame rect
   if (display->IsRelativelyPositionedStyle()) {
     // Move the frame
     if (display->mPosition == NS_STYLE_POSITION_STICKY) {
-      if (display->IsInnerTableStyle()) {
-        // We don't currently support sticky positioning of inner table
-        // elements (bug 975644). Bail.
-        //
-        // When this is fixed, remove the null-check for the computed
-        // offsets in nsTableRowFrame::ReflowChildren.
-        return true;
-      }
-
       // Update sticky positioning for an entire element at once, starting with
       // the first continuation or ib-split sibling.
       // It's rare that the frame we already have isn't already the first
       // continuation or ib-split sibling, but it can happen when styles differ
       // across continuations such as ::first-line or ::first-letter, and in
       // those cases we will generally (but maybe not always) do the work twice.
       nsIFrame* firstContinuation =
         nsLayoutUtils::FirstContinuationOrIBSplitSibling(aFrame);
--- a/layout/generic/StickyScrollContainer.cpp
+++ b/layout/generic/StickyScrollContainer.cpp
@@ -40,17 +40,17 @@ StickyScrollContainer::GetStickyScrollCo
     nsLayoutUtils::GetNearestScrollableFrame(aFrame->GetParent(),
       nsLayoutUtils::SCROLLABLE_SAME_DOC |
       nsLayoutUtils::SCROLLABLE_INCLUDE_HIDDEN);
   if (!scrollFrame) {
     // We might not find any, for instance in the case of
     // <html style="position: fixed">
     return nullptr;
   }
-  auto frame = static_cast<nsIFrame*>(do_QueryFrame(scrollFrame));
+  nsIFrame* frame = do_QueryFrame(scrollFrame);
   StickyScrollContainer* s =
     frame->GetProperty(StickyScrollContainerProperty());
   if (!s) {
     s = new StickyScrollContainer(scrollFrame);
     frame->SetProperty(StickyScrollContainerProperty(), s);
   }
   return s;
 }
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -685,25 +685,22 @@ nsFrame::Init(nsIContent*       aContent
       (IsFrameOfType(eSupportsCSSTransforms) &&
        nsLayoutUtils::HasAnimationOfProperty(this, eCSSProperty_transform))) {
     // The frame gets reconstructed if we toggle the -moz-transform
     // property, so we can set this bit here and then ignore it.
     AddStateBits(NS_FRAME_MAY_BE_TRANSFORMED);
   }
   if (disp->mPosition == NS_STYLE_POSITION_STICKY &&
       !aPrevInFlow &&
-      !(mState & NS_FRAME_IS_NONDISPLAY) &&
-      !disp->IsInnerTableStyle()) {
+      !(mState & NS_FRAME_IS_NONDISPLAY)) {
     // Note that we only add first continuations, but we really only
     // want to add first continuation-or-ib-split-siblings.  But since we
     // don't yet know if we're a later part of a block-in-inline split,
     // we'll just add later members of a block-in-inline split here, and
     // then StickyScrollContainer will remove them later.
-    // We don't currently support relative positioning of inner table
-    // elements (bug 35168), so exclude them from sticky positioning too.
     StickyScrollContainer* ssc =
       StickyScrollContainer::GetStickyScrollContainerForFrame(this);
     if (ssc) {
       ssc->AddFrame(this);
     }
   }
 
   if (nsLayoutUtils::FontSizeInflationEnabled(PresContext()) || !GetParent()
@@ -721,18 +718,17 @@ nsFrame::Init(nsIContent*       aContent
         AddStateBits(NS_FRAME_FONT_INFLATION_FLOW_ROOT);
       }
     }
     NS_ASSERTION(GetParent() ||
                  (GetStateBits() & NS_FRAME_FONT_INFLATION_CONTAINER),
                  "root frame should always be a container");
   }
 
-  if (PresShell()->AssumeAllFramesVisible() &&
-      TrackingVisibility()) {
+  if (PresShell()->AssumeAllFramesVisible() && TrackingVisibility()) {
     IncApproximateVisibleCount();
   }
 
   DidSetStyleContext(nullptr);
 
   if (::IsXULBoxWrapped(this))
     ::InitBoxMetrics(this, false);
 }
deleted file mode 100644
--- a/layout/reftests/position-sticky/inner-table-1-ref.html
+++ /dev/null
@@ -1,26 +0,0 @@
-<!DOCTYPE html>
-<!-- Any copyright is dedicated to the Public Domain.
-   - http://creativecommons.org/publicdomain/zero/1.0/ -->
-<html>
-  <head>
-    <link rel="author" title="Corey Ford" href="mailto:corey@coreyford.name">
-  </head>
-  <body>
-    <table>
-      <thead>
-        <tr>
-          <td>a</td>
-        </tr>
-      </thead>
-      <tr>
-        <td>b</td>
-      </tr>
-      <tr>
-        <td>c</td>
-      </tr>
-      <tr>
-        <td>d</td>
-      </tr>
-    </table>
-  </body>
-</html>
deleted file mode 100644
--- a/layout/reftests/position-sticky/inner-table-1.html
+++ /dev/null
@@ -1,35 +0,0 @@
-<!DOCTYPE html>
-<!-- Any copyright is dedicated to the Public Domain.
-   - http://creativecommons.org/publicdomain/zero/1.0/ -->
-<html>
-  <head>
-    <title>CSS Test: Sticky Positioning - inner table elements</title>
-    <link rel="author" title="Corey Ford" href="mailto:corey@coreyford.name">
-    <link rel="match" href="inner-table-1-ref.html">
-    <meta name="assert" content="Sticky positioning on inner table elements should have no effect (until bug 35168 is fixed)">
-    <style>
-      .sticky {
-        position: sticky;
-        top: 1000px;
-      }
-    </style>
-  </head>
-  <body>
-    <table>
-      <thead class="sticky">
-        <tr>
-          <td>a</td>
-        </tr>
-      </thead>
-      <tr class="sticky">
-        <td>b</td>
-      </tr>
-      <tr>
-        <td class="sticky">c</td>
-      </tr>
-      <tr>
-        <td>d</td>
-      </tr>
-    </table>
-  </body>
-</html>
--- a/layout/reftests/position-sticky/reftest.list
+++ b/layout/reftests/position-sticky/reftest.list
@@ -43,10 +43,9 @@ fuzzy-if(OSX,99,210) == inline-3.html in
 skip-if(!asyncPan) == inline-4.html inline-4-ref.html
 fails == column-contain-1a.html column-contain-1-ref.html
 == column-contain-1b.html column-contain-1-ref.html
 == column-contain-2.html column-contain-2-ref.html
 == block-in-inline-1.html block-in-inline-1-ref.html
 fuzzy-if(skiaContent,1,22) fuzzy-if(winWidget&&!layersGPUAccelerated,116,1320) fuzzy-if(Android,8,1533) == block-in-inline-2.html block-in-inline-2-ref.html
 fuzzy-if(Android,8,630) fuzzy-if(OSX,1,11) fuzzy-if(skiaContent,1,220) fuzzy-if(winWidget&&!layersGPUAccelerated,116,1320) == block-in-inline-3.html block-in-inline-3-ref.html
 == block-in-inline-continuations.html block-in-inline-continuations-ref.html
-fuzzy-if(winWidget&&!layersGPUAccelerated,140,140) == inner-table-1.html inner-table-1-ref.html
 == iframe-1.html iframe-1-ref.html
--- a/layout/tables/nsTableRowFrame.cpp
+++ b/layout/tables/nsTableRowFrame.cpp
@@ -936,22 +936,23 @@ nsTableRowFrame::ReflowChildren(nsPresCo
         // We reflowed. Apply relative positioning in the normal way.
         kidReflowInput->ApplyRelativePositioning(&kidPosition, containerSize);
       } else if (kidFrame->IsRelativelyPositioned()) {
         // We didn't reflow.  Do the positioning part of what
         // MovePositionBy does internally.  (This codepath should really
         // be merged into the else below if we can.)
         nsMargin* computedOffsetProp =
           kidFrame->GetProperty(nsIFrame::ComputedOffsetProperty());
-        // Bug 975644: a position:sticky kid can end up with a null
-        // property value here.
-        LogicalMargin computedOffsets(wm, computedOffsetProp ?
-                                            *computedOffsetProp : nsMargin());
-        ReflowInput::ApplyRelativePositioning(kidFrame, wm, computedOffsets,
-                                                    &kidPosition, containerSize);
+
+        // On our fist reflow sticky children may not have the property yet (we
+        // need to reflow the children first to size the scroll frame).
+        LogicalMargin computedOffsets(
+          wm, computedOffsetProp ? *computedOffsetProp : nsMargin());
+        ReflowInput::ApplyRelativePositioning(
+            kidFrame, wm, computedOffsets, &kidPosition, containerSize);
       }
 
       // In vertical-rl mode, we are likely to have containerSize.width = 0
       // because ComputedWidth() was NS_UNCONSTRAINEDSIZE.
       // For cases where that's wrong, we will fix up the position later.
       FinishReflowChild(kidFrame, aPresContext, desiredSize, nullptr,
                         wm, kidPosition, containerSize, 0);
 
deleted file mode 100644
--- a/testing/web-platform/meta/css/css-position/position-sticky-table-tfoot-bottom.html.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[position-sticky-table-tfoot-bottom.html]
-  type: reftest
-  expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/css/css-position/position-sticky-table-th-bottom.html.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[position-sticky-table-th-bottom.html]
-  type: reftest
-  expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/css/css-position/position-sticky-table-th-left.html.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[position-sticky-table-th-left.html]
-  type: reftest
-  expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/css/css-position/position-sticky-table-th-right.html.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[position-sticky-table-th-right.html]
-  type: reftest
-  expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/css/css-position/position-sticky-table-th-top.html.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[position-sticky-table-th-top.html]
-  type: reftest
-  expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/css/css-position/position-sticky-table-thead-top.html.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[position-sticky-table-thead-top.html]
-  type: reftest
-  expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/css/css-position/position-sticky-table-tr-bottom.html.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[position-sticky-table-tr-bottom.html]
-  type: reftest
-  expected: FAIL
deleted file mode 100644
--- a/testing/web-platform/meta/css/css-position/position-sticky-table-tr-top.html.ini
+++ /dev/null
@@ -1,3 +0,0 @@
-[position-sticky-table-tr-top.html]
-  type: reftest
-  expected: FAIL