Fix sticky constraints and update sticky layer positions recursively after scroll.

third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2005 Allan Sandfeld Jensen (kde@carewolf.com)
  *           (C) 2005, 2006 Samuel Weinig (sam.weinig@gmail.com)
  * Copyright (C) 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ skipping 617 matching lines @@
         && (!containingBlock->hasAutoHeightOrContainingBlockWithAutoHeight()
             || !style()->bottom().hasPercent()
             || containingBlock->stretchesToViewport()))
         offset.expand(LayoutUnit(), -valueForLength(style()->bottom(), containingBlock->availableHeight()));
 
     return offset;
 }
 
 void LayoutBoxModelObject::updateStickyPositionConstraints() const
 {
-    // TODO(flackr): This method is reasonably complicated and should have some direct unit testing.
     const FloatSize constrainingSize = computeStickyConstrainingRect().size();
 
     PaintLayerScrollableArea* scrollableArea = layer()->ancestorOverflowLayer()->getScrollableArea();
     StickyPositionScrollingConstraints constraints;
     FloatSize skippedContainersOffset;
     LayoutBlock* containingBlock = this->containingBlock();
     // Skip anonymous containing blocks.
     while (containingBlock->isAnonymous()) {
         skippedContainersOffset += toFloatSize(FloatPoint(containingBlock->frameRect().location()));
         containingBlock = containingBlock->containingBlock();
     }
     LayoutBox* scrollAncestor = layer()->ancestorOverflowLayer()->isRootLayer() ? nullptr : toLayoutBox(layer()->ancestorOverflowLayer()->layoutObject());
 
-    LayoutRect containerContentRect = containingBlock->contentBoxRect();
+    LayoutRect containerContentRect = containingBlock->layoutOverflowRect();

[flackr, 2016/06/09 17:13:32]

When the container is the scroll ancestor, we need to use layoutOverflowRect in
order to get the full scrollable rect rather than the clipped rect.

     LayoutUnit maxWidth = containingBlock->availableLogicalWidth();
+    containerContentRect.contractEdges(

[chrishtr, 2016/06/06 22:18:39]

Contract for padding and then margin? Is this trying to subtract off both? If
so, why aren't they
added?

[flackr, 2016/06/09 17:13:33]

Done, note that we remove the padding from the containing block and the margin
from the sticky element. I suspect for the container's padding I need to resolve
the lengths against the containing block's containing block's available logical
width. I've added a test to verify resolving percentage values.

+        minimumValueForLength(containingBlock->style()->paddingTop(), maxWidth),
+        minimumValueForLength(containingBlock->style()->paddingRight(), maxWidth),
+        minimumValueForLength(containingBlock->style()->paddingBottom(), maxWidth),
+        minimumValueForLength(containingBlock->style()->paddingLeft(), maxWidth));
 
     // Sticky positioned element ignore any override logical width on the containing block (as they don't call

[chrishtr, 2016/06/06 22:18:39]

Does this comment belong above line 651?

[flackr, 2016/06/09 17:13:33]

Done.

     // containingBlockLogicalWidthForContent). It's unclear whether this is totally fine.
     // Compute the container-relative area within which the sticky element is allowed to move.
     containerContentRect.contractEdges(
         minimumValueForLength(style()->marginTop(), maxWidth),
         minimumValueForLength(style()->marginRight(), maxWidth),
         minimumValueForLength(style()->marginBottom(), maxWidth),
         minimumValueForLength(style()->marginLeft(), maxWidth));
 
     // Map to the scroll ancestor.
-    constraints.setScrollContainerRelativeContainingBlockRect(containingBlock->localToAncestorQuad(FloatRect(containerContentRect), scrollAncestor).boundingBox());
+    FloatRect scrollContainerRelativeContainingBlockRect(containingBlock->localToAncestorQuad(FloatRect(containerContentRect), scrollAncestor).boundingBox());
+    FloatSize scrollOffset(scrollAncestor ? toFloatSize(scrollAncestor->getScrollableArea()->adjustedScrollOffset()) : FloatSize());

[chrishtr, 2016/06/06 22:18:39]

Why should this rect include scroll offset?

[flackr, 2016/06/09 17:13:33]

Because we want the container's rect with respect to the scrollable area of the
scroll ancestor.

[chrishtr, 2016/06/13 19:46:39]

I see, ok.

Why doesn't localToAncestorQuad handle this then? I would have expected this
line of code:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/La...
(call to offsetFromContainer())

to include scroll offset when passing to the scrollAncestor from the object for
which it is a containing block.

[flackr, 2016/06/15 13:45:49]

offsetFromContainer is specifically removing the scroll offset to return the
offset between the two boxes visually given the current scroll.

+
+    // Include scroll offset in container position if the container is not our scroll ancestor.
+    if (containingBlock != scrollAncestor)

[chrishtr, 2016/06/06 22:18:39]

Why conditional? What if the containing block is above the scroll ancestor?

[flackr, 2016/06/09 17:13:32]

We want to compute the bounds relative to the scroll ancestor. If the containing
block is the scroll ancestor it starts at (0, 0) regardless of the current
scroll position, however if it's a descendant the scroll position needs to be
added to get it's position within the scroll ancestor.
This should be impossible, position sticky's containing block is the first block
ancestor (which a scroll ancestor must be).

[chrishtr, 2016/06/13 19:46:39]

Ah. Yes you are right.

+        scrollContainerRelativeContainingBlockRect.move(scrollOffset);
+    constraints.setScrollContainerRelativeContainingBlockRect(scrollContainerRelativeContainingBlockRect);
 
     FloatRect stickyBoxRect = isLayoutInline()
         ? FloatRect(toLayoutInline(this)->linesBoundingBox())
         : FloatRect(toLayoutBox(this)->frameRect());
     FloatRect flippedStickyBoxRect = stickyBoxRect;
     containingBlock->flipForWritingMode(flippedStickyBoxRect);
     FloatPoint stickyLocation = flippedStickyBoxRect.location() + skippedContainersOffset;
 
     // TODO(flackr): Unfortunate to call localToAncestorQuad again, but we can't just offset from the previously computed rect if there are transforms.
     // Map to the scroll ancestor.
     FloatRect scrollContainerRelativeContainerFrame = containingBlock->localToAncestorQuad(FloatRect(FloatPoint(), FloatSize(containingBlock->size())), scrollAncestor).boundingBox();
+    scrollContainerRelativeContainerFrame.move(scrollOffset);

[chrishtr, 2016/06/06 22:18:39]

localToAncestorQuad should already be adding in the scroll offset. Not sure what
you are trying to do here.

[flackr, 2016/06/09 17:13:32]

It does not seem to. If the container is at (0, 0) and you're scrolled down
200px this reports the element as being at (0, -200). We want all computed rects
with respect to the top of the scrollable area.

[chrishtr, 2016/06/13 19:46:39]

See my comment above. Curious why it is not working as I expected..

[chrishtr, 2016/06/15 16:13:46]

Any response to this comment?

[flackr, 2016/06/16 07:22:51]

Ah, sorry, I commented on the other location. offsetFromContainer is
specifically removing the scroll offset presumably to return the offset between
the two boxes visually given the current scroll.

The specific line is here:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/layout/La...

 
     // If the containing block is our scroll ancestor, its location will not include the scroll offset which we need to include as
     // part of the sticky box rect so we include it here.
-    if (containingBlock->hasOverflowClip()) {
-        FloatSize scrollOffset(toFloatSize(containingBlock->layer()->getScrollableArea()->adjustedScrollOffset()));
+    if (containingBlock == scrollAncestor)
         stickyLocation -= scrollOffset;
-    }
 
     constraints.setScrollContainerRelativeStickyBoxRect(FloatRect(scrollContainerRelativeContainerFrame.location() + toFloatSize(stickyLocation), flippedStickyBoxRect.size()));
 
     // We skip the right or top sticky offset if there is not enough space to honor both the left/right or top/bottom offsets.
     LayoutUnit horizontalOffsets = minimumValueForLength(style()->right(), LayoutUnit(constrainingSize.width())) +
         minimumValueForLength(style()->left(), LayoutUnit(constrainingSize.width()));
     bool skipRight = false;
     bool skipLeft = false;
     if (!style()->left().isAuto() && !style()->right().isAuto()) {
         if (horizontalOffsets > containerContentRect.width()
@@ skipping 37 matching lines @@
     scrollableArea->stickyConstraintsMap().set(layer(), constraints);
 }
 
 FloatRect LayoutBoxModelObject::computeStickyConstrainingRect() const
 {
     if (layer()->ancestorOverflowLayer()->isRootLayer())
         return view()->frameView()->visibleContentRect();
 
     LayoutBox* enclosingClippingBox = toLayoutBox(layer()->ancestorOverflowLayer()->layoutObject());
     FloatRect constrainingRect;
-    constrainingRect = FloatRect(enclosingClippingBox->overflowClipRect(LayoutPoint()));
+    constrainingRect = FloatRect(enclosingClippingBox->overflowClipRect(LayoutPoint(DoublePoint(enclosingClippingBox->getScrollableArea()->adjustedScrollOffset()))));
     constrainingRect.move(enclosingClippingBox->paddingLeft(), enclosingClippingBox->paddingTop());
     constrainingRect.contract(FloatSize(enclosingClippingBox->paddingLeft() + enclosingClippingBox->paddingRight(),
         enclosingClippingBox->paddingTop() + enclosingClippingBox->paddingBottom()));
     return constrainingRect;
 }
 
 LayoutSize LayoutBoxModelObject::stickyPositionOffset() const
 {
     const PaintLayer* ancestorOverflowLayer = layer()->ancestorOverflowLayer();
     // TODO: Force compositing input update if we ask for offset before compositing inputs have been computed?
@@ skipping 377 matching lines @@
     if (rootElementStyle->hasBackground())
         return false;
 
     if (node() != document().firstBodyElement())
         return false;
 
     return true;
 }
 
 } // namespace blink