[Qt][WK2] Add support for touch event testing to WebKitTestRunner
Created attachment 113205 [details] Patch work in progress
Created attachment 113290 [details] [Qt][WK2] Add support for touch event testing to WebKitTestRunner
Attachment 113290 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1 Tools/WebKitTestRunner/EventSenderProxy.h:55: The parameter name "modifier" adds no information, so it should be removed. [readability/parameter_name] [5] Tools/WebKitTestRunner/EventSenderProxy.h:71: The parameter name "type" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 2 in 18 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 113292 [details] [Qt][WK2] Add support for touch event testing to WebKitTestRunner
View in context: https://bugs.webkit.org/attachment.cgi?id=113290&action=review > Source/WebKit2/ChangeLog:15 > + (QtDesktopWebPageProxy::handleTouchEvent): Forward touch events to WebPagePRoxy. Spelling issue > Source/WebKit2/UIProcess/WebPageProxy.cpp:963 > + if (m_shouldSendEventsSynchronously) { Make it more obvious that this is for testing only? > Source/WebKit2/WebProcess/WebPage/WebPage.h:512 > + void touchEventSyncForTesting(const WebTouchEvent&, bool&); it is not obvious what the bool means here. I would add 'handled' > Tools/WebKitTestRunner/EventSenderProxy.h:54 > + void updateTouchPoint(int index, int x, int y); isn;t it more an id than an index? At least Lars told me that iOS uses ids and not indices > Tools/WebKitTestRunner/qt/EventSenderProxyQt.cpp:325 > + if (index < 0 || index >= m_touchPoints.count()) Shouldn't this be an assert? > Tools/WebKitTestRunner/qt/EventSenderProxyQt.cpp:363 > + for (int i = 0; i < m_touchPoints.count(); ++i) > + if (m_touchPoints[i].state() != Qt::TouchPointReleased) { > + sendTouchEvent(QEvent::TouchUpdate); > + return; > + } Bad coding style... add braces
(In reply to comment #5) > View in context: https://bugs.webkit.org/attachment.cgi?id=113290&action=review > > > Source/WebKit2/ChangeLog:15 > > + (QtDesktopWebPageProxy::handleTouchEvent): Forward touch events to WebPagePRoxy. > > Spelling issue Oops. fixing :) > > Source/WebKit2/UIProcess/WebPageProxy.cpp:963 > > + if (m_shouldSendEventsSynchronously) { > > Make it more obvious that this is for testing only? Hmm, this is just the same pattern that's also used for mouse events when testing. It's set through WKPageSetShouldSendEventsSynchronously from WTR. > > Source/WebKit2/WebProcess/WebPage/WebPage.h:512 > > + void touchEventSyncForTesting(const WebTouchEvent&, bool&); > > it is not obvious what the bool means here. I would add 'handled' Yeah, it's handled. I copied the signature from mouseEventSyncForTesting, but I can add the handled. > > Tools/WebKitTestRunner/EventSenderProxy.h:54 > > + void updateTouchPoint(int index, int x, int y); > > isn;t it more an id than an index? At least Lars told me that iOS uses ids and not indices No, this one is actually an index, not the touch point id. The API works like this: addTouchPoint(10, 10); // gets index 0 addTouchPoint(20, 20); // second touch point at index 1 touchStart(); updateTouchPoint(1, 30, 30); // Move second touch point to 30, 30 ... > > Tools/WebKitTestRunner/qt/EventSenderProxyQt.cpp:325 > > + if (index < 0 || index >= m_touchPoints.count()) > > Shouldn't this be an assert? Hmm, I guess so. I'll change it :) > > Tools/WebKitTestRunner/qt/EventSenderProxyQt.cpp:363 > > + for (int i = 0; i < m_touchPoints.count(); ++i) > > + if (m_touchPoints[i].state() != Qt::TouchPointReleased) { > > + sendTouchEvent(QEvent::TouchUpdate); > > + return; > > + } > > Bad coding style... add braces Uh, I agree.
Created attachment 113295 [details] [Qt][WK2] Add support for touch event testing to WebKitTestRunner
Comment on attachment 113295 [details] [Qt][WK2] Add support for touch event testing to WebKitTestRunner Great stuff, r=me
Comment on attachment 113295 [details] [Qt][WK2] Add support for touch event testing to WebKitTestRunner Clearing flags on attachment: 113295 Committed r99051: <http://trac.webkit.org/changeset/99051>
All reviewed patches have been landed. Closing bug.