[webkit-reviews] review denied: [Bug 71355] [Qt] Merge QTouchWebView and QDesktopWebView into one class : [Attachment 114376] Patch

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has denied Alexis Menard
<alexis.menard at openbossa.org>'s request for review:
Bug 71355: [Qt] Merge QTouchWebView and QDesktopWebView into one class
https://bugs.webkit.org/show_bug.cgi?id=71355

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

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=114376&action=review


> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:147
> +    if (ev->type() == QEvent::InputMethod)
> +	   return false; // This is necessary to avoid an endless loop in
connection with QQuickItem::event().

I guess this will need some changes when we will actually start working on IM
support

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:156
> +void QQuickWebPage::itemChange(ItemChange change, const ItemChangeData
&data)

coding style - & placement

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:172
> +void QQuickWebPagePrivate::initSceneGraphConnections()

initialize?

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:191
> +    this->pageProxy = pageProxy;
> +
> +}

unneeded newline

> Source/WebKit2/UIProcess/API/qt/qquickwebpage.cpp:196
> +    if (!item)
> +	   return 1.0;

I think webkit style tells you to just return 1

> Source/WebKit2/UIProcess/API/qt/qquickwebview.cpp:180
> +    wkPrefs->setDeviceHeight(720);
> +
> +    WebCore::ViewportAttributes attr =
WebCore::computeViewportAttributes(viewportArguments,
wkPrefs->layoutFallbackWidth(), wkPrefs->deviceWidth(),
wkPrefs->deviceHeight(), wkPrefs->deviceDPI(), availableSize);
> +    WebCore::restrictMinimumScaleFactorToViewportSize(attr, availableSize);
> +    WebCore::restrictScaleFactorToInitialScaleIfNotUserScalable(attr);

You missed some new change here from yesterday

> Source/WebKit2/UIProcess/API/qt/qquickwebview_p.h:59
> +    static QQuickWebViewPrivate *get(QQuickWebView *view)

coding style