Bug 113498

Summary: [GTK][WK2] Implement WebInspectorProxy::platformInspectedWindowWidth
Product: WebKit Reporter: Zan Dobersek <zan>
Component: New BugsAssignee: Zan Dobersek <zan>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cgarcia, gustavo, mrobinson, sam, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 113281    
Attachments:
Description Flags
Patch
none
Patch none

Description Zan Dobersek 2013-03-28 04:04:59 PDT
[GTK][WK2] Implement WebInspectorProxy::platformInspectedWindowWidth
Comment 1 Zan Dobersek 2013-03-28 04:09:55 PDT
Created attachment 195523 [details]
Patch
Comment 2 WebKit Review Bot 2013-03-28 04:16:02 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 Build Bot 2013-03-28 05:34:41 PDT
Comment on attachment 195523 [details]
Patch

Attachment 195523 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17330290
Comment 4 Carlos Garcia Campos 2013-03-28 05:37:26 PDT
Comment on attachment 195523 [details]
Patch

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

> Source/WebKit2/UIProcess/API/gtk/tests/TestInspector.cpp:128
> -        resizeView(300, (gMinimumAttachedInspectorHeight + 1) * 4 / 3);
> +        resizeView((gMinimumAttachedInspectorWidth + 1) * 4 / 3, (gMinimumAttachedInspectorHeight + 1) * 4 / 3);

Why 4/3 for width too? the inspector always uses the view width, we just want to make sure there's a minimum width, no?

> Source/WebKit2/UIProcess/gtk/WebInspectorProxyGtk.cpp:185
> -    notImplemented();
> -    return 0;
> +    GtkAllocation allocation;
> +    gtk_widget_get_allocation(m_page->viewWidget(), &allocation);
> +    return allocation.width;

This can be just gtk_widget_get_allocated_width(m_page->viewWidget());
Comment 5 Zan Dobersek 2013-03-28 07:44:58 PDT
Comment on attachment 195523 [details]
Patch

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

>> Source/WebKit2/UIProcess/API/gtk/tests/TestInspector.cpp:128
>> +        resizeView((gMinimumAttachedInspectorWidth + 1) * 4 / 3, (gMinimumAttachedInspectorHeight + 1) * 4 / 3);
> 
> Why 4/3 for width too? the inspector always uses the view width, we just want to make sure there's a minimum width, no?

Right, makes sense.

>> Source/WebKit2/UIProcess/gtk/WebInspectorProxyGtk.cpp:185
>> +    return allocation.width;
> 
> This can be just gtk_widget_get_allocated_width(m_page->viewWidget());

Will reimplement WebInspectorProxy::platformInspectedWindowHeight in a similar fashion.
Comment 6 Zan Dobersek 2013-03-28 08:15:28 PDT
Created attachment 195569 [details]
Patch
Comment 7 Carlos Garcia Campos 2013-04-08 11:58:34 PDT
Comment on attachment 195569 [details]
Patch

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

This looks good to me, thanks!

> Source/WebKit2/UIProcess/gtk/WebInspectorProxyGtk.cpp:176
> -    GtkAllocation allocation;
> -    gtk_widget_get_allocation(m_page->viewWidget(), &allocation);
> -    return allocation.height;
> +    return gtk_widget_get_allocated_height(m_page->viewWidget());

Thanks for the cleanup here.
Comment 8 Zan Dobersek 2013-04-09 03:21:41 PDT
CC-ing a couple of WK2 owners to possibly give a go-ahead for the patch.
Comment 9 Benjamin Poulain 2013-04-09 14:26:03 PDT
Comment on attachment 195569 [details]
Patch

Signed off by me.
Comment 10 Zan Dobersek 2013-04-10 00:36:31 PDT
Comment on attachment 195569 [details]
Patch

Clearing flags on attachment: 195569

Committed r148082: <http://trac.webkit.org/changeset/148082>
Comment 11 Zan Dobersek 2013-04-10 00:36:38 PDT
All reviewed patches have been landed.  Closing bug.