Bug 135637 - [Mac] Unable to scroll to bottom of nested scrollable areas
Summary: [Mac] Unable to scroll to bottom of nested scrollable areas
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac All
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-08-05 20:06 PDT by Brent Fulgham
Modified: 2014-08-06 11:26 PDT (History)
10 users (show)

See Also:


Attachments
Patch (1.72 KB, patch)
2014-08-05 20:12 PDT, Brent Fulgham
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2 (1.20 MB, application/zip)
2014-08-05 21:00 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion (605.22 KB, application/zip)
2014-08-05 21:34 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion (605.01 KB, application/zip)
2014-08-05 22:32 PDT, Build Bot
no flags Details
Patch (15.84 KB, patch)
2014-08-06 10:29 PDT, Brent Fulgham
zalan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2014-08-05 20:06:31 PDT
A user reported that "On some pages, I can’t scroll to the bottom of the page using the trackpad, but I can get there with the down arrow key."

This seems to be an issue with sub-pixel calculations when a scrollable <div> is nested inside another scrollable <div>.
Comment 1 Brent Fulgham 2014-08-05 20:07:00 PDT
<rdar://problem/17910241>
Comment 2 Brent Fulgham 2014-08-05 20:12:52 PDT
Created attachment 236077 [details]
Patch
Comment 3 zalan 2014-08-05 20:26:24 PDT
Comment on attachment 236077 [details]
Patch

I don't know much about scrolling, but wouldn't it make sense to fix those totalSize()/visibleSize() values too? It seems like we can't use them to calculate the scroll size value anymore and that could potentially cause other issues throughout the code -wherever they are used (if they are used at all)
Comment 4 Simon Fraser (smfr) 2014-08-05 20:42:58 PDT
Comment on attachment 236077 [details]
Patch

I think you should be able to make a layout test for this.
Comment 5 Darin Adler 2014-08-05 20:49:01 PDT
Comment on attachment 236077 [details]
Patch

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

> Source/WebCore/rendering/RenderLayer.cpp:2603
> +    IntSize scrollRange = maximumScrollPosition() - minimumScrollPosition();
> +    if (orientation == HorizontalScrollbar)
> +        return scrollRange.width();
> +
> +    return scrollRange.height();

The blank line makes this read awkwardly. I think ? : would be nicer.
Comment 6 Darin Adler 2014-08-05 20:49:19 PDT
Comment on attachment 236077 [details]
Patch

Why no regression test?
Comment 7 Build Bot 2014-08-05 21:00:36 PDT
Comment on attachment 236077 [details]
Patch

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

New failing tests:
ietestcenter/css3/bordersbackgrounds/background-attachment-local-scrolling.htm
Comment 8 Build Bot 2014-08-05 21:00:40 PDT
Created attachment 236079 [details]
Archive of layout-test-results from webkit-ews-13 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-13  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 9 Build Bot 2014-08-05 21:34:14 PDT
Comment on attachment 236077 [details]
Patch

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

New failing tests:
ietestcenter/css3/bordersbackgrounds/background-attachment-local-scrolling.htm
Comment 10 Build Bot 2014-08-05 21:34:18 PDT
Created attachment 236080 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 11 Build Bot 2014-08-05 22:32:53 PDT
Comment on attachment 236077 [details]
Patch

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

New failing tests:
ietestcenter/css3/bordersbackgrounds/background-attachment-local-scrolling.htm
Comment 12 Build Bot 2014-08-05 22:32:57 PDT
Created attachment 236081 [details]
Archive of layout-test-results from webkit-ews-04 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-04  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 13 Brent Fulgham 2014-08-05 22:34:38 PDT
(In reply to comment #6)
> (From update of attachment 236077 [details])
> Why no regression test?

Yes - I'm working on that now!
Comment 14 Brent Fulgham 2014-08-06 10:29:49 PDT
Created attachment 236108 [details]
Patch
Comment 15 Brent Fulgham 2014-08-06 11:26:22 PDT
Committed r172160: <http://trac.webkit.org/changeset/172160>