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 tree / graph
 
Reported: 2013-02-22 18:00 PST by Beth Dakin
Modified: 2023-07-16 03:45 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
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

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