[Webkit-unassigned] [Bug 103644] [GTK] Split GtkAuthenticationDialog in two widgets

[bugzilla-daemon at webkit.org]

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


Xan Lopez <xan.lopez at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #189975|review?                     |review+
               Flag|                            |




--- Comment #8 from Xan Lopez <xan.lopez at gmail.com>  2013-03-06 00:46:26 PST ---
(From update of attachment 189975)
View in context: https://bugs.webkit.org/attachment.cgi?id=189975&action=review

This looks great to me, I think a common core and two small widgets for wk1 and wk2 makes this more readable. I just have two or three very minor comments, but r+ as far as I'm concerned.

> Source/WebKit/gtk/webkit/webkitauthenticationdialog.h:21
> +

I think it's probably not needed here but I'd just add the G_BEGIN_DECLS stuff for consistency's sake.

> Source/WebKit/gtk/webkit/webkitsoupauthdialog.cpp:27
> +#include <wtf/text/CString.h>

This extra include seems odd?

> Source/autotools/Versions.m4:-18
> -m4_define([gtk2_required_version], [2.10])

If it's not a pain in the ass I'd just try to not bump this, since the gtk2 stuff is on its way out and asking people to bump it for a very minor feature seems odd. But no strong feelings about it.

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