[Webkit-unassigned] [Bug 32432] [Qt] Add support for touch events in QWebView and QGraphicsWebView

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=32432


Simon Hausmann <hausmann at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #44866|review?                     |review-
               Flag|                            |




--- Comment #3 from Simon Hausmann <hausmann at webkit.org>  2009-12-15 06:20:27 PST ---
(From update of attachment 44866)
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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.