API to allow interact with the inspector and also implement custom attach/detach
Created attachment 143263 [details] Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment on attachment 143263 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=143263&action=review Looks great. Couple nits below, but in general I really like the new API. > Source/WebKit2/UIProcess/API/gtk/WebKitWebInspector.cpp:134 > + * To prevent the inspector from being shown you can connecto the this Nit connectto -> "connect to" > Source/WebKit2/UIProcess/API/gtk/WebKitWebInspector.cpp:150 > + * WebKitWebInspector::bring-to-front: Perhaps this should be called "raise" to match "gdk_window_raise" or "present" to match "gtk_window_present" > Source/WebKit2/UIProcess/API/gtk/WebKitWebInspector.cpp:161 > + * In both cases, if this signal is not hanlded, the default implementation hanlded -> handled > Source/WebKit2/UIProcess/API/gtk/WebKitWebInspector.cpp:200 > + * attached to the inspected view, so you only need tyo handle this signal need tyo -> need to > Source/WebKit2/UIProcess/API/gtk/WebKitWebInspector.cpp:224 > + * Emitted when the inspector is requested to be detached from the window > + * where it currently is. The inspector is detached when the inspector page Perhaps better phrased: "Emitted when the #WebKitWebInspector is requesting to be detached from the window it is currently attached to." > Source/WebKit2/UIProcess/API/gtk/WebKitWebInspector.cpp:226 > + * #WebKitWebInspector::closed, or when the user clicks on detach button on detach button -> on the detach button > Source/WebKit2/UIProcess/API/gtk/WebKitWebInspector.cpp:340 > + * Returns: the URI that is currently being inspected. Can this be %NULL? If so, it would be good to mention it. > Source/WebKit2/UIProcess/API/gtk/tests/TestInspector.cpp:351 > + CustomInspectorTest::add("WebKitWebInspector", "custom", testInspectorCustom); This could probably be more specific about what it's testing. Perhaps "manual-attach-detach" > Source/WebKit2/UIProcess/API/gtk/tests/TestInspector.cpp:352 > + CustomInspectorTest::add("WebKitWebInspector", "inspector-window-destroyed", testInspectorWindowDestroyed); Perhaps "containing-window-destroyed" ?
(In reply to comment #3) > (From update of attachment 143263 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=143263&action=review > > Looks great. Couple nits below, but in general I really like the new API. Thanks!, and sorry for the typos :-( > > Source/WebKit2/UIProcess/API/gtk/WebKitWebInspector.cpp:150 > > + * WebKitWebInspector::bring-to-front: > > Perhaps this should be called "raise" to match "gdk_window_raise" or "present" to match "gtk_window_present" Well, this depends on the implementation, for example, if the inspector is attached in a browser tab, bring-to-front is not just raise or present the window, but also make the tab active. > > Source/WebKit2/UIProcess/API/gtk/WebKitWebInspector.cpp:224 > > + * Emitted when the inspector is requested to be detached from the window > > + * where it currently is. The inspector is detached when the inspector page > > Perhaps better phrased: "Emitted when the #WebKitWebInspector is requesting to be detached from the window it is currently attached to." Indeed. > > Source/WebKit2/UIProcess/API/gtk/WebKitWebInspector.cpp:340 > > + * Returns: the URI that is currently being inspected. > > Can this be %NULL? If so, it would be good to mention it. I think it's about:blank, but I'll check it. > > Source/WebKit2/UIProcess/API/gtk/tests/TestInspector.cpp:351 > > + CustomInspectorTest::add("WebKitWebInspector", "custom", testInspectorCustom); > > This could probably be more specific about what it's testing. Perhaps "manual-attach-detach" Ok. > > Source/WebKit2/UIProcess/API/gtk/tests/TestInspector.cpp:352 > > + CustomInspectorTest::add("WebKitWebInspector", "inspector-window-destroyed", testInspectorWindowDestroyed); > > Perhaps "containing-window-destroyed" ? containing window might be the inspected window when the inspector is attached, inspector window always refers to the external window that contains the inspector. This is testing the case when the external custom window, which not known by wk, is destroyed.
(In reply to comment #4) > > > Source/WebKit2/UIProcess/API/gtk/tests/TestInspector.cpp:352 > > > + CustomInspectorTest::add("WebKitWebInspector", "inspector-window-destroyed", testInspectorWindowDestroyed); > > > > Perhaps "containing-window-destroyed" ? > > containing window might be the inspected window when the inspector is attached, inspector window always refers to the external window that contains the inspector. This is testing the case when the external custom window, which not known by wk, is destroyed. "custom-container-destroyed" or something similar?
(In reply to comment #5) > (In reply to comment #4) > > > > > Source/WebKit2/UIProcess/API/gtk/tests/TestInspector.cpp:352 > > > > + CustomInspectorTest::add("WebKitWebInspector", "inspector-window-destroyed", testInspectorWindowDestroyed); > > > > > > Perhaps "containing-window-destroyed" ? > > > > containing window might be the inspected window when the inspector is attached, inspector window always refers to the external window that contains the inspector. This is testing the case when the external custom window, which not known by wk, is destroyed. > > "custom-container-destroyed" or something similar? yeah, that sounds better.
Committed r118146: <http://trac.webkit.org/changeset/118146>