Bug 44330

Summary: [Qt] WebKit2 needs to support touchevents
Product: WebKit Reporter: Juha Savolainen <juha.savolainen>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Enhancement CC: andersca, commit-queue, hausmann, kenneth, kim.1.gronholm, koivisto, sam, tonikitoo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
This patch adds QTouchvents-support for webkit2-qt
kenneth: review-, kenneth: commit-queue-
Fixed version(review comments)
kenneth: review+
Fixed patch none

Description Juha Savolainen 2010-08-20 03:50:25 PDT
[Qt] WebKit2 needs to support QTouchEvents.
Comment 1 Juha Savolainen 2010-08-20 03:57:39 PDT
Created attachment 64942 [details]
This patch adds QTouchvents-support for webkit2-qt
Comment 2 Kenneth Rohde Christiansen 2010-08-20 04:14:40 PDT
Comment on attachment 64942 [details]
This patch adds QTouchvents-support for webkit2-qt

WebKit2/ChangeLog:6
 +          https://bugs.webkit.org/show_bug.cgi?id=44330
The ChangeLog could be a bit more descriptive about what you did, like why and how.

WebKit2/Shared/WebEvent.h:57
 +         , TouchMove
The , should be last. The first one is acceptable only due to it not compiling without and I might even add that on a separate line

WebKit2/Shared/WebEvent.h:409
 +      Type type() const { return (Type)m_type; }
Why not use a C++ cast instead?

WebKit2/Shared/WebEvent.h:418
 +      static bool decode(CoreIPC::ArgumentDecoder* decoder, WebTouchEvent& t)
I think "ev" or "event" makes more sense than t

WebKit2/Shared/WebEvent.h:426
 +      return true;
indentation issue here

WebKit2/Shared/WebEvent.h:442
 +  
additional newline, remove

WebKit2/Shared/WebEventConversion.cpp:157
 +      WebKit2PlatformTouchPoint(const WebTouchPoint& webTouchPoint)
The contents of this method has wrong indentation

WebKit2/Shared/WebEventConversion.cpp:157
 +      WebKit2PlatformTouchPoint(const WebTouchPoint& webTouchPoint)
Maybe this class should be called WebPlatformTouchPoint for consistency reasons?

WebKit2/Shared/WebEventConversion.cpp:183
 +  
extra newline, remove

WebKit2/Shared/WebEventConversion.cpp:185
 +  
here as well

WebKit2/Shared/WebEventConversion.cpp:192
 +  
here as well plus the whole method has wrong indentation

WebKit2/Shared/qt/WebEventFactoryQt.cpp:187
 +          case Qt::TouchPointReleased: state = WebTouchPoint::TouchReleased; break;
wrong coding style!

WebKit2/UIProcess/API/qt/qgraphicswkview.cpp:153
 +                  || event->type() == QEvent::TouchEnd
wrong coding style

WebKit2/UIProcess/API/qt/qgraphicswkview.cpp:152
 +      if ( event->type() == QEvent::TouchBegin
wrong coding style
Comment 3 Juha Savolainen 2010-08-20 05:54:52 PDT
(In reply to comment #2)
> (From update of attachment 64942 [details])

> WebKit2/Shared/WebEvent.h:409
>  +      Type type() const { return (Type)m_type; }
> Why not use a C++ cast instead?

We remove this because we can use function from base-class.

> WebKit2/Shared/WebEvent.h:418
>  +      static bool decode(CoreIPC::ArgumentDecoder* decoder, WebTouchEvent& t)
> I think "ev" or "event" makes more sense than t

This function is inherited from WebEvent and also all other WebEventXXX-classes has named parameters in the same way.
Comment 4 Juha Savolainen 2010-08-20 05:57:27 PDT
Created attachment 64948 [details]
Fixed version(review comments)
Comment 5 Kenneth Rohde Christiansen 2010-08-20 06:04:37 PDT
Comment on attachment 64948 [details]
Fixed version(review comments)

WebKit2/UIProcess/API/qt/qgraphicswkview.cpp:153
 +             || event->type() == QEvent::TouchEnd
This indentation here wasnt fixed

WebKit2/UIProcess/API/qt/qgraphicswkview.h:69
 +      virtual void touchEvent(QTouchEvent* ev);
remove ev here

R=me, with the above changes.
Comment 6 Juha Savolainen 2010-08-21 01:57:53 PDT
Created attachment 65023 [details]
Fixed patch
Comment 7 WebKit Commit Bot 2010-08-22 15:43:06 PDT
Comment on attachment 65023 [details]
Fixed patch

Clearing flags on attachment: 65023

Committed r65788: <http://trac.webkit.org/changeset/65788>