Bug 191322 - Momentum scrolling ends at the wrong place when a scrolling overflow element has a non-zero border
Summary: Momentum scrolling ends at the wrong place when a scrolling overflow element ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: Safari Technology Preview
Hardware: Macintosh macOS 10.12
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-11-06 12:36 PST by jonjohnjohnson
Modified: 2018-11-27 14:39 PST (History)
6 users (show)

See Also:


Attachments
Simple testcase (612 bytes, text/html)
2018-11-20 09:59 PST, Simon Fraser (smfr)
no flags Details
Patch (7.12 KB, patch)
2018-11-20 14:17 PST, Simon Fraser (smfr)
dino: review+
ews: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews124 for ios-simulator-wk2 (2.64 MB, application/zip)
2018-11-20 16:54 PST, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description jonjohnjohnson 2018-11-06 12:36:36 PST
Firstly, I'm sorry that this isn't as reduced of a test case as I, or probably you guys, would like, but I've struggled to isolate it apart from using negative margins of large/viewport units in conjunction with the sticky positioning scheme...

cc wenson_hsieh@apple.com

1. Go to test case -> https://jsbin.com/votodew/edit?css,output in large enough screen see it's background as white.
2. Scroll down through the red bordered "stacking/sticking" dialogue, reaching the very bottom.
3. Repeat the action of scrolling away from the bottom edge of the scrollHeight.
4. Notice if you sufficiently throw a scrolling gesture to reach scrolling end, but not with excessive force, more often than not, the final resting scroll position, will leave the scrollTop just short of reaching the end of the scroll container.
*Note, the easy visual that notes it not scrolling to the end is that the very last line of the "dialogue" is not aligned with the rest of the text, resting extra pixels below, depending on the viewport height.
5. When it "falls short" you can start another scrolling downward gesture that will reach it's end. But quite often it's miscalculating.

Problem
It seems as though mid scroll action, a sufficient gesture for reaching the calculated "scroll end" is shorter than the elements scrollHeight.

Expectation
Scrolling that "feels" as though it should reach the end of the scrolling container should not fall *just short of reaching the end.
Comment 1 jonjohnjohnson 2018-11-18 05:23:11 PST
I put more elbow grease into isolating the bug to being from a scroller having a border-width set to any length in the same axis of scrolling.

Here is an replacement (and more concise) test case:
https://jsfiddle.net/9v514o37/

Four scrollers provided, that scroll in the dimension to see lined pattern move through view. Whether horizontal or vertical, reaching the "scrollend" position is chunky and requires extra effort on macOS STP when scrolling in the dimension that also has a border-width value. See the major difference in effort when scrolling blocks that do not have border in that dimension.

Sorry for not isolating as well or even correctly with the first test case. Completely ignore that in light of this comment.
Comment 2 Simon Fraser (smfr) 2018-11-20 09:59:04 PST
Created attachment 355350 [details]
Simple testcase
Comment 3 Simon Fraser (smfr) 2018-11-20 10:23:28 PST
Heh, RenderLayer::visibleContentRectInternal() has:

    // FIXME: This seems wrong: m_layerSize includes borders. Can we just use the ScrollableArea implementation?
Comment 4 Simon Fraser (smfr) 2018-11-20 14:17:27 PST
Created attachment 355364 [details]
Patch
Comment 5 Build Bot 2018-11-20 16:54:18 PST
Comment on attachment 355364 [details]
Patch

Attachment 355364 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10092782

New failing tests:
media/no-fullscreen-when-hidden.html
accessibility/ios-simulator/scroll-in-overflow-div.html
Comment 6 Build Bot 2018-11-20 16:54:19 PST
Created attachment 355370 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 7 Dean Jackson 2018-11-23 09:50:51 PST
Comment on attachment 355364 [details]
Patch

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

> LayoutTests/fast/scrolling/momentum-scroll-with-borders.html:46
> +            let sendMomenumScroll = function() {

momenum!!!
Comment 8 Radar WebKit Bug Importer 2018-11-27 11:48:38 PST
<rdar://problem/46283201>
Comment 9 Simon Fraser (smfr) 2018-11-27 14:03:37 PST
https://trac.webkit.org/r238576
Comment 10 jonjohnjohnson 2018-11-27 14:39:35 PST
\( ゚ヮ゚)/

Thanks for being so speedy simon.fraser@apple.com & dino@apple.com