Bug 101644 - Fixed header on Facebook news feed becomes detached from top of viewport after rubber band scrolling
Summary: Fixed header on Facebook news feed becomes detached from top of viewport afte...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac Unspecified
: P2 Normal
Assignee: Beth Dakin
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-11-08 14:12 PST by Beth Dakin
Modified: 2012-11-08 17:02 PST (History)
8 users (show)

See Also:


Attachments
Patch (12.33 KB, patch)
2012-11-08 14:30 PST, Beth Dakin
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Beth Dakin 2012-11-08 14:12:53 PST
When scrolling upwards rapidly on my Facebook news feed, the blue header at the top of the page often detaches from the top of the page after my scrolling reaches the top and rubber-bands back.

This regression appeared once we started scrolling pages with fixed positioning on the scrolling thread. 

<rdar://problem/12651944>
Comment 1 Beth Dakin 2012-11-08 14:30:05 PST
Created attachment 173118 [details]
Patch
Comment 2 Simon Fraser (smfr) 2012-11-08 16:50:18 PST
Comment on attachment 173118 [details]
Patch

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

> Source/WebCore/page/scrolling/ScrollingCoordinator.cpp:88
> +IntSize scrollOffsetForFixedPosition(const IntRect visibleContentRect, const IntSize contentsSize, const IntPoint scrollPosition, const IntPoint scrollOrigin, float frameScaleFactor, bool fixedElementsLayoutRelativeToFrame)

All the const IntFoo should be references.

> Source/WebCore/page/scrolling/ScrollingCoordinator.cpp:281
> +            LayoutRect viewportRect = IntRect(scrollOffsetForFixed.width(), scrollOffsetForFixed.height(), visibleContentRect.width(), visibleContentRect.height());

I think we have toPoint() and toSize() now.

> Source/WebCore/page/scrolling/mac/ScrollingTreeScrollingNodeMac.mm:285
> +    IntRect viewportRect = IntRect(scrollOffsetForFixedChildren.width(), scrollOffsetForFixedChildren.height(), viewportSize.width(), viewportSize.height());

Here you can just do 

viewportRect = this->viewportRect();
viewportRect.setLocation(toInt(scrollOffsetForFixedChildren)).
Comment 3 Beth Dakin 2012-11-08 17:02:08 PST
Thanks, Simon! I made the suggested changes.

http://trac.webkit.org/changeset/133974