Bug 117369 - Scrolling with platformWidget and delegateScrolling is incorrectly clamped
Summary: Scrolling with platformWidget and delegateScrolling is incorrectly clamped
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Benjamin Poulain
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-06-07 22:09 PDT by Benjamin Poulain
Modified: 2013-06-09 12:53 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.21 KB, patch)
2013-06-07 22:15 PDT, Benjamin Poulain
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (562.12 KB, application/zip)
2013-06-07 23:03 PDT, Build Bot
no flags Details
Patch (2.78 KB, patch)
2013-06-08 17:33 PDT, Benjamin Poulain
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Poulain 2013-06-07 22:09:16 PDT
Scrolling with platformWidget and delegateScrolling is incorrectly clamped
Comment 1 Benjamin Poulain 2013-06-07 22:15:33 PDT
Created attachment 204085 [details]
Patch
Comment 2 Tim Horton 2013-06-07 22:23:21 PDT
Comment on attachment 204085 [details]
Patch

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

> Source/WebCore/ChangeLog:14
> +        we do not let those external scrolling handle the out of bound scrolling.

external scrolling… what?
Comment 3 Benjamin Poulain 2013-06-07 22:26:50 PDT
<rdar://problem/13985064>
Comment 4 Benjamin Poulain 2013-06-07 22:27:14 PDT
Check this up ^^, that will explain a bit :)
Comment 5 Build Bot 2013-06-07 23:03:23 PDT
Comment on attachment 204085 [details]
Patch

Attachment 204085 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/725418

New failing tests:
fast/dom/rtl-scroll-to-leftmost-and-resize.html
fast/block/positioning/vertical-rl/fixed-positioning.html
fast/dom/horizontal-scrollbar-when-dir-change.html
fast/dom/vertical-scrollbar-when-dir-change.html
fast/dom/scroll-reveal-top-overflow.html
fast/dom/scroll-reveal-left-overflow.html
Comment 6 Build Bot 2013-06-07 23:03:26 PDT
Created attachment 204086 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.3
Comment 7 Benjamin Poulain 2013-06-08 17:33:36 PDT
Created attachment 204103 [details]
Patch
Comment 8 Benjamin Poulain 2013-06-08 17:37:09 PDT
requestScrollPositionUpdate() is virtual. This makes more sense.
Comment 9 Darin Adler 2013-06-08 18:44:51 PDT
Comment on attachment 204103 [details]
Patch

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

> Source/WebCore/platform/ScrollView.cpp:429
> +    if (requestScrollPositionUpdate(newScrollPosition))
> +        return;

Any downside to calling this for non-frame views?
Comment 10 Benjamin Poulain 2013-06-08 18:57:00 PDT
(In reply to comment #9)
> (From update of attachment 204103 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=204103&action=review
> 
> > Source/WebCore/platform/ScrollView.cpp:429
> > +    if (requestScrollPositionUpdate(newScrollPosition))
> > +        return;
> 
> Any downside to calling this for non-frame views?

3 jumps.
The base implementation of requestScrollPositionUpdate() just return false.

Thanks for the review.
Comment 11 Benjamin Poulain 2013-06-09 12:53:01 PDT
Committed r151356: <http://trac.webkit.org/changeset/151356>