Bug 110673 - RenderLayer::scrollTo() should call FrameLoaderClient::didChangeScrollOffset()
Summary: RenderLayer::scrollTo() should call FrameLoaderClient::didChangeScrollOffset()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Beth Dakin
URL:
Keywords: InRadar
Depends on: 111278
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-22 18:00 PST by Beth Dakin
Modified: 2013-03-19 14:33 PDT (History)
8 users (show)

See Also:


Attachments
Patch (1.79 KB, patch)
2013-02-22 18:03 PST, Beth Dakin
sam: review+
Details | Formatted Diff | Diff
Patch (1.45 KB, patch)
2013-03-19 14:14 PDT, 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 2013-02-22 18:00:02 PST
Right now, FrameLoaderClient::didChangeScrollOffset() doesn't get called for web pages like http://instagram.com/p/UrCCIVBQX3/ because they have overflow on the body. We can easily address this by calling it at the end of RenderLayer::scrollTo().

<rdar://problem/13258596>
Comment 1 Beth Dakin 2013-02-22 18:03:46 PST
Created attachment 189884 [details]
Patch
Comment 2 Beth Dakin 2013-02-22 18:05:27 PST
Comment on attachment 189884 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        FrameLoaderClient::didChangeScrollOffset() doesn't get called for web pages like 

I have removed the "like" in my local copy.
Comment 3 Sam Weinig 2013-02-22 18:12:22 PST
Comment on attachment 189884 [details]
Patch

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

> Source/WebCore/ChangeLog:6
> +        RenderLayer::scrollTo() should call FrameLoaderClient::didChangeScrollOffset()
> +        https://bugs.webkit.org/show_bug.cgi?id=110673
> +        -and corresponding-
> +        <rdar://problem/13258596>

This is perfect.

>> Source/WebCore/ChangeLog:10
>> +        FrameLoaderClient::didChangeScrollOffset() doesn't get called for web pages like 
> 
> I have removed the "like" in my local copy.

Like totally.
Comment 4 Beth Dakin 2013-02-22 18:15:49 PST
http://trac.webkit.org/changeset/143825
Comment 5 Adam Barth 2013-03-17 10:16:00 PDT
This patch might have caused <https://code.google.com/p/chromium/issues/detail?id=193555>.  We're investigating.
Comment 6 Adam Barth 2013-03-19 00:41:34 PDT
The discussion continues on Bug 111278.  The folks there confirm that this patch caused the regression.
Comment 7 Adam Barth 2013-03-19 00:43:03 PDT
This patch also lacks a regression test.
Comment 8 Adam Barth 2013-03-19 00:59:23 PDT
I rolled out this patch in http://trac.webkit.org/changeset/146185.  Please feel free to re-land it once it doesn't cause a regression in the Chromium port.
Comment 9 Beth Dakin 2013-03-19 14:14:16 PDT
Created attachment 193917 [details]
Patch

This patch should not break Chromium.
Comment 10 Beth Dakin 2013-03-19 14:33:39 PDT
Thanks, Simon! http://trac.webkit.org/changeset/146260