Bug 32432

Summary: [Qt] Add support for touch events in QWebView and QGraphicsWebView
Product: WebKit Reporter: Simon Hausmann <hausmann>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, jonni.rainisto, kim.1.gronholm, webkit.review.bot
Priority: P3 Keywords: Qt
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 32434, 32485    
Attachments:
Description Flags
Patch for adding support for touch events in QWebView and QGraphicsWebView.
hausmann: review-
Updated patch according to Simon's comments none

Simon Hausmann
Reported 2009-12-11 08:01:43 PST
QWebPage has been extended with support for handling QTouchEvents and forwarding them to the event handler. However what is still missing is 1) Forwarding touch events from QGraphicsWebView and QWebView to QWebPage 2) Enabling the WA_AcceptsTouch widget attribute It may seem like we would have to dynamically toggle the WA_AcceptsTouchEvents widget attribute, but since in the long run we want to have have QWebPage also interpret touch events (for example for panning), we might as well enable it all the time ;-)
Attachments
Patch for adding support for touch events in QWebView and QGraphicsWebView. (2.63 KB, patch)
2009-12-15 04:53 PST, Kim Grönholm 		hausmann: review-
DetailsFormatted DiffDiff
Updated patch according to Simon's comments (2.73 KB, patch)
2009-12-15 13:57 PST, Kim Grönholm 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Kim Grönholm
Comment 1 2009-12-15 04:53:14 PST
Created attachment 44866 [details] Patch for adding support for touch events in QWebView and QGraphicsWebView.
WebKit Review Bot
Comment 2 2009-12-15 04:55:12 PST
style-queue ran check-webkit-style on attachment 44866 [details] without any errors.
Simon Hausmann
Comment 3 2009-12-15 06:20:27 PST
Comment on attachment 44866 [details] Patch for adding support for touch events in QWebView and QGraphicsWebView. The patch looks good to me! Just a few minor style nitpicks that need to be fixed before we can land this: > + > +#if QT_VERSION >= QT_VERSION_CHECK(4, 6, 0) > + if (d->page) { > + if (event->type() == QEvent::TouchBegin > + || event->type() == QEvent::TouchEnd > + || event->type() == QEvent::TouchUpdate) { > + d->page->event(event); > + } The curly braces are left out when the body is only a one-liner. I would recommend combining the two if statements: if (d->page && event->type() == ... && ... ) d->page->event(event); Also, with the above code we always call QWidget::event() with the touch event, which will convert the first touch to a mouse event (according to the docs). I think perhaps the code should be like: if (...) { d->page->event(event); if (event->isAccepted()) return true; } ... else we call QWidget::event. The same for QGraphicsWebView and QWebView of course.
Kim Grönholm
Comment 4 2009-12-15 13:57:59 PST
Created attachment 44908 [details] Updated patch according to Simon's comments
WebKit Review Bot
Comment 5 2009-12-15 14:02:33 PST
style-queue ran check-webkit-style on attachment 44908 [details] without any errors.
Simon Hausmann
Comment 6 2009-12-17 08:58:29 PST
Comment on attachment 44908 [details] Updated patch according to Simon's comments r=me
Benjamin Poulain
Comment 7 2009-12-17 09:04:36 PST
Comment on attachment 44908 [details] Updated patch according to Simon's comments >+ if (d->page && (event->type() == QEvent::TouchBegin >+ || event->type() == QEvent::TouchEnd >+ || event->type() == QEvent::TouchUpdate)) { I think the indentation is not correct here. Looks good otherwise.
WebKit Commit Bot
Comment 8 2009-12-17 09:09:33 PST
Comment on attachment 44908 [details] Updated patch according to Simon's comments Clearing flags on attachment: 44908 Committed r52256: <http://trac.webkit.org/changeset/52256>
WebKit Commit Bot
Comment 9 2009-12-17 09:09:40 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.