[Webkit-unassigned] [Bug 64297] [Qt][WK2] Add the Web Inspector to WebKit2

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=64297


Benjamin Poulain <benjamin at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #103333|review?                     |review-
               Flag|                            |




--- Comment #7 from Benjamin Poulain <benjamin at webkit.org>  2011-08-09 02:31:37 PST ---
(From update of attachment 103333)
View in context: https://bugs.webkit.org/attachment.cgi?id=103333&action=review

You should update to Qt 5 first, and look how to expose the inspector in the cleanest way possible.

> ChangeLog:3
> +        Add WebInspector to WebKit2 Qt4.7 (revision 91860) MiniBrowser

You should update the title. The task title is now "[Qt][WK2] Add the Web Inspector to WebKit2"

> Source/WebKit.pro:12
> -    lessThan(QT_MAJOR_VERSION, 5) {
> +    lessThan(QT_MAJOR_VERSION, 4) {

Just no :)
WebKit2 trunk does not even build with Qt 4 anymore.

> Source/WebKit2/UIProcess/WebInspectorProxy.cpp:47
> -
> +    

Junk change

> Source/WebKit2/UIProcess/WebInspectorProxy.cpp:94
> +    m_inspectorView = 0;

This is defined only for platform Qt, this will not compile on the other ports.

> Source/WebKit2/UIProcess/WebInspectorProxy.h:52
> +#include <WKView.h>
> +#include <QGraphicsView>
> +#include <QGraphicsScene>

You should use forward declaration.
Those are not the right types.

> Source/WebKit2/UIProcess/API/qt/qdesktopwebview.cpp:35
> -    : q(q)
> +    : q(q ? q : new QDesktopWebView(contextRef, pageGroupRef))

This is a really bad idea. You create implicitely a view that nobody own.

> Source/WebKit2/UIProcess/API/qt/qdesktopwebview.cpp:197
> +void QDesktopWebView::toggleWebInspector()
> +{
> +    WKPageGroupRef pageGroupRef = WKPageGetPageGroup(pageRef());
> +    WKPreferencesRef preferences = WKPageGroupGetPreferences(pageGroupRef);
> +    if (WKInspectorIsVisible(WKPageGetInspector(pageRef()))) { 
> +        WKPreferencesSetDeveloperExtrasEnabled(preferences, false);
> +        WKInspectorClose(WKPageGetInspector(pageRef()));
> +    } else {
> +        WKPreferencesSetDeveloperExtrasEnabled(preferences, true);
> +        WKInspectorShow(WKPageGetInspector(pageRef()));
> +    }
> +}

The interface should be in the view, but the implementation should not. This is typically code for QtWebPageProxy.

> Source/WebKit2/UIProcess/API/qt/qdesktopwebview.h:51
> +    void toggleWebInspector();

We do not want to expose that in QDesktopWebView.
The API should also be property based so it is accessible easily in QML2.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.