Bug 118958 – StickyPositionConstraints should not need to change to account for a RenderLayer's scrollOffset

• Component: CSS
• Status: RESOLVED FIXED

[Beth Dakin]

Currently, to get sticky offsets right in overflow areas, the StickyPositionContraints change on every scroll to factor it in. This will be a problem once we can scroll overflow areas on the scrolling thread. The constraints should never have to change to account for the scroll position. 

<rdar://problem/12469203>

[Beth Dakin]

Created attachment 207219 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 207219 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=207219&action=review

> Source/WebCore/page/scrolling/ScrollingConstraints.h:154
> +    // m_containingBlockRect is in the scrolling ancestor's coordinate space.

I think this comment belongs next to m_containingBlockRect. You could also leave a comment here like:
// containingBlockRect() is in the scrolling ancestor's coordinate space.

> Source/WebCore/rendering/RenderBoxModelObject.cpp:537
> +        containerContentRect = containingBlock->layoutOverflowRect();

I don't understand why this is different for oveflow:scroll-sticky?

> Source/WebCore/rendering/RenderBoxModelObject.cpp:538
> +        LayoutPoint conatinerLocation = containerContentRect.location() + LayoutPoint(containingBlock->borderLeft() + containingBlock->paddingLeft(), containingBlock->borderTop() + containingBlock->paddingTop());

conatinerLocation!

> Source/WebCore/rendering/RenderBoxModelObject.cpp:557
> +        FloatPoint containerLocationRelativeToScrollingAncestor = containerRectRelativeToScrollingAncestor.location() - FloatSize(enclosingClippingBox->borderLeft() + enclosingClippingBox->paddingLeft(), enclosingClippingBox->borderTop() + enclosingClippingBox->paddingTop());

Would be nice to wrap this line.

[Beth Dakin]

Simon and I discussed this in person, and I will be uploading a new patch shortly.

[Beth Dakin]

Created attachment 207267 [details]
New patch

[Simon Fraser (smfr)]

Comment on attachment 207267 [details]
New patch

r+ but I'd like to see a testcase like the one we made today.

[Beth Dakin]

Committed http://trac.webkit.org/changeset/152998 including a test. Thanks Simon!