This bug addresses a couple issues: 1. Two finger panning in one direction can sometimes cause some unexpected scrolling in the other direction when the gesture first begins(directions meaning horizontal and vertical) 2. Single finger horizontal panning should only be disabled when attempting to select text 3. Scrolling via panning should be clamped, other wise we can scroll contents completely out of the scrollview. 4. Horizontal Overpan should work 5. Overpan should work only when the main document is scrolled
Each of these fixes by itself is rather small. I'm not sure if I should just break this up into 5 smaller bugs.
<rdar://problem/12712495>
Created attachment 212245 [details] patch
Comment on attachment 212245 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=212245&action=review Looks great! We might want to add a FIXME for Right-to-left languages if that's something we are likely to encounter for scroll bar support. > Source/WebKit/win/WebView.cpp:1611 > + || (view->horizontalScrollbar() && (gestureBeginPoint.y > (webViewRect.bottom - view->horizontalScrollbar()->theme()->scrollbarThickness()))); This might not be right for a right-to-left environment. > Source/WebKit/win/WebView.cpp:1640 > + // Disable single-fingered horizontal panning only if the target node text. ... target node *is* text? > Source/WebKit/win/WebView.cpp:1741 > + UpdatePanningFeedbackPtr()(m_viewWindow, xpan, ypan, gi.dwFlags & GF_INERTIA); Nice!
(In reply to comment #4) > (From update of attachment 212245 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=212245&action=review > > Looks great! We might want to add a FIXME for Right-to-left languages if that's something we are likely to encounter for scroll bar support. > > > Source/WebKit/win/WebView.cpp:1611 > > + || (view->horizontalScrollbar() && (gestureBeginPoint.y > (webViewRect.bottom - view->horizontalScrollbar()->theme()->scrollbarThickness()))); > > This might not be right for a right-to-left environment. > I'll run a quick test.
Committed: http://trac.webkit.org/changeset/156219 Setting dir=rtl on the document doesn't flip the position of the vertical scrollbar. Perhaps that's actually controlled by the HWND? If that's the case, GetWindowRect might actually take the flipped-ness into account. Will investigate further later.
(In reply to comment #6) > Committed: http://trac.webkit.org/changeset/156219 > > Setting dir=rtl on the document doesn't flip the position of the vertical scrollbar. > > Perhaps that's actually controlled by the HWND? If that's the case, GetWindowRect might actually take the flipped-ness into account. > > Will investigate further later. I revoke that statement. It definitely won't work. Even if GetWindowRect flips the left and right values appropriately I'll need to add the scrollbar width, not subtract.
This patch caused 5 tests to hit assertions on Mac: http://build.webkit.org/builders/Apple%20MountainLion%20Release%20WK1%20%28Tests%29/builds/13132
Created attachment 212253 [details] follow-up fix > This patch caused 5 tests to hit assertions on Mac: > http://build.webkit.org/builders/Apple%20MountainLion%20Release%20WK1%20%28Tests%29/builds/13132 That warrants either an immediate rollout or an immediate fix. It's not OK to keep crashes in the tree over weekend.
EWS didn't catch this because the patch in the bug is not quite what got landed.
Landed the follow-up fix in r156223.
Comment on attachment 212253 [details] follow-up fix View in context: https://bugs.webkit.org/attachment.cgi?id=212253&action=review > Source/WebCore/rendering/RenderLayer.cpp:-2181 > - *scrolledView = &renderer().view().frameView(); I should have caught this when I reviewed. :(