[Webkit-unassigned] [Bug 19392] [Gtk] Enable WebInspector in the Gtk port

[bugzilla-daemon at webkit.org]

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


christian at imendio.com changed:

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




------- Comment #21 from christian at imendio.com  2008-07-26 16:21 PDT -------
(From update of attachment 22364)
>+    /**
>+     * WebKitWebInspector::destroy:
>+     * @web_inspector: the object on which the signal is emitted
>+     *
>+     * Emitted when the inspector is destroyed.
>+     */
>+    webkit_web_inspector_signals[DESTROY] = g_signal_new("destroy",
>+            G_TYPE_FROM_CLASS(klass),
>+            (GSignalFlags)(G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION),
>+            0,
>+            NULL,
>+            NULL,
>+            g_cclosure_marshal_VOID__VOID,
>+            G_TYPE_NONE , 0);

GtkObject, and therefore every GtkWidget, has a "destroy" signal with that same
signature, that means "the object is disposed, with no way to stop that". But
this signal merely means "finished" and it's still possible to keep a reference
of the instance.
I suggest the signal should have a different name to avoid confusion about how
this signal works.

>+static void webkit_web_inspector_finalize(GObject* object)
>+{
>+    WebKitWebInspector* web_inspector = WEBKIT_WEB_INSPECTOR(object);
>+    WebKitWebInspectorPrivate* priv = web_inspector->priv;
>+
>+    if (priv->inspector_view)
>+        g_object_unref(priv->inspector_view);
>+
>+    if (priv->inspected_uri)
>+        g_free(priv->inspected_uri);

No need to check whether a string array is non-NULL, g_free does that
implicitly.

>+void webkit_web_inspector_set_web_view(WebKitWebInspector *web_inspector, WebKitWebView *web_view)
>+{
>+    g_return_if_fail(WEBKIT_IS_WEB_INSPECTOR(web_inspector));
>+    g_return_if_fail(WEBKIT_IS_WEB_VIEW(web_view));
>+
>+    WebKitWebInspectorPrivate* priv = web_inspector->priv;
>+
>+    if (priv->inspector_view)
>+        g_object_unref(priv->inspector_view);
>+    
>+    g_object_ref(web_view);
>+    priv->inspector_view = web_view;
>+}

No documentation at all here... Please add that.

And, would it be a good idea to allow _set_web_view with a NULL web_view to
unset the web view?

>+void webkit_web_inspector_set_inspected_uri(WebKitWebInspector* web_inspector, const gchar* inspected_uri)
>+{
>+    g_return_if_fail(WEBKIT_IS_WEB_INSPECTOR(web_inspector));
>+
>+    WebKitWebInspectorPrivate* priv = web_inspector->priv;
>+
>+    if (priv->inspected_uri)
>+        g_free(priv->inspected_uri);
>+    
>+    priv->inspected_uri = g_strdup(inspected_uri);
>+}

Again, no need for the if (priv->inspected_uri).

>+    /**
>+    * WebKitWebSettings:enable-developer-extras:
>+    *
>+    * Whether developer extensions should be enabled.
>+    *
>+    * Since: 1.0.2
>+    */

I suggest you should mention the Web Inspector expressly here. Otherwise
"special extensions" is rather meaningless to newcomers.

>@@ -1206,6 +1212,13 @@ static void webkit_web_view_class_init(WebKitWebViewClass* webViewClass)
>                                                         WEBKIT_TYPE_WEB_SETTINGS,
>                                                         WEBKIT_PARAM_READWRITE));
> 
>+    g_object_class_install_property(objectClass, PROP_WEB_INSPECTOR,
>+                                    g_param_spec_object("web-inspector",
>+                                                        "Web Inspector",
>+                                                        "The associated WebKitWebInspector instance",
>+                                                        WEBKIT_TYPE_WEB_INSPECTOR,
>+                                                        WEBKIT_PARAM_READABLE));

This needs documentation, particularly a Since tag.

>+WebKitWebInspector* webkit_web_view_get_inspector(WebKitWebView* webView)
>+{
>+    g_return_val_if_fail(WEBKIT_IS_WEB_VIEW(webView), NULL);
>+
>+    WebKitWebViewPrivate* priv = webView->priv;
>+    return priv->webInspector;
>+}

Documentation missing. Particularly it should be documented if this can be
NULL.


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