| Summary: | [Qt] Add support for touch events in QWebView and QGraphicsWebView | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Simon Hausmann <hausmann> | ||||||
| Component: | New Bugs | Assignee: | 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: |
|
||||||||
Created attachment 44866 [details]
Patch for adding support for touch events in QWebView and QGraphicsWebView.
style-queue ran check-webkit-style on attachment 44866 [details] without any errors.
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. Created attachment 44908 [details]
Updated patch according to Simon's comments
style-queue ran check-webkit-style on attachment 44908 [details] without any errors.
Comment on attachment 44908 [details]
Updated patch according to Simon's comments
r=me
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. 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> All reviewed patches have been landed. Closing bug. |
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 ;-)