[Qt][WK2] Refactor the tap gesture recognizer
Created attachment 145313 [details] Patch
Comment on attachment 145313 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=145313&action=review r=me > Source/WebKit2/UIProcess/qt/QtTapGestureRecognizer.cpp:96 > + // The singleTapTimeout was ignored earlier but the gesture has been finished > + // before the tapAndHoldTimeout fired, therefore this is a single tap gesture. > + m_eventHandler->handleSingleTapEvent(touchPoint); I think it would help if the comment would directly say that this happens when the finger is released (gesture finished) after the single tap timeout elapsed (500ms) but _before_ the tapAndHold (1000ms), i.e. explain it using numbers instead of states. Just a thought :) > Source/WebKit2/UIProcess/qt/QtTapGestureRecognizer.h:39 > const int maxPanDistance = 10; > const int maxDoubleTapDistance = 120; > -const int tapAndHoldTime = 800; > -const int maxDoubleTapInterval = 400; > -const int highlightDelay = 80; > +const int tapAndHoldTime = 1000; > +const int maxDoubleTapInterval = 500; > +const int highlightDelay = 100; Why are these global constants in a header file? I understand that due to them being const they will only have internal linkage, but nevertheless they are not shared with any other files and IMHO would fit just as well into the .cpp file. I also wonder if these timeouts should be consistent across the platform where tap gestures are recognized in different places, i.e. if they should be read from some setting? (think platform plugin) For example I can see that our tapAndHoldTime is 1 second while in qquickmouseareap.cpp it's 800ms. That's subtle, but still different. > Source/WebKit2/UIProcess/qt/QtWebPageEventHandler.cpp:505 > if (!m_panGestureRecognizer.isRecognized()) > - m_tapGestureRecognizer.update(ev->type(), touchPoints.first()); > + m_tapGestureRecognizer.finish(touchPoints.first()); > m_panGestureRecognizer.finish(touchPoints.first(), eventTimestampMillis); I think this code would be more readable if it was if (m_panGestureRecognizer.isRecognized()) m_panGestureRecognizer.finish(...); else m_tapGestureRecognizer.finish();
Committed r119385: <http://trac.webkit.org/changeset/119385>
Comment on attachment 145313 [details] Patch Clearing flags from attachment.