[webkit-reviews] review denied: [Bug 41995] Enable binding QObjects to javascript environment for inspector frontend : [Attachment 61104] patch to enable binding QObject to the javascript environment of a QWebInspector

[bugzilla-daemon at webkit.org]

Andreas Kling <andreas.kling at nokia.com> has denied Jamey Hicks
<jamey.hicks at nokia.com>'s request for review:
Bug 41995: Enable binding QObjects to javascript environment for inspector
frontend
https://bugs.webkit.org/show_bug.cgi?id=41995

Attachment 61104: patch to enable binding QObject to the javascript environment
of a QWebInspector
https://bugs.webkit.org/attachment.cgi?id=61104&action=review

------- Additional Comments from Andreas Kling <andreas.kling at nokia.com>
First off, every patch needs a ChangeLog entry.
You can use WebKitTools/Scripts/prepare-ChangeLog to generate most of it.

>WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:79
> +	     QVariant qvJsObjectMap = property("_q_inspector_js_objects");
Missing #ifndef QT_NO_PROPERTIES

>WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:80
> +	     if (qvJsObjectMap.isValid()) {
WebKit normally uses early-return style, so this would be:
if (!qvJsObjectMap.isValid())
    return;

>WebKit/qt/WebCoreSupport/InspectorClientQt.cpp:123
> +	  QVariant qvJsObjectMap =
inspector->property("_q_inspector_js_objects");
One too many spaces in the indentation here.

r- for missing QT_NO_PROPERTIES check. Change itself looks reasonable to me.