RESOLVED FIXED 109316
REGRESSION: A page can be scrolled with touch when its body has style overflow-x:hidden or overflow-y:hidden 
https://bugs.webkit.org/show_bug.cgi?id=109316
Summary REGRESSION: A page can be scrolled with touch when its body has style overflo...
Terry Anderson
Reported 2013-02-08 13:01:30 PST
Created attachment 187352 [details] Example page to reproduce the bug in chromium The attached page has style="overflow-y:hidden" on its body. It cannot be scrolled with mousewheels but can be scrolled with touch in chromium.
Attachments
Example page to reproduce the bug in chromium (249 bytes, text/html)
2013-02-08 13:01 PST, Terry Anderson 		no flags
Details
WIP, not for review (3.14 KB, patch)
2013-02-10 17:22 PST, Terry Anderson 		no flags
DetailsFormatted DiffDiff
WIP, not for review (3.14 KB, patch)
2013-02-11 08:54 PST, Terry Anderson 		no flags
DetailsFormatted DiffDiff
WIP, not for review (3.16 KB, patch)
2013-02-11 15:54 PST, Terry Anderson 		no flags
DetailsFormatted DiffDiff
Page to reproduce the bug. This scrolls with touch but does not scroll with mousewheels in chromium. (348 bytes, text/html)
2013-02-12 10:52 PST, Terry Anderson 		no flags
Details
Patch (31.01 KB, patch)
2013-02-12 14:03 PST, Terry Anderson 		eric: review-
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Alexey Proskuryakov
Comment 1 2013-02-08 14:33:55 PST
This page scrolls with trackpad on Mac. Behavior is different in Safari 6.0.2 and current WebKit build though.
Terry Anderson
Comment 2 2013-02-10 17:22:46 PST
Created attachment 187497 [details] WIP, not for review
Build Bot
Comment 3 2013-02-10 17:52:33 PST
Comment on attachment 187497 [details] WIP, not for review Attachment 187497 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/16471981 New failing tests: fast/css/sticky/inline-sticky-abspos-child.html
Terry Anderson
Comment 4 2013-02-11 08:54:27 PST
Created attachment 187592 [details] WIP, not for review
Terry Anderson
Comment 5 2013-02-11 12:20:48 PST
Comment on attachment 187352 [details] Example page to reproduce the bug in chromium This page does not accurately reproduce the problem in chromium, please ignore.
Terry Anderson
Comment 6 2013-02-11 15:54:58 PST
Created attachment 187704 [details] WIP, not for review
Terry Anderson
Comment 7 2013-02-12 10:52:03 PST
Created attachment 187897 [details] Page to reproduce the bug. This scrolls with touch but does not scroll with mousewheels in chromium.
Terry Anderson
Comment 8 2013-02-12 14:03:09 PST
Created attachment 187930 [details] Patch
Eric Seidel (no email)
Comment 9 2013-02-12 15:13:33 PST
Terry tells me this bug is all about making Gesture scrolling match wheel-event scrolling. Currently <div> with overflow hidden scroll: text-scroll: YES wheel: NO gesture: YES <body> with overflow hidden scroll: text-scroll: YES wheel: NO gesture: YES A few months back, before Terry moved the gesture scroll path off of wheel events gesture and wheel were the same in those above cases. The goal of this change is to make them match again.
Eric Seidel (no email)
Comment 10 2013-02-12 15:15:18 PST
(In reply to comment #9) > Terry tells me this bug is all about making Gesture scrolling match wheel-event scrolling. > > Currently <div> with overflow hidden scroll: > text-scroll: YES > wheel: NO > gesture: YES Sorry, Terry tells me that bug 109087 fixed <div> scrolling for gesture to match wheel event, so that should be gesture: NO for <div> Also, to clarify "text scrolling" I mean selecting text and dragging outside of the scroll region.
Simon Fraser (smfr)
Comment 11 2013-02-12 15:21:00 PST
overflow:hidden should never be scrollable by the user (only programmatically).
Eric Seidel (no email)
Comment 12 2013-02-12 15:30:25 PST
Comment on attachment 187930 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=187930&action=review > Source/WebCore/rendering/RenderLayer.cpp:2047 > + if (body && body->renderer() && body->hasTagName(bodyTag)) { document->body() always hasTagName(bodyTag)
Eric Seidel (no email)
Comment 13 2013-02-12 16:42:58 PST
Comment on attachment 187930 [details] Patch OK. I spoke with Jamesr and we learned a few things. 1. Wheel events seem to scroll each direction separately, gestures may need to do the same if they aren't already. 2. This appears to be a programatic scroll path. You really want to be heading down the user-scroll paths instead. RenderBox::scroll/RenderLayer::scroll/ScrollableArea::scroll are the ones you want. 3. In the wheel-event case, scrolls are prevented for overflow: none by ScrollableArea::scroll detecting that it doesn't have a scrollbar in the direction you're asking it to scroll and bailing. Gestures should likely go down that same path.
Terry Anderson
Comment 14 2013-02-13 16:38:55 PST
After discussing the matter with Eric, it is clear that we should not even be using RenderLayer::scrollBy() to perform gesture scrolling. Switching code paths will fix this bug, among others. I have filed https://bugs.webkit.org/show_bug.cgi?id=109769 to track.
Terry Anderson
Comment 15 2013-03-01 17:14:56 PST
This issue is resolved by http://trac.webkit.org/changeset/144519.
Note You need to log in before you can comment on or make changes to this bug.