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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jul 16 03:23:17 PDT 2008


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





------- Comment #13 from christian at imendio.com  2008-07-16 03:23 PDT -------
(From update of attachment 22282)
>+     * WebKitWebInspector::inspect-web-view:
>+     * @web_inspector: the object on which the signal is emitted
>+     * @web_view: the #WebKitWeb which will be inspected
>+     * @return: a newly allocated #WebKitWebView or %NULL
>+     *
>+     * Emitted when the user activates the 'inspect' context menu item
>+     * to inspect a web view. The application which is interested in
>+     * the inspector should create a window, or otherwise add the
>+     * #WebKitWebView it creates to an existing window.
>+     *
>+     * The #WebKitWebView must never be destroyed; that means that you
>+     * probably want to handle the delete-event and destory signals of
>+     * windows containing the inspector #WebKitWebView, so that you
>+     * can hide them when they are closed, and show them again when
>+     * ::show-window is emitted.

This is slightly wrong, when "destroy" is emitted it's already too late for
saving the window, the only remaining option is to reparent the widget.

Would it be too complicated to work around the issue of a destroyed web view on
the WebKit side? It should be possible, to unset the view via
gtk_widget_destroyed and request a new web view as needed, shouldn't it?

>+     * WebKitWebInspector::attach-window:
>+     * @web_inspector: the object on which the signal is emitted
>+     * @return: %TRUE if the signal has been handled
>+     *
>+     * Emitted when the inspector window should be attached to the
>+     * window in which the #WebKitWebView it is inspecting is located.
>+     */
>+    webkit_web_inspector_signals[ATTACH_WINDOW] = g_signal_new("attach-window",
>+            G_TYPE_FROM_CLASS(klass),
>+            (GSignalFlags)(G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION),
>+            G_STRUCT_OFFSET (WebKitWebInspectorClass, attach_window),
>+            g_signal_accumulator_true_handled,
>+            NULL,
>+            webkit_marshal_BOOLEAN__VOID,
>+            G_TYPE_BOOLEAN , 0);

Could the documentation be a little clearer? To me "attaching to a window"
doesn't intuitively make much sense.

>+     * WebKitWebInspector::uri-changed:
>+     * @web_inspector: the object on which the signal is emitted
>+     * @return: %TRUE if the signal has been handled
>+     *
>+     * Emitted when the URI being inspected changes.
>+     */
>+    webkit_web_inspector_signals[URI_CHANGED] = g_signal_new("uri-changed",
>+            G_TYPE_FROM_CLASS(klass),
>+            (GSignalFlags)(G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION),
>+            G_STRUCT_OFFSET (WebKitWebInspectorClass, uri_changed),
>+            g_signal_accumulator_true_handled,
>+            NULL,
>+            webkit_marshal_BOOLEAN__STRING,
>+            G_TYPE_BOOLEAN , 1,
>+            G_TYPE_STRING);

I suggest an URI property instead of a signal. The current patch doesn't seem
to do anything with the boolean return value anyway.

>+    /**
>+     * WebKitWebInspector::inspector-destroyed:
>+     * @web_inspector: the object on which the signal is emitted
>+     * @return: %TRUE if the signal has been handled
>+     *
>+     * Emitted when the inspector is destroyed.
>+     */
>+    webkit_web_inspector_signals[INSPECTOR_DESTROYED] = g_signal_new("inspector-destroyed",
>+            G_TYPE_FROM_CLASS(klass),
>+            (GSignalFlags)(G_SIGNAL_RUN_LAST | G_SIGNAL_ACTION),
>+            G_STRUCT_OFFSET (WebKitWebInspectorClass, inspector_destroyed),
>+            g_signal_accumulator_true_handled,
>+            NULL,
>+            webkit_marshal_BOOLEAN__VOID,
>+            G_TYPE_BOOLEAN , 0);

The boolean return value isn't used at all. Would it make sense to use
GtkObject, gtk_object_destroy and "destroy" here? If not, what about
s/inspector-destroyed/destroy, which is obvious enough as I see it.

>+    /*
>+     * properties
>+     */
>+    g_object_class_install_property(gobject_class, PROP_INSPECTOR_WEB_VIEW,
>+                                    g_param_spec_object("inspector-web-view",
>+                                                        "Inspector Web View",
>+                                                        "The Web View that renders the Web Inspector itself",
>+                                                        WEBKIT_TYPE_WEB_VIEW,
>+                                                        WEBKIT_PARAM_READWRITE));

What about s/inspector-web-view/web-view, since it's probably obvious enough.

>+     */
>+    klass->inspect_web_view = webkit_web_inspector_real_inspect_web_view;
>+    klass->show_window = webkit_web_inspector_real_show_window;
>+    klass->attach_window = webkit_web_inspector_real_attach_window;
>+    klass->dettach_window = webkit_web_inspector_real_dettach_window;
>+    klass->close_window = webkit_web_inspector_real_close_window;
>+    klass->uri_changed = webkit_web_inspector_real_uri_changed;
>+    klass->inspector_destroyed = webkit_web_inspector_real_inspector_destroyed;

Do we need all those defaults, most of which are empty placeholders?

>@@ -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_INSPECTOR,
>+                                    g_param_spec_object("inspector",
>+                                                        "Inspector",
>+                                                        "The associated WebKitWebInspector instance",
>+                                                        WEBKIT_TYPE_WEB_INSPECTOR,
>+                                                        WEBKIT_PARAM_READABLE));
>+

The property ought to be "web-inspector", right?

Looking at the code I see the use case of the WebInspector class and I think
it's actually a good idea. I imagine you can keep one application wide instance
around like you can with the WebSettings.


-- 
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.



More information about the webkit-unassigned mailing list