Bug 106946

Summary: Sticky-position elements can jump around/hide on rubber-banding
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: Layout and RenderingAssignee: Beth Dakin <bdakin>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, bdakin, benjamin, eric, jer.noble, ojan.autocc, simon.fraser, webkit-bug-importer, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 104276    
Attachments:
Description Flags
Patch
simon.fraser: review-
Patch simon.fraser: review+

Description Simon Fraser (smfr) 2013-01-15 14:47:12 PST
Sticky-postion elements behave oddly on rubber-banding. For example, the bug review bar on https://bugs.webkit.org/attachment.cgi?id=179870&action=review jumps around and disappear when you rubber-band at the top of the document.
Comment 1 Radar WebKit Bug Importer 2013-01-15 14:47:35 PST
<rdar://problem/13019346>
Comment 2 Beth Dakin 2013-01-18 17:27:20 PST
I think I know how to fix this.
Comment 3 Beth Dakin 2013-01-18 17:56:03 PST
Created attachment 183585 [details]
Patch
Comment 4 Simon Fraser (smfr) 2013-01-18 18:03:08 PST
Comment on attachment 183585 [details]
Patch

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

> Source/WebCore/rendering/RenderBoxModelObject.cpp:513
>      LayoutRect viewportRect = view()->frameView()->visibleContentRect();
> +    viewportRect.setLocation(toPoint(view()->frameView()->scrollOffsetForFixedPosition()));

We do this in so many places that I think it's time to add something to FrameView for this. Maybe viewportConstrainedObjectRect() ?
Comment 5 Beth Dakin 2013-01-18 18:17:13 PST
Created attachment 183588 [details]
Patch
Comment 6 Simon Fraser (smfr) 2013-01-18 18:18:57 PST
Comment on attachment 183588 [details]
Patch

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

> Source/WebCore/page/FrameView.h:194
> +    LayoutRect viewportConstrainedVisibleContentRect() const;

I think this deserves a comment saying how it's different from visibleContentRect() (rubber-banding and zooming).
Comment 7 Beth Dakin 2013-01-18 18:24:23 PST
Thank you! I added a comment http://trac.webkit.org/changeset/140229
Comment 8 Benjamin Poulain 2013-01-18 19:33:05 PST
It looks like this caused a regression on fast/css/sticky/sticky-top-zoomed.html:
    http://build.webkit.org/results/Apple%20MountainLion%20Release%20WK2%20(Tests)/r140230%20(4996)/results.html
Comment 9 Simon Fraser (smfr) 2013-01-18 20:58:40 PST
I think that change is expected, since this commit made sticky take zooming into account. We'll have to fix the ref test.
Comment 10 Jer Noble 2013-01-18 21:51:23 PST
Filed https://bugs.webkit.org/show_bug.cgi?id=107356, tracking the test failure.  Test was added to TestExpectations in <http://trac.webkit.org/changeset/140234>.