Bug 86823 – [GTK] Allow to attach/detach the inspector in WebKit2

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Carlos Garcia Campos]

Currently attach/detach is not implemented, we always create a new window for the inspector, and the attach button does nothing.

[Carlos Garcia Campos]

Created attachment 142667 [details]
Patch

[WebKit Review Bot]

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

[Gustavo Noronha (kov)]

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?

[Carlos Garcia Campos]

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

[Carlos Garcia Campos]

Committed r117595: <http://trac.webkit.org/changeset/117595>