[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
bugzilla-daemon at webkit.org
Fri Oct 7 04:58:23 PDT 2011
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.
More information about the webkit-reviews
mailing list