[Webkit-unassigned] [Bug 136615] [GTK] Use a nicer HTTP authentication dialog

[bugzilla-daemon at webkit.org]

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


Carlos Garcia Campos <cgarcia at igalia.com> changed:

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




--- Comment #9 from Carlos Garcia Campos <cgarcia at igalia.com>  2014-09-10 01:31:57 PST ---
(From update of attachment 237810)
View in context: https://bugs.webkit.org/attachment.cgi?id=237810&action=review

> Source/WebCore/ChangeLog:8
> +        No new tests (OOPS!).

You should remove this line.

> Source/WebCore/platform/gtk/WebKitAuthenticationWidget.cpp:-151
> -    GtkWidget* icon = gtk_image_new_from_stock(GTK_STOCK_DIALOG_AUTHENTICATION, GTK_ICON_SIZE_DIALOG);
> -    gtk_misc_set_alignment(GTK_MISC(icon), 0.5, 0.0);
> -    gtk_box_pack_start(GTK_BOX(authWidget), icon, FALSE, FALSE, 0);
> -    gtk_widget_show(icon);

Why removing the dialog icon?

> Source/WebCore/platform/gtk/WebKitAuthenticationWidget.cpp:140
> +        /* Prompt on HTTP authentication dialog */

Use C++ comment, and finish it with period.

> Source/WebCore/platform/gtk/WebKitAuthenticationWidget.cpp:141
> +        _("Authentication required by %s:"),

Why removing the port?

> Source/WebCore/platform/gtk/WebKitAuthenticationWidget.cpp:144
> +    gtk_misc_set_alignment(GTK_MISC(label), 0, 0.5);

Can we take advantage to not use GtkMisc?

> Source/WebCore/platform/gtk/WebKitAuthenticationWidget.cpp:158
> +    /* Check button on the HTTP authentication dialog */

C++ comments finished with a period.

> Source/WebCore/platform/gtk/WebKitAuthenticationWidget.cpp:159
> +    priv->rememberCheckButton = gtk_check_button_new_with_mnemonic(_("_Remember password"));

Why removing the ':' ? Were these strings translated? Since the file was not listed in the POTFILES.in. If we already have translations of these, maybe we can try to improve the dialog without changing strings, since it's late in the release cycle.

> Source/WebCore/platform/gtk/WebKitAuthenticationWidget.cpp:162
> +    /* Entry on the HTTP authentication dialog */

C++ comments finished with a period.

> Source/WebCore/platform/gtk/WebKitAuthenticationWidget.cpp:163
> +    GtkWidget* username_label = gtk_label_new_with_mnemonic(_("_Username"));

username_label -> usernameLabel

> Source/WebCore/platform/gtk/WebKitAuthenticationWidget.cpp:164
> +    GtkWidget* username_entry = createEntry(&priv->loginEntry);

username_entry -> usernameEntry

> Source/WebCore/platform/gtk/WebKitAuthenticationWidget.cpp:167
> +    /* Entry on the HTTP authentication dialog */

C++ comments finished with a period.

> Source/WebCore/platform/gtk/WebKitAuthenticationWidget.cpp:168
> +    GtkWidget* password_label = gtk_label_new_with_mnemonic(_("_Password"));

password_label -> passwordLabel

> Source/WebCore/platform/gtk/WebKitAuthenticationWidget.cpp:169
> +    GtkWidget* password_entry = createEntry(&priv->passwordEntry);

password_entry -> passwordEntry

> Source/WebCore/platform/gtk/WebKitAuthenticationWidget.cpp:175
> +#if GTK_CHECK_VERSION(3, 0, 0)
> +    gtk_style_context_add_class(gtk_widget_get_style_context(username_label), "dim-label");
> +    gtk_style_context_add_class(gtk_widget_get_style_context(password_label), "dim-label");
> +#endif

What is this dim-label class for? We can remove the GTK+2 support here, maybe we can do it that first, and then this patch would be simpler.

> Source/WebKit2/UIProcess/API/gtk/WebKitAuthenticationDialog.cpp:71
> +    /* Title of the HTTP authentication dialog */

C++ comments finished with a period.

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