[webkit-reviews] review granted: [Bug 69617] [Qt][WK2] Touch mocking is broken with Qt 5 post refactor merge : [Attachment 110123] Patch

[bugzilla-daemon at webkit.org]

Andreas Kling <kling at webkit.org> has granted Simon Hausmann
<hausmann at webkit.org>'s request for review:
Bug 69617: [Qt][WK2] Touch mocking is broken with Qt 5 post refactor merge
https://bugs.webkit.org/show_bug.cgi?id=69617

Attachment 110123: Patch
https://bugs.webkit.org/attachment.cgi?id=110123&action=review

------- Additional Comments from Andreas Kling <kling at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=110123&action=review


r=me with some tweaks:

> Tools/MiniBrowser/qt/MiniBrowserApplication.cpp:96
> +    if (isMouseEvent(event) && targetWindow) {

Nit: The targetWindow null check is cheaper so it should be first.

> Tools/MiniBrowser/qt/MiniBrowserApplication.cpp:100
> +	   touchPoint.area = QRectF(mouseEvent->globalPos(), QSizeF(1., 1.));

Style: Unless required in order to force floating point math, do not append .0,
.f and .0f to floating point literals.

> Tools/MiniBrowser/qt/MiniBrowserApplication.cpp:132
> +	   for (QHash<int, QWindowSystemInterface::TouchPoint>::iterator it =
m_touchPoints.begin(); it != m_touchPoints.end(); ++it) {

Nit: I like it when we cache the end iterator. :)

> Tools/MiniBrowser/qt/MiniBrowserApplication.cpp:142
> +	   QEvent::Type eventType;
> +	    switch (touchPoint.state) {

Something goes wrong with indentation here.

> Tools/MiniBrowser/qt/MiniBrowserApplication.cpp:150
> +		// don't send the event if nothing changed

Style: comments should be in sentence form.