Bug 904197 - Use the union of continuations' rects in sticky positioning calculations. r=dholbert

Reftest inline-3.html fails because handling the sticky element's margin correctly will take more work; similarly, border/padding on containing-block continuations won't be handled quite right. Reftest column-contain-1a fails because some of the anonymous blocks inside an nsColumnSetFrame have 0 height.

layout/base/RestyleManager.cpp

@@ -350,7 +350,16 @@ RestyleManager::RecomputePosition(nsIFrame* aFrame)
 
     // Move the frame
     if (display->mPosition == NS_STYLE_POSITION_STICKY) {
-      StickyScrollContainer::ComputeStickyOffsets(aFrame);
+      // Update sticky positioning for an entire element at once when
+      // RecomputePosition is called with the first continuation in a chain.
+      if (!aFrame->GetPrevContinuation()) {
+        StickyScrollContainer::ComputeStickyOffsets(aFrame);
+        StickyScrollContainer* ssc =
+          StickyScrollContainer::GetStickyScrollContainerForFrame(aFrame);
+        if (ssc) {
+          ssc->PositionContinuations(aFrame);
+        }
+      }
     } else {
       MOZ_ASSERT(NS_STYLE_POSITION_RELATIVE == display->mPosition,
                  "Unexpected type of positioning");
@@ -362,14 +371,14 @@ RestyleManager::RecomputePosition(nsIFrame* aFrame)
       NS_ASSERTION(newOffsets.left == -newOffsets.right &&
                    newOffsets.top == -newOffsets.bottom,
                    "ComputeRelativeOffsets should return valid results");
+
+      // nsHTMLReflowState::ApplyRelativePositioning would work here, but
+      // since we've already checked mPosition and aren't changing the frame's
+      // normal position, go ahead and add the offsets directly.
+      aFrame->SetPosition(aFrame->GetNormalPosition() +
+                          nsPoint(newOffsets.left, newOffsets.top));
     }
 
-    nsPoint position = aFrame->GetNormalPosition();
-
-    // This handles both relative and sticky positioning.
-    nsHTMLReflowState::ApplyRelativePositioning(aFrame, newOffsets, &position);
-    aFrame->SetPosition(position);
-
     return true;
   }
 

layout/generic/StickyScrollContainer.cpp

@@ -129,6 +129,9 @@ void
 StickyScrollContainer::ComputeStickyLimits(nsIFrame* aFrame, nsRect* aStick,
                                            nsRect* aContain) const
 {
+  NS_ASSERTION(!aFrame->GetPrevContinuation(),
+               "Can't sticky position individual continuations");
+
   aStick->SetRect(nscoord_MIN/2, nscoord_MIN/2, nscoord_MAX, nscoord_MAX);
   aContain->SetRect(nscoord_MIN/2, nscoord_MIN/2, nscoord_MAX, nscoord_MAX);
 
@@ -146,17 +149,16 @@ StickyScrollContainer::ComputeStickyLimits(nsIFrame* aFrame, nsRect* aStick,
     nsLayoutUtils::IsProperAncestorFrame(scrolledFrame, cbFrame),
     "Scroll frame should be an ancestor of the containing block");
 
-  nsRect rect = aFrame->GetRect();
-  nsMargin margin = aFrame->GetUsedMargin();
+  nsRect rect =
+    nsLayoutUtils::GetAllInFlowRectsUnion(aFrame, aFrame->GetParent());
 
   // Containing block limits
   if (cbFrame != scrolledFrame) {
-    nsMargin cbBorderPadding = cbFrame->GetUsedBorderAndPadding();
-    aContain->SetRect(nsPoint(cbBorderPadding.left, cbBorderPadding.top) -
-                      aFrame->GetParent()->GetOffsetTo(cbFrame),
-                      cbFrame->GetContentRectRelativeToSelf().Size() -
-                      rect.Size());
-    aContain->Deflate(margin);
+    *aContain = nsLayoutUtils::GetAllInFlowRectsUnion(cbFrame, cbFrame);
+    aContain->MoveBy(-aFrame->GetParent()->GetOffsetTo(cbFrame));
+    aContain->Deflate(cbFrame->GetUsedBorderAndPadding());
+    aContain->Deflate(aFrame->GetUsedMargin());
+    aContain->Deflate(nsMargin(0, rect.width, rect.height, 0));
   }
 
   nsMargin sfPadding = scrolledFrame->GetUsedPadding();
@@ -197,6 +199,10 @@ StickyScrollContainer::ComputeStickyLimits(nsIFrame* aFrame, nsRect* aStick,
     aStick->SetRightEdge(mScrollPosition.x + sfPadding.left + sfSize.width -
                          computedOffsets->right - rect.width - sfOffset.x);
   }
+
+  // These limits are for the bounding box of aFrame's continuations. Convert
+  // to limits for aFrame itself.
+  aStick->MoveBy(aFrame->GetPosition() - rect.TopLeft());
 }
 
 nsPoint
