[webkit-reviews] review denied: [Bug 64297] [Qt][WK2] Add the Web Inspector to WebKit2 : [Attachment 105110] Updated Web Inspector patch for Qt5 WebKit2. Patch modified according Alexis comments. Fixed Alphabetical sorting problem

[bugzilla-daemon at webkit.org]

Noam Rosenthal <noam.rosenthal at nokia.com> has denied Genisim
<genisim at yahoo.com>'s request for review:
Bug 64297: [Qt][WK2] Add the Web Inspector to WebKit2
https://bugs.webkit.org/show_bug.cgi?id=64297

Attachment 105110: Updated Web Inspector patch for Qt5 WebKit2. Patch modified
according Alexis comments. Fixed Alphabetical sorting problem
https://bugs.webkit.org/attachment.cgi?id=105110&action=review

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


In general it's going in a good direction. See comments.

>> Source/WebKit2/UIProcess/API/qt/qdesktopwebview.cpp:42
>> +#include "WebInspectorProxy.h"
> 
> Alphabetical sorting problem.  [build/include_order] [4]

Please fix.

> Source/WebKit2/UIProcess/API/qt/qdesktopwebview.cpp:319
> +    if (toImpl(m_inspector)->isVisible()) {
> +	   toImpl(m_preferences)->setDeveloperExtrasEnabled(false);
> +	   toImpl(m_inspector)->close();
> +    } else {
> +	   toImpl(m_preferences)->setDeveloperExtrasEnabled(true);
> +	   toImpl(m_inspector)->show();
> +    }

Wouldn't work. You need to separate the developer-extras setting from showing
the inspector; otherwise inspector would only collect information from the page
while it's shown.

> Source/WebKit2/UIProcess/API/qt/qtouchwebview.cpp:116
> +void QTouchWebView::toggleWebInspector()
> +{
> +}

Maybe add notImplemented() or a FIXME comment.