[webkit-reviews] review granted: [Bug 72030] [Qt] the QQuickWebView should support double-tap to zoom : [Attachment 114515] Rebased

[bugzilla-daemon at webkit.org]

Andreas Kling <kling at webkit.org> has granted Kenneth Rohde Christiansen
<kenneth at webkit.org>'s request for review:
Bug 72030: [Qt] the QQuickWebView should support double-tap to zoom
https://bugs.webkit.org/show_bug.cgi?id=72030

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

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


r=me with some cool stories, bro.

> Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:251
> +    if (scrollAnimationActive() || scaleAnimationActive())
> +	   return;
> +
> +    if (!targetArea.isValid())
> +	   return;

Nit: I'd put the !targetArea.isValid() check first, since it's inline.

> Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:253
> +    int margin = 10; // We want at least a little bit or margin.

This variable should be const.

> Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:273
> +    QRectF endPosRange = computePosRangeForItemAtScale(endItemScale);

endPositionRange would be a better name I think.
Actually, thinking about it, I personally prefer "final" to "end" in this
context, but maybe "end" is an existing QtQuick convention. Regardless, NABD.