[webkit-reviews] review granted: [Bug 103644] [GTK] Split GtkAuthenticationDialog in two widgets : [Attachment 189975] Updated patch

[bugzilla-daemon at webkit.org]

Xan Lopez <xan.lopez at gmail.com> has granted Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 103644: [GTK] Split GtkAuthenticationDialog in two widgets
https://bugs.webkit.org/show_bug.cgi?id=103644

Attachment 189975: Updated patch
https://bugs.webkit.org/attachment.cgi?id=189975&action=review

------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
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.