[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


Michael Catanzaro <mcatanzaro at gnome.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends on|                            |136700




--- Comment #11 from Michael Catanzaro <mcatanzaro at gnome.org>  2014-09-10 06:11:22 PST ---
(In reply to comment #9)
> > 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?

It looks bad!

Also, I'm working from a mockup in https://bugzilla.gnome.org/show_bug.cgi?id=727149

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

It's a confusing technical detail. Internet Explorer and Firefox don't show the port, so I think we can omit it too. Safari and Chrome do (at least in the screenshots I checked).

> > 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?

After bug #136700 is fixed, we can use gtk_widget_set_halign instead. Here it had to be GtkMisc (or an ifdef) for compatibility with Gtk+ 2.

> > 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.

Yes, it was translated, and it actually is listed in POTFILES.in. How about I write an initial version of this patch that uses the old strings, then a separate patch to update the strings after we have branched?

> > 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.

It's supposed to make the text gray instead of black, like in the mockup, but there is some bug that causes this to happen only after the window loses focus. :/

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