[webkit-reviews] review denied: [Bug 70315] [Qt][WK2] Rewrite MiniBrowser in QML : [Attachment 111980] patch for review.

[bugzilla-daemon at webkit.org]

Tor Arne Vestbø <vestbo at webkit.org> has denied	review:
Bug 70315: [Qt][WK2] Rewrite MiniBrowser in QML
https://bugs.webkit.org/show_bug.cgi?id=70315

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

------- Additional Comments from Tor Arne Vestbø <vestbo at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=111980&action=review


> Tools/MiniBrowser/qt/BrowserWindow.cpp:37
> +#include <QtDeclarative/QDeclarativeEngine>

#include <QDeclarativeEngine> should be enough

> Tools/MiniBrowser/qt/BrowserWindow.cpp:40
> +Q_DECLARE_METATYPE(QDesktopWebView*)
> +Q_DECLARE_METATYPE(QTouchWebView*)

Do we still need these?

> Tools/MiniBrowser/qt/BrowserWindow.cpp:90
> +    if ((event->modifiers() == Qt::ControlModifier && event->key() ==
Qt::Key_L)
> +	   || event->key() == Qt::Key_F6) {
> +	   QMetaObject::invokeMethod(rootObject(), "setFocusToAddressBar",
Qt::DirectConnection);
> +	   event->accept();
> +    } else
> +	   QSGView::keyPressEvent(event);

Can we do this using http://doc.qt.nokia.com/4.7-snapshot/qml-keys.html ?

> Tools/MiniBrowser/qt/MiniBrowserApplication.cpp:76
> +    qmlRegisterType<QWebNavigationController>();

We don't register this in the qml plugin?

> Tools/MiniBrowser/qt/MiniBrowserApplication.h:48
> +    WindowOptions(QObject * parent = 0)

style: star

> Tools/MiniBrowser/qt/MiniBrowserApplication.h:61
> +    void setPrintLoadedUrls(bool p) { m_printLoadedUrls = p; }
> +    bool printLoadedUrls() const { return m_printLoadedUrls; }
> +    void setUseTouchWebView(bool t) { m_useTouchWebView = t; }
> +    bool useTouchWebView() const { return m_useTouchWebView; }
> +    void setStartMaximized(bool m) { m_startMaximized = m; }
> +    bool startMaximized() const { return m_startMaximized; }

use words, not single characters for the arguments

> Tools/MiniBrowser/qt/qml/BrowserWindow.qml:5
> +    //do not define anchors or a initialisize here! This would messup with
QSGViewSizeRootObjetToView.

initial size, "mess up".