Bug 42892 – [Qt] Decouple QTouchEvent's accepted flag from JS prevent default

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Kim Grönholm]

Currently the QTouchEvent is not accepted if the default action was not prevented in the touchevent 
listener. This results in touchmove and touchend events not being sent if touchstart event listener 
doesn't exist or doesn't prevent the default action. This is due to the fact that Qt sends
TouchUpdate and TouchEnd events only if TouchBegin was accepted.

We need to always accept the QTouchEvents in order to get TouchUpdate and TouchEnd events, and use the
QWebView::event() and QGraphicsWebView::sceneEvent() return values for indicating whether the default
action was prevented or not.

[Kim Grönholm]

We can't use the QWebView::event() and QGraphicsWebView::sceneEvent() return values for indicating whether the default action was prevented as they also need to return true in order to get TouchUpdate and TouchEnd events.

Instead we can make the QWebPage::event() to return whether the default action was prevented for the touch events. This way QtWebKit will receive all QTouchEvents without any extra effort needed from the application developer but with some inconvenience the developer is still able to figure out if the default action was prevented for the touch events.

I'll submit a patch for this soon.

[Kim Grönholm]

Created attachment 62648 [details]
Decouple QTouchEvent's accepted flag from JS prevent default

[WebKit Commit Bot]

Comment on attachment 62648 [details]
Decouple QTouchEvent's accepted flag from JS prevent default

Clearing flags on attachment: 62648

Committed r64118: <http://trac.webkit.org/changeset/64118>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Antonio Gomes]

It'd be great if changes like this in the code had comment, so one would not remove it, and would understand easily why it is as is.

kim, could you add it?

[Kim Grönholm]

Created attachment 62798 [details]
Commenting touch event handling code

Sure, here comes a patch :)

[Antonio Gomes]

Reopening to add this nice missing bit: the comment :)

[Antonio Gomes]

Comment on attachment 62798 [details]
Commenting touch event handling code


> diff --git a/WebKit/qt/Api/qgraphicswebview.cpp b/WebKit/qt/Api/qgraphicswebview.cpp
> index 853e637..8a94c9b 100644
> --- a/WebKit/qt/Api/qgraphicswebview.cpp
> +++ b/WebKit/qt/Api/qgraphicswebview.cpp
> @@ -319,6 +319,8 @@ bool QGraphicsWebView::sceneEvent(QEvent* event)
>                  || event->type() == QEvent::TouchEnd
>                  || event->type() == QEvent::TouchUpdate)) {
>          d->page->event(event);
> +
> +        // Always return true so that we'll receive also TouchUpdate and TouchEnd events
>          return true;

From QWebPage::event you return if the event was canceled in JS, but you ignore the value returned here and on QWebView's case as well.

>      }
>  #endif
> diff --git a/WebKit/qt/Api/qwebpage.cpp b/WebKit/qt/Api/qwebpage.cpp
> index 1dda884..b689062 100644
> --- a/WebKit/qt/Api/qwebpage.cpp
> +++ b/WebKit/qt/Api/qwebpage.cpp
> @@ -1397,7 +1397,10 @@ bool QWebPagePrivate::touchEvent(QTouchEvent* event)
>      if (!frame->view())
>          return false;
>  
> +    // Always accept the QTouchEvent so that we'll receive also TouchUpdate and TouchEnd events
>      event->setAccepted(true);
> +
> +    // Return whether the default action was cancelled in the JS event handler
>      return frame->eventHandler()->handleTouchEvent(PlatformTouchEvent(event));
>  }
>  #endif
> @@ -2842,6 +2845,7 @@ bool QWebPage::event(QEvent *ev)
>      case QEvent::TouchBegin:
>      case QEvent::TouchUpdate:
>      case QEvent::TouchEnd:
> +        // Return whether the default action was cancelled in the JS event handler
>          return d->touchEvent(static_cast<QTouchEvent*>(ev));

You comments also raised a question: what indicates the touch engine to send follow up TouchUpdate and TouchEnd events? the setAccepted(true) sentence or returning 'true' from sceneEvent? your comments did not clarify that.

[Kim Grönholm]

Yes, I discussed this with Simon and he suggested that doing it like this allows us to always get all touch events and the developer can subclass QWebPage and re-implement event() if the preventDefault information is needed. We know it's inconvenient but Simon didn't want to make any API changes.

In order to get the TouchUpdate and TouchEnd events from Qt, both the accepted flag and the return value must be true as the comments imply.

[Kim Grönholm]

Oops, sorry, I'm typing without thinking :( I meant "... subclass QWebView or QGraphicsWebView and re-implement sceneEvent() or event() ..."

[Antonio Gomes]

Comment on attachment 62798 [details]
Commenting touch event handling code


Ok.


from http://doc.trolltech.com/4.7-snapshot/qtouchevent.html :

"The QEvent::TouchUpdate and QEvent::TouchEnd events are sent to the widget or item that accepted the QEvent::TouchBegin event. If the QEvent::TouchBegin event is not accepted and not filtered by an event filter, then no further touch events are sent until the next QEvent::TouchBegin."

and from http://doc.trolltech.com/4.7-snapshot/qgraphicsitem.html#sceneEvent :
"Returns true if the event was recognized and handled; otherwise, (e.g., if the event type was not recognized,) false is returned."

[WebKit Commit Bot]

Comment on attachment 62798 [details]
Commenting touch event handling code

Clearing flags on attachment: 62798

Committed r64245: <http://trac.webkit.org/changeset/64245>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Simon Hausmann]

Revision r64118 cherry-picked into qtwebkit-2.1 with commit 15e28c036805866fcaff7d11d525ebc46b5b6ddd
Revision r64245 cherry-picked into qtwebkit-2.1 with commit 5b6f155620fb6b368fe07dc33a6e31357bab9394