@@ -255,6 +261,22 @@ StickyScrollContainer::GetScrollRanges(nsIFrame* aFrame, nsRect* aOuter,
   }
 }
 
+void
+StickyScrollContainer::PositionContinuations(nsIFrame* aFrame)
+{
+  NS_ASSERTION(!aFrame->GetPrevContinuation(),
+               "Should be starting from the first continuation");
+  nsPoint newPosition = ComputePosition(aFrame);
+  nsPoint translation = newPosition - aFrame->GetPosition();
+  aFrame->SetPosition(newPosition);
+
+  // Move all continuation frames by the same amount.
+  for (nsIFrame* cont = aFrame->GetNextContinuation(); cont;
+       cont = cont->GetNextContinuation()) {
+    cont->SetPosition(cont->GetPosition() + translation);
+  }
+}
+
 void
 StickyScrollContainer::UpdatePositions(nsPoint aScrollPosition,
                                        nsIFrame* aSubtreeRoot)
@@ -272,12 +294,18 @@ StickyScrollContainer::UpdatePositions(nsPoint aScrollPosition,
   oct.SetSubtreeRoot(aSubtreeRoot);
   for (nsTArray<nsIFrame*>::size_type i = 0; i < mFrames.Length(); i++) {
     nsIFrame* f = mFrames[i];
+
     if (aSubtreeRoot) {
       // Reflowing the scroll frame, so recompute offsets.
       ComputeStickyOffsets(f);
     }
-    f->SetPosition(ComputePosition(f));
-    oct.AddFrame(f);
+    // mFrames will only contain first continuations, because we filter in
+    // nsIFrame::Init.
+    PositionContinuations(f);
+
+    for (nsIFrame* cont = f; cont; cont = cont->GetNextContinuation()) {
+      oct.AddFrame(cont);
+    }
   }
   oct.Flush();
 }

layout/generic/StickyScrollContainer.h

@@ -63,6 +63,11 @@ public:
    */
   void GetScrollRanges(nsIFrame* aFrame, nsRect* aOuter, nsRect* aInner) const;
 
