[Webkit-unassigned] [Bug 68235] [GTK][WK2] Initial implementation of WebInspector

[bugzilla-daemon at webkit.org]

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





--- Comment #6 from Carlos Garcia Campos <cgarcia at igalia.com>  2011-09-18 23:57:50 PST ---
(From update of attachment 107661)
View in context: https://bugs.webkit.org/attachment.cgi?id=107661&action=review

> Source/WebKit2/UIProcess/API/C/gtk/WKView.cpp:50
> +void WKViewSetDeveloperExtrasEnabled(WKPageRef pageRef, bool enable)
> +{
> +    toImpl(pageRef)->setDeveloperExtrasEnabled(enable);
> +}

I don't think we need specific API for this, we should use the existing preferences API, WKPreferencesSetDeveloperExtrasEnabled

> Source/WebKit2/UIProcess/gtk/WebInspectorGtk.cpp:79
> +    m_inspectorView = GTK_WIDGET(webkitWebViewBaseCreate(page()->process()->context(), inspectorPageGroup()));

You can use the C API here instead of the private view base API, m_inspectorView = GTK_WIDGET(WKViewCreate(toAPI(page()->process()->context()), toAPI(inspectorPageGroup()));

> Source/WebKit2/UIProcess/gtk/WebInspectorGtk.cpp:80
> +    gtk_widget_set_size_request(m_inspectorView, initialWindowWidth, initialWindowHeight);

Do you need to enforce the view size to the initial window size?

> Source/WebKit2/UIProcess/gtk/WebInspectorGtk.cpp:82
> +    return webkitWebViewBaseGetPage(WEBKIT_WEB_VIEW_BASE(m_inspectorView));

WKViewGetPage()

>> Source/WebKit2/UIProcess/gtk/WebInspectorGtk.cpp:95
>> +    gtk_widget_show_all(m_inspectorWindow);
> 
> This doesn't seem to handle the embedded view? Almost certainly this should be passed up to the API-layer somehow.

Please, don't use show_all(), show the view widget and the window.

> Source/WebKit2/UIProcess/gtk/WebInspectorGtk.cpp:103
> +    if (m_inspectorView) {
> +        gtk_widget_destroy(m_inspectorView);
> +        m_inspectorView = 0;
> +    }

You don't need to destroy the view, just destroy the window and the view will be destroyed too.

> Source/WebKit2/UIProcess/gtk/WebInspectorGtk.cpp:118
> +    gtk_window_set_title(GTK_WINDOW(m_inspectorWindow), title.latin1().data());

gtk_window_set_title() expects utf8 not latin1

> Source/WebKit2/UIProcess/gtk/WebInspectorGtk.cpp:129
> +    GtkWidget* inspectorWindow = m_page->viewWidget();
> +    return static_cast<unsigned>(gtk_widget_get_allocated_height(inspectorWindow));

Why do you need to get the view from the page? you have m_inspectorWindow

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