[Webkit-unassigned] [Bug 128813] [GTK] Minibrowser: Add shortcut to open Web Inspector

[bugzilla-daemon at webkit.org]

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


Sergio Villar Senin <svillar at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #224210|review?                     |review-
               Flag|                            |




--- Comment #2 from Sergio Villar Senin <svillar at igalia.com>  2014-02-14 08:22:34 PST ---
(From update of attachment 224210)
View in context: https://bugs.webkit.org/attachment.cgi?id=224210&action=review

There are a couple of minor nits, but this is a r- because the logic is not correct.

> Tools/ChangeLog:8
> +        Added shortcut Ctrl+i for toggling Web Inspector.

Is this shortcut used by some other port? Is it used in GtkLauncher?

> Tools/MiniBrowser/gtk/BrowserWindow.c:503
> +static gboolean toggleWebInspector(GObject *gObject, gpointer user_data)

Don't use camel case for attributes in this file. Also I guess you can use here BrowserWindow directly instead of GObject as you're using G_CALLBACK().

> Tools/MiniBrowser/gtk/BrowserWindow.c:511
> +    height = webkit_web_inspector_get_attached_height(inspectorWindow);

If you're going to use the variable just once, better do not define it and directly use the return value from the function.

In any case I don't think the logic is correct. If the inspector is shown in a dettached mode, the height will be 0 so you won't close it.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.