+  /**
+   * Compute and set the position of a frame and its following continuations.
+   */
+  void PositionContinuations(nsIFrame* aFrame);
+
   /**
    * Compute and set the position of all sticky frames, given the current
    * scroll position of the scroll frame. If not in reflow, aSubtreeRoot should

layout/generic/nsFrame.cpp

@@ -512,6 +512,7 @@ nsFrame::Init(nsIContent*      aContent,
     mState |= NS_FRAME_MAY_BE_TRANSFORMED;
   }
   if (disp->mPosition == NS_STYLE_POSITION_STICKY &&
+      !aPrevInFlow &&
       !(mState & NS_FRAME_IS_NONDISPLAY)) {
     StickyScrollContainer* ssc =
       StickyScrollContainer::GetStickyScrollContainerForFrame(this);

layout/generic/nsHTMLReflowState.cpp

@@ -871,7 +871,14 @@ nsHTMLReflowState::ApplyRelativePositioning(nsIFrame* aFrame,
   const nsStyleDisplay* display = aFrame->StyleDisplay();
   if (NS_STYLE_POSITION_RELATIVE == display->mPosition) {
     *aPosition += nsPoint(aComputedOffsets.left, aComputedOffsets.top);
-  } else if (NS_STYLE_POSITION_STICKY == display->mPosition) {
+  } else if (NS_STYLE_POSITION_STICKY == display->mPosition &&
+             !aFrame->GetNextContinuation() && !aFrame->GetPrevContinuation()) {
+    // Sticky positioning for elements with multiple frames needs to be
+    // computed all at once. We can't safely do that here because we might be
+    // partway through (re)positioning the frames, so leave it until the scroll
+    // container reflows and calls StickyScrollContainer::UpdatePositions.
+    // For single-frame sticky positioned elements, though, go ahead and apply
+    // it now to avoid unnecessary overflow updates later.
     StickyScrollContainer* ssc =
       StickyScrollContainer::GetStickyScrollContainerForFrame(aFrame);
     if (ssc) {

layout/reftests/position-sticky/column-contain-1-ref.html

@@ -0,0 +1,42 @@
+<!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:cford@mozilla.com">
+    <style>
+      #scroll {
+        overflow: hidden;
+        height: 200px;
+        line-height: 1;
+      }
+      #contain {
+        -moz-column-count: 2;
+        column-count: 2;
+        -moz-column-rule: 1px solid black;
+        column-rule: 1px solid black;
+        height: 200px;
+        width: 200px;
+      }
+      #fill {
+        height: 390px;
+        background: blue;
+      }
+      #sticky {
+        position: relative;
+        left: 190px;
+        width: 10px;
+        height: 10px;
+        background: black;
+      }
+    </style>
+  </head>
+  <body>
+    <div id="scroll">
+      <div id="contain">
+        <div id="sticky"></div>
+        <div id="fill"></div>
+      </div>
+    </div>
+  </body>
+</html>

layout/reftests/position-sticky/column-contain-1a.html

@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+<html>
+  <head>
+    <title>CSS Test: Sticky Positioning - multiframe containing-block element</title>
+    <link rel="author" title="Corey Ford" href="mailto:cford@mozilla.com">
+    <link rel="match" href="column-contain-1-ref.html">
+    <meta name="assert" content="Sticky positioning with a multiple-frame containing-block element should stay contained within the bounding box of all that element's frames">
+    <style>
+      #scroll {
+        overflow: hidden;
+        height: 200px;
+        line-height: 1;
+      }
+      #contain {
+        -moz-column-count: 2;
+        column-count: 2;
+        -moz-column-rule: 1px solid black;
+        column-rule: 1px solid black;
+        height: 200px;
+        width: 200px;
+      }
+      #fill {
+        height: 390px;
+        background: blue;
+      }
+      #sticky {
+        position: sticky;
+        left: 190px;
+        width: 10px;
+        height: 10px;
+        background: black;
+      }
+    </style>
+  </head>
+  <body>
+    <div id="scroll">
+      <div id="contain">
+        <div id="sticky"></div>
+        <div id="fill"></div>
+      </div>
+    </div>
+  </body>
+</html>

layout/reftests/position-sticky/column-contain-1b.html

@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+<html>
+  <head>
+    <title>CSS Test: Sticky Positioning - multiframe containing-block element</title>
+    <link rel="author" title="Corey Ford" href="mailto:cford@mozilla.com">
+    <link rel="match" href="column-contain-1-ref.html">
+    <meta name="assert" content="Sticky positioning with a multiple-frame containing-block element should stay contained within the bounding box of all that element's frames">
+    <style>
+      #scroll {
+        overflow: hidden;
+        height: 200px;
+        line-height: 1;
+      }
+      #columns {
+        -moz-column-count: 2;
+        column-count: 2;
+        -moz-column-rule: 1px solid black;
+        column-rule: 1px solid black;
+        height: 200px;
+        width: 200px;
+      }
+      #fill {
+        height: 390px;
+        background: blue;
+      }
+      #sticky {
+        position: sticky;
+        left: 190px;
+        width: 10px;
+        height: 10px;
+        background: black;
+      }
+    </style>
+  </head>
+  <body>
+    <div id="scroll">
+      <div id="columns">
+        <div id="contain">
+          <div id="sticky"></div>
+          <div id="fill"></div>
+        </div>
+      </div>
+    </div>
+  </body>
+</html>

layout/reftests/position-sticky/inline-1-ref.html

@@ -0,0 +1,29 @@
+<!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:cford@mozilla.com">
+    <style>
+      #scroll {
+        overflow: hidden;
+        height: 200px;
+        position: relative;
+        line-height: 1;
+      }
+      #sticky {
+        position: absolute;
+        top: 20px;
+        left: 20px;
+      }
+      #hidden {
+        visibility: hidden;
+      }
+    </style>
+  </head>
+  <body>
+    <div id="scroll">
+      hello <div id="sticky"><span id="hidden">hello </span>there<br>everyone</div>
+    </div>
+  </body>
+</html>

layout/reftests/position-sticky/inline-1.html

@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+<html>
+  <head>
+    <title>CSS Test: Sticky Positioning - inline</title>
+    <link rel="author" title="Corey Ford" href="mailto:cford@mozilla.com">
+    <link rel="match" href="inline-1-ref.html">
+    <meta name="assert" content="Sticky positioning on inline elements should use the bounding box of all continuations">
+    <style>
+      #scroll {
+        overflow: hidden;
+        height: 200px;
+        line-height: 1;
+      }
+      #sticky {
+        position: sticky;
+        top: 20px;
+        left: 20px;
+      }
+    </style>
+  </head>
+  <body>
+    <div id="scroll">
+      hello <span id="sticky">there<br>everyone</span>
+    </div>
+  </body>
+</html>

layout/reftests/position-sticky/inline-2-ref.html

@@ -0,0 +1,29 @@
+<!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:cford@mozilla.com">
+    <style>
+      #scroll {
+        overflow: hidden;
+        height: 200px;
+        position: relative;
+        line-height: 1;
+      }
+      #sticky {
+        position: absolute;
+        top: 30px;
+        left: 30px;
+      }
+      #hidden {
+        visibility: hidden;
+      }
+    </style>
+  </head>
+  <body>
+    <div id="scroll">
+      hello <div id="sticky"><span id="hidden">hello </span>there<br>everyone</div>
+    </div>
+  </body>
+</html>

layout/reftests/position-sticky/inline-2.html

@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+<html class="reftest-wait">
+  <head>
+    <title>CSS Test: Sticky Positioning - inline, dynamic changes</title>
+    <link rel="author" title="Corey Ford" href="mailto:cford@mozilla.com">
+    <link rel="match" href="inline-2-ref.html">
+    <meta name="assert" content="Sticky positioning on inline elements should move all continuations when the offsets are changed">
+    <meta name="flags" content="dom">
+    <style>
+      #scroll {
+        overflow: hidden;
+        height: 200px;
+        line-height: 1;
+      }
+      #sticky {
+        position: sticky;
+        top: 20px;
+        left: 20px;
+      }
+    </style>
+  </head>
+  <body>
+    <div id="scroll">
+      hello <span id="sticky">there<br>everyone</span>
+    </div>
+    <script type="text/javascript">
+      document.addEventListener("MozReftestInvalidate", function() {
+        document.getElementById("sticky").style.top = "30px";
+        document.getElementById("sticky").style.left = "30px";
+        document.documentElement.removeAttribute("class");
+      }, false);
+    </script>
+  </body>
+</html>

layout/reftests/position-sticky/inline-3-ref.html

@@ -0,0 +1,35 @@
+<!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:cford@mozilla.com">
+    <style>
+      #scroll {
+        overflow: hidden;
+        height: 200px;
+        position: relative;
+        line-height: 1;
+      }
+      #sticky {
+        position: absolute;
+        top: 20px;
+        left: 10px;
+      }
+      #hidden {
+        visibility: hidden;
+      }
+      b {
+        display: inline-block;
+        width: 40px;
+        height: 10px;
+        background: black;
+      }
+    </style>
+  </head>
+  <body>
+    <div id="scroll">
+      <b></b><span id="sticky"><b id="hidden"></b><b></b><br><b></b></span>
+    </div>
+  </body>
+</html>

layout/reftests/position-sticky/inline-3.html

@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+<html>
+  <head>
+    <title>CSS Test: Sticky Positioning - inline with margins</title>
+    <link rel="author" title="Corey Ford" href="mailto:cford@mozilla.com">
+    <link rel="match" href="inline-3-ref.html">
+    <meta name="assert" content="Sticky positioning on inline elements should keep the bounding box of all continuations' margin boxes contained">
+    <style>
+      #scroll {
+        overflow: hidden;
+        height: 200px;
+        line-height: 1;
+      }
+      #sticky {
+        position: sticky;
+        top: 20px;
+        left: 20px;
+        margin-right: 10px;
+      }
+      #contain {
+        width: 90px;
+        font-size: 10px;
+      }
+      b {
+        display: inline-block;
+        width: 40px;
+        height: 10px;
+        background: black;
+      }
+    </style>
+  </head>
+  <body>
+    <div id="scroll">
+      <div id="contain">
+        <b></b><span id="sticky"><b></b><br><b></b></span>
+      </div>
+    </div>
+  </body>
+</html>

layout/reftests/position-sticky/reftest.list

@@ -41,3 +41,8 @@ fuzzy-if(Android,4,1) == containing-block-1.html containing-block-1-ref.html
 == overconstrained-1.html overconstrained-1-ref.html
 == overconstrained-2.html overconstrained-2-ref.html
 == overconstrained-3.html overconstrained-3-ref.html
+== inline-1.html inline-1-ref.html
+== inline-2.html inline-2-ref.html
+fails == inline-3.html inline-3-ref.html # bug 916302
+fails == column-contain-1a.html column-contain-1-ref.html
+== column-contain-1b.html column-contain-1-ref.html