Bug 86823

Summary: [GTK] Allow to attach/detach the inspector in WebKit2
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: gustavo, mrobinson, webkit.review.bot
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Patch gustavo: review+

Description Carlos Garcia Campos 2012-05-18 01:31:21 PDT
Currently attach/detach is not implemented, we always create a new window for the inspector, and the attach button does nothing.
Comment 1 Carlos Garcia Campos 2012-05-18 01:57:42 PDT
Created attachment 142667 [details]
Patch
Comment 2 WebKit Review Bot 2012-05-18 02:01:50 PDT
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 3 Gustavo Noronha (kov) 2012-05-18 05:49:08 PDT
Comment on attachment 142667 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=142667&action=review

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:438
> -    return FALSE;
> +    return TRUE;

These look like a somewhat unrelated fix. In the interest of better bisectability, can you land these three in a separate commit? rs=me on doing that

> Source/WebKit2/UIProcess/gtk/WebInspectorProxyGtk.cpp:156
> +    // This method is called to decide whether to attach the inspector or not depending on whether
> +    // the inspector view fits into the window. Attach is implemented by the client, so return 0
> +    // when the client doesn't implement attach, to make sure the inspector is never attached.

I don't understand this comment, who would return 0 here?
Comment 4 Carlos Garcia Campos 2012-05-18 06:01:00 PDT
(In reply to comment #3)
> (From update of attachment 142667 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=142667&action=review
> 
> > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:438
> > -    return FALSE;
> > +    return TRUE;
> 
> These look like a somewhat unrelated fix. In the interest of better bisectability, can you land these three in a separate commit? rs=me on doing that

It's actually related, before this patch, it doesn't really matter whether we return TRUE or FALSE, because the even has been handled by the view. Now, if you click on the inspector view, and the inspector propagates the event, it gets to the inspected web view. This is specially noticeable when scrolling in the inspector and the inspected view is scrolled too at the same time. In any case, I can split this if you want.

> > Source/WebKit2/UIProcess/gtk/WebInspectorProxyGtk.cpp:156
> > +    // This method is called to decide whether to attach the inspector or not depending on whether
> > +    // the inspector view fits into the window. Attach is implemented by the client, so return 0
> > +    // when the client doesn't implement attach, to make sure the inspector is never attached.
> 
> I don't understand this comment, who would return 0 here?

oops, that comment doesn't make any sense, I forgot to remove it, it's part of another patch that I discarded.
Comment 5 Carlos Garcia Campos 2012-05-18 08:20:42 PDT
Committed r117595: <http://trac.webkit.org/changeset/117595>