Bug 916302 - Part 2: In sticky positioning calculations, use the union of the element's margin boxes and the union of the containing-block element's content boxes. r=dholbert
authorCorey Ford <cford@mozilla.com>
Tue, 03 Dec 2013 11:28:32 -0800
changeset 174470 920b595433d84855d785b0e14dc873de4be164ff
parent 174469 9ceffd9514da3f9b5dd8d2a41f9980dccbc3ee09
child 174471 2e7fee91e62ac2b092af04e65a3a6ccc8fb59877
push id445
push userffxbld
push dateMon, 10 Mar 2014 22:05:19 +0000
treeherdermozilla-release@dc38b741b04e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs916302
milestone28.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 916302 - Part 2: In sticky positioning calculations, use the union of the element's margin boxes and the union of the containing-block element's content boxes. r=dholbert
layout/generic/StickyScrollContainer.cpp
layout/reftests/position-sticky/column-contain-2-ref.html
layout/reftests/position-sticky/column-contain-2.html
layout/reftests/position-sticky/inline-3-ref.html
layout/reftests/position-sticky/inline-3.html
layout/reftests/position-sticky/reftest.list
--- a/layout/generic/StickyScrollContainer.cpp
+++ b/layout/generic/StickyScrollContainer.cpp
@@ -187,26 +187,35 @@ 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());
 
-  // Containing block limits
+  // 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, cbFrame);
-    aContain->MoveBy(-aFrame->GetParent()->GetOffsetTo(cbFrame));
-    // FIXME (Bug 920688): GetUsedBorderAndPadding / GetUsedMargin
-    // consider skip-sides, which doesn't quite mesh with the use of
-    // GetAllInFlowRectsUnion here.  This probably needs to do that
-    // computation *inside* the accumuation function over the in-flows.
-    aContain->Deflate(cbFrame->GetUsedBorderAndPadding());
-    aContain->Deflate(aFrame->GetUsedMargin());
+    *aContain = nsLayoutUtils::
+      GetAllInFlowRectsUnion(cbFrame, aFrame->GetParent(),
+                             nsLayoutUtils::RECTS_USE_CONTENT_BOX);
+    nsRect marginRect = nsLayoutUtils::
+      GetAllInFlowRectsUnion(aFrame, aFrame->GetParent(),
+                             nsLayoutUtils::RECTS_USE_MARGIN_BOX);
+
+    // Deflate aContain by the difference between the union of aFrame's
+    // continuations' margin boxes and the union of their border boxes, so that
+    // by keeping aFrame within aContain, we keep the union of the margin boxes
+    // within the containing block's content box.
+    aContain->Deflate(marginRect - rect);
+
+    // Deflate aContain by the border-box size, to form a constraint on the
+    // upper-left corner of aFrame and continuations.
     aContain->Deflate(nsMargin(0, rect.width, rect.height, 0));
   }
 
   nsMargin sfPadding = scrolledFrame->GetUsedPadding();
   nsPoint sfOffset = aFrame->GetParent()->GetOffsetTo(scrolledFrame);
 
   // Top
   if (computedOffsets->top != NS_AUTOOFFSET) {
@@ -241,17 +250,19 @@ StickyScrollContainer::ComputeStickyLimi
        direction == NS_STYLE_DIRECTION_RTL ||
        rect.width <= sfSize.width - computedOffsets->LeftRight())) {
     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 frameOffset = aFrame->GetPosition() - rect.TopLeft();
+  aStick->MoveBy(frameOffset);
+  aContain->MoveBy(frameOffset);
 }
 
 nsPoint
 StickyScrollContainer::ComputePosition(nsIFrame* aFrame) const
 {
   nsRect stick;
   nsRect contain;
   ComputeStickyLimits(aFrame, &stick, &contain);
new file mode 100644
--- /dev/null
+++ b/layout/reftests/position-sticky/column-contain-2-ref.html
@@ -0,0 +1,48 @@
+<!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">
+    <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;
+      }
+      #contain {
+        padding-bottom: 10px;
+        border-bottom: 10px solid gray;
+      }
+      #fill {
+        height: 370px;
+        background: blue;
+      }
+      #sticky {
+        position: relative;
+        top: 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>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/position-sticky/column-contain-2.html
@@ -0,0 +1,51 @@
+<!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:corey@coreyford.name">
+    <link rel="match" href="column-contain-2-ref.html">
+    <meta name="assert" content="Sticky positioning where the containing-block element has multiple frames and border/padding should stay contained within the union of the content boxes of all 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;
+      }
+      #contain {
+        padding-bottom: 10px;
+        border-bottom: 10px solid gray;
+      }
+      #fill {
+        height: 370px;
+        background: blue;
+      }
+      #sticky {
+        position: sticky;
+        top: 195px;
+        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>
--- a/layout/reftests/position-sticky/inline-3-ref.html
+++ b/layout/reftests/position-sticky/inline-3-ref.html
@@ -1,36 +1,30 @@
 <!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">
+    <link rel="author" title="Corey Ford" href="mailto:corey@coreyford.name">
     <link rel="stylesheet" type="text/css" href="ahem.css">
     <style>
       #scroll {
         overflow: hidden;
         height: 200px;
         position: relative;
         font: 10px/1 Ahem;
       }
       #sticky {
         position: absolute;
-        top: 20px;
+        top: 0;
         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>
+      test <span id="sticky"><span id="hidden">test </span>test<br>test</span>
     </div>
   </body>
 </html>
--- a/layout/reftests/position-sticky/inline-3.html
+++ b/layout/reftests/position-sticky/inline-3.html
@@ -1,42 +1,34 @@
 <!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="author" title="Corey Ford" href="mailto:corey@coreyford.name">
     <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">
     <link rel="stylesheet" type="text/css" href="ahem.css">
     <style>
       #scroll {
         overflow: hidden;
         height: 200px;
         font: 10px/1 Ahem;
       }
       #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;
+        width: 100px;
       }
     </style>
   </head>
   <body>
     <div id="scroll">
       <div id="contain">
-        <b></b><span id="sticky"><b></b><br><b></b></span>
+        test <span id="sticky">test<br>test</span>
       </div>
     </div>
   </body>
 </html>
--- a/layout/reftests/position-sticky/reftest.list
+++ b/layout/reftests/position-sticky/reftest.list
@@ -38,15 +38,16 @@ fuzzy-if(Android,2,3) == stacking-contex
 == left-right-2.html left-right-2-ref.html
 == left-right-3.html left-right-3-ref.html
 fuzzy-if(Android,4,810) == 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
+fuzzy-if(OSX==10.6||OSX==10.7,64,100) fuzzy-if(OSX==10.8,99,210) == inline-3.html inline-3-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(Android,8,1533) == block-in-inline-2.html block-in-inline-2-ref.html
 fuzzy-if(Android,8,630) fuzzy-if(OSX==10.8,1,11) == block-in-inline-3.html block-in-inline-3-ref.html
 == inner-table-1.html inner-table-1-ref.html