[webkit-reviews] review granted: [Bug 79119] [Qt][WK2] Infinite loop on history navigation, when panning. : [Attachment 129877] updated patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has granted Andras Becsi
<abecsi at webkit.org>'s request for review:
Bug 79119: [Qt][WK2] Infinite loop on history navigation, when panning.
https://bugs.webkit.org/show_bug.cgi?id=79119

Attachment 129877: updated patch
https://bugs.webkit.org/attachment.cgi?id=129877&action=review

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=129877&action=review


r=me with a few quirks :)

> Tools/MiniBrowser/qt/MiniBrowserApplication.cpp:41
> +    static QRectF touchRect(0, 0, 40, 40);

I think the static keyboard should be removed. I doubt that there's a
performance impact here, and IMO it makes the code harder to read ("What does
he mean with static here?")

> Tools/MiniBrowser/qt/MiniBrowserApplication.cpp:138
> +	   static QPointF lastPos;

Why not make this a member variable? IMHO that's cleaner.

> Tools/MiniBrowser/qt/MiniBrowserApplication.cpp:147
> +	   static QPointF lastScreenPos;

Same here.

> Tools/MiniBrowser/qt/MiniBrowserApplication.cpp:168
> +		   event->accept();

Perhaps a comment here (as well as below) would be good here, to explain why we
_swallow_ the event instead of delivering it. (flickable mess...)