[webkit-reviews] review denied: [Bug 44330] [Qt] WebKit2 needs to support touchevents : [Attachment 64942] This patch adds QTouchvents-support for webkit2-qt

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has denied Juha Savolainen
<juha.savolainen at weego.fi>'s request for review:
Bug 44330: [Qt] WebKit2 needs to support touchevents
https://bugs.webkit.org/show_bug.cgi?id=44330

Attachment 64942: This patch adds QTouchvents-support for webkit2-qt
https://bugs.webkit.org/attachment.cgi?id=64942&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
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