Bug 81144

Summary: [Qt][WK2] Fix bounce-back behaviour for panning
Product: WebKit Reporter: Andras Becsi <abecsi>
Component: New BugsAssignee: Andras Becsi <abecsi>
Status: RESOLVED FIXED    
Severity: Normal CC: dinu.jacob, gustavo, kenneth, menard, pnormand, webkit.review.bot, xan.lopez, zoltan
Priority: P3 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: All   
OS: Linux   
Attachments:
Description Flags
proposed patch
none
proposed patch none

Andras Becsi
Reported 2012-03-14 12:39:18 PDT
The pan gesture should be cancelled and the content should be moved to the end position immediately when tapping during the bounce-back animation.
Attachments
proposed patch (2.77 KB, patch)
2012-03-14 13:03 PDT, Andras Becsi
no flags
proposed patch (4.38 KB, patch)
2012-03-16 07:53 PDT, Andras Becsi
no flags
Andras Becsi
Comment 1 2012-03-14 13:03:27 PDT
Created attachment 131904 [details] proposed patch
Kenneth Rohde Christiansen
Comment 2 2012-03-15 02:48:31 PDT
Comment on attachment 131904 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=131904&action=review > Source/WebKit2/UIProcess/qt/QtPanGestureRecognizer.cpp:70 > interactionEngine()->panGestureCancelled(); > + interactionEngine()->ensureContentWithinViewportBoundary(/*immediate*/ true); > return false; hmm, yes you move it back immediately, but isn't the other animation still running?
Andras Becsi
Comment 3 2012-03-15 06:59:42 PDT
(In reply to comment #2) > (From update of attachment 131904 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=131904&action=review > > > Source/WebKit2/UIProcess/qt/QtPanGestureRecognizer.cpp:70 > > interactionEngine()->panGestureCancelled(); > > + interactionEngine()->ensureContentWithinViewportBoundary(/*immediate*/ true); > > return false; > > hmm, yes you move it back immediately, but isn't the other animation still running? No, it is stopped by the interactionEngine()->panGestureCancelled() call. This results in a cancelFlick() call to the Flickable, which just stops the animation where it was without returning into valid boundaries. Since Flickable has no public API for immediate position correction, we need to use our own implementation here.
Andras Becsi
Comment 4 2012-03-15 08:59:01 PDT
Comment on attachment 131904 [details] proposed patch Clearing flags on attachment: 131904 Committed r110850: <http://trac.webkit.org/changeset/110850>
Andras Becsi
Comment 5 2012-03-15 08:59:12 PDT
All reviewed patches have been landed. Closing bug.
Andras Becsi
Comment 6 2012-03-16 07:41:26 PDT
This causes an assert in the interaction engine, reopening and preparing a new patch.
Andras Becsi
Comment 7 2012-03-16 07:53:12 PDT
Created attachment 132283 [details] proposed patch
Kenneth Rohde Christiansen
Comment 8 2012-03-16 08:11:37 PDT
Comment on attachment 132283 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=132283&action=review > Source/WebKit2/ChangeLog:10 > + ongoing kinetic scroll or bounce-back animation from a previous pan > + gesture the animation is stopped and the content is positioned back weird english > Source/WebKit2/ChangeLog:11 > + to valid boundaries immediately. I would use immediately earlier > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:498 > + // ongoing kinetic scroll or bounce-back animation from a previous pan > + // gesture the animation is stopped and the content is positioned back We only want that for kinetic scroll. Bounce back should be handled differently. ie. if there is a bounce back or other kind of scale animation (tap to zoom) then it should be completed before other events can be accepted
Philippe Normand
Comment 9 2012-03-16 08:33:54 PDT
Comment on attachment 132283 [details] proposed patch Attachment 132283 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11962348
Andras Becsi
Comment 10 2012-03-16 09:37:12 PDT
(In reply to comment #9) > (From update of attachment 132283 [details]) > Attachment 132283 [details] did not pass gtk-ews (gtk): > Output: http://queues.webkit.org/results/11962348 This looks bogus to me, the ews seem busted.
Philippe Normand
Comment 11 2012-03-16 09:42:24 PDT
(In reply to comment #10) > (In reply to comment #9) > > (From update of attachment 132283 [details] [details]) > > Attachment 132283 [details] [details] did not pass gtk-ews (gtk): > > Output: http://queues.webkit.org/results/11962348 > > This looks bogus to me, the ews seem busted. It was indeed. I've fixed this since then! Sorry for the noise
Andras Becsi
Comment 12 2012-03-16 09:43:55 PDT
Andras Becsi
Comment 13 2012-03-16 09:44:46 PDT
Comment on attachment 132283 [details] proposed patch Clearing flags from attachment.
Dinu Jacob
Comment 14 2012-03-16 12:58:09 PDT
*** Bug 81382 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.