[webkit-reviews] review denied: [Bug 84532] [Qt][WK2] Private non-QtQuick API : [Attachment 145713] patch

[bugzilla-daemon at webkit.org]

Noam Rosenthal <noam.rosenthal at nokia.com> has denied Luiz Agostini
<luiz at webkit.org>'s request for review:
Bug 84532: [Qt][WK2] Private non-QtQuick API
https://bugs.webkit.org/show_bug.cgi?id=84532

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

------- Additional Comments from Noam Rosenthal <noam.rosenthal at nokia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=145713&action=review


I think this needs a test, to make sure people don't break it. Also some
nitpicks :)

> Source/WebKit2/ChangeLog:6
> +	   [Qt][WK2] Private non-QtQuick API
> +	   https://bugs.webkit.org/show_bug.cgi?id=84532
> +
> +	   Reviewed by NOBODY (OOPS!).

Needs some explanation about this being a private API (meaning not packaged
together with Qt or part of the Qt port).
Also explain that it's guarded by a qmake CONFIG flag.

> Source/WebKit2/UIProcess/API/qt/WKView.h:23
> +    the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
> +    Boston, MA 02110-1301, USA.
> +*/
> +
> +#ifndef WKView_h
> +#define WKView_h
> +#include "qwebkitglobal.h"
> +

I think this needs a warning that it's not part of the Qt API in any way.

> Source/WebKit2/UIProcess/API/qt/WKViewImpl.cpp:118
> +void WKViewImpl::setUrl(const QUrl& url)
> +{
> +    m_url = url;
> +    m_webPageProxy->loadURL(url.toString());
> +}

I'd rather if we just had loadURL function.

> Source/WebKit2/UIProcess/API/qt/WKViewImpl.h:163
> +
> +

Extra lines

> Tools/MiniBrowser/qtcb/View.cpp:151
> +    qDebug() << __PRETTY_FUNCTION__ << __LINE__;

Need to remove these debug messages before upstreaming.