Merge lp:~robert-ancell/unity-greeter/bug-919298 into lp:unity-greeter

[Robert Ancell]

Makes sure that the manual entry is shown if there are no user accounts listed. Adds some test behaviour for manipulating the user list - Ctrl++ to add, Ctrl+- to remove, Ctrl+0 to remove all, Ctrl+= to add all. This shows some rendering errors when users are added/removed.

[Robert Ancell (robert-ancell)]

Note I also changed the ordering of the user list to be sorted (otherwise it gets a bit random adding/removing) and changed the name of the long name test users to be clearer.

[Michael Terry (mterry)]

Sure, seems fine. I can't seem to login with the manual entry in test mode (I enter my name, the label changes to my name, but the prompt for password never comes, so I can't enter a password in the entry). Is that expected?

Aside from that though, approved.

review: Needs Fixing

[Robert Ancell (robert-ancell)]

That's been broken since I made the patch before this one. I just checked and it doesn't work in revisions, 327, 320, 305 so I think something must have broken in GTK+? I chased down the focus calls and we are calling focus to the entry but it appears it's not getting it for some reason.

[Robert Ancell (robert-ancell)]

The simpler test is to enter an invalid password and the when it retries we've lost focus.

=== modified file 'NEWS'
--- NEWS	2012-02-23 20:26:29 +0000
+++ NEWS	2012-03-07 04:56:18 +0000
@@ -1,6 +1,7 @@
 Overview of changes in unity-greeter 0.2.5
 
     * Don't allow dragging the window by the top bar
+    * Show manual entry when no users available
 
 Overview of changes in unity-greeter 0.2.4
 

=== modified file 'src/fixes.vapi'
--- src/fixes.vapi	2012-02-09 00:42:44 +0000
+++ src/fixes.vapi	2012-03-07 04:56:18 +0000
@@ -25,8 +25,14 @@
     public const int KEY_KP_Up;
     public const int KEY_KP_Down;
     public const int KEY_F10;
+    public const int KEY_0;
+    public const int KEY_g;
+    public const int KEY_G;
     public const int KEY_s;
     public const int KEY_h;
+    public const int KEY_plus;
+    public const int KEY_minus;
+    public const int KEY_equal;
     public const int CONTROL_MASK;
 }
 

=== modified file 'src/unity-greeter.vala'
--- src/unity-greeter.vala	2012-03-07 02:41:53 +0000
+++ src/unity-greeter.vala	2012-03-07 04:56:18 +0000
@@ -52,6 +52,31 @@
     /* User to authenticate against */
     private string ?authenticate_user = null;
 
+    private const TestEntry[] test_entries =
+    {
+        { "has-password",       "Has Password",      "*",              "uk;us", false, false, null },
+        { "different-prompt",   "Different Prompt",  "*",              "uk;us", false, false, null },
+        { "no-password",        "No Password",       "*",              "uk;us", false, false, null },
+        { "change-password",    "Change Password",   "*",              "uk;us", false, false, null },
+        { "two-factor",         "Two Factor",        "*",              "uk;us", false, false, null },
+        { "info-prompt",        "Info Prompt",       "*",              "uk;us", false, false, null },
+        { "long-info-prompt",   "Long Info Prompt",  "*",              "uk;us", false, false, null },
+        { "multi-info-prompt",  "Multi Info Prompt", "*",              "uk;us", false, false, null },
+        { "very-very-long-name",    "Long name (far too long to fit)", "*", "uk;us", false, false, null },
+        { "long-name-and-messages", "Long name and messages",          "*", "uk;us", false, true,  null },
+        { "active",             "Active Account",    "*",              "uk;us", true,  false, null },
+        { "has-messages",       "Has Messages",      "*",              "uk;us", false, true,  null },
+        { "gnome",              "GNOME",             "*",              "uk;us", false, false, "gnome-shell" },
+        { "locked",             "Locked Account",    "*",              "uk;us", false, false, null },
+        { "color-background",   "Color Background",  "#dd4814",        "uk;us", false, false, null },
+        { "white-background",   "White Background",  "#ffffff",        "uk;us", false, false, null },
+        { "black-background",   "Black Background",  "#000000",        "uk;us", false, false, null },
+        { "no-background",      "No Background",     null,             "uk;us", false, false, null },
+        { "unicode",            "가나다라마",        "*",              "uk;us", false, false, null },
+        { "",                   "",                  null,             null,    false, false, null }
+    };
+    private List<string> test_backgrounds;
+    private int n_test_entries = 0;
     private string? test_username = null;
     private bool test_prompted_sso = false;
     private bool test_request_new_password = false;
@@ -109,7 +134,7 @@
 
         if (test_mode)
         {
-            var backgrounds = new List<string> ();
+            test_backgrounds = new List<string> ();
             try
             {
                 var dir = Dir.open ("/usr/share/backgrounds/");
@@ -118,45 +143,26 @@
                     var bg = dir.read_name ();
                     if (bg == null)
                         break;
-                    backgrounds.append ("/usr/share/backgrounds/" + bg);
+                    test_backgrounds.append ("/usr/share/backgrounds/" + bg);
                 }
             }
             catch (FileError e)
             {
             }
-            var i = 0;
+
+            while (add_test_entry ());
+            user_list.add_entry ("*guest", _("Guest Session"));
             
-            var layout_list = make_layout_list ("uk;us");
-
-            user_list.add_entry ("has-password", "Has Password", next_background (backgrounds, ref i), layout_list);
-            user_list.add_entry ("different-prompt", "Different Prompt", next_background (backgrounds, ref i), layout_list);
-            user_list.add_entry ("no-password", "No Password", next_background (backgrounds, ref i), layout_list);
-            user_list.add_entry ("change-password", "Change Password", next_background (backgrounds, ref i), layout_list);
-            user_list.add_entry ("two-factor", "Two Factor", next_background (backgrounds, ref i), layout_list);
-            user_list.add_entry ("info-prompt", "Info Prompt", next_background (backgrounds, ref i), layout_list);
-            user_list.add_entry ("long-info-prompt", "Long Info Prompt", next_background (backgrounds, ref i), layout_list);
-            user_list.add_entry ("multi-info-prompt", "Multi Info Prompt", next_background (backgrounds, ref i), layout_list);
-            user_list.add_entry ("very-very-long-name", "User with a very very very long name", next_background (backgrounds, ref i), layout_list);
-            user_list.add_entry ("long-name-and-messages", "User with a long name and messages", next_background (backgrounds, ref i), layout_list, false, true);
-            user_list.add_entry ("active", "Active Account", next_background (backgrounds, ref i), layout_list, true);
-            user_list.add_entry ("has-messages", "Has Messages", next_background (backgrounds, ref i), layout_list, false, true);
-            user_list.add_entry ("gnome", "GNOME", next_background (backgrounds, ref i), layout_list, false, false, "gnome-shell");
-            user_list.add_entry ("locked", "Locked Account", next_background (backgrounds, ref i), layout_list);
-            user_list.add_entry ("color-background", "Color Background", "#dd4814", layout_list);
-            user_list.add_entry ("white-background", "White Background", "#ffffff", layout_list);
-            user_list.add_entry ("black-background", "Black Background", "#000000", layout_list);
-            user_list.add_entry ("no-background", "No Background", null, layout_list);
-            user_list.add_entry ("unicode", "가나다라마", next_background (backgrounds, ref i), layout_list);
-            user_list.add_entry ("*guest", _("Guest Session"), null, null, true);
+            main_window.key_press_event.connect (key_press_cb);
 
             if (last_user != null)
                 user_list.set_active_entry (last_user);
         }
         else
         {
-            if (greeter.hide_users_hint)
-                user_list.add_entry ("*other", _("Login"), null, null, false, false, greeter.default_session_hint);
-            else
+            user_list.default_session = greeter.default_session_hint;
+
+            if (!greeter.hide_users_hint)
             {
                 var users = LightDM.UserList.get_instance ();
                 users.user_added.connect (user_added_cb);
@@ -169,7 +175,7 @@
             if (greeter.has_guest_account_hint)
             {
                 debug ("Adding guest account entry");
-                user_list.add_entry ("*guest", _("Guest Session"), null, null, false, false, greeter.default_session_hint);
+                user_list.add_entry ("*guest", _("Guest Session"));
             }
 
             if (greeter.select_user_hint != null)
@@ -187,17 +193,66 @@
         Gdk.threads_add_idle (ready_cb); 
     }
     
-    private string? next_background (List<string> backgrounds, ref int index)
-    {
-        if (backgrounds == null)
-            return null;
-
-        if (index >= backgrounds.length ())
-            index = 0;
-        var background = backgrounds.nth_data (index);
-        index++;
-        
-        return background;
+    private bool key_press_cb (Gdk.EventKey event)
+    {
+        if ((event.state & Gdk.CONTROL_MASK) == 0)
+            return false;
+
+        if (event.keyval == Gdk.KEY_plus)
+            add_test_entry ();
+        if (event.keyval == Gdk.KEY_minus)
+            remove_test_entry ();
+        if (event.keyval == Gdk.KEY_0)
+        {
+            while (remove_test_entry ());
+            user_list.remove_entry ("*guest");
+        }
+        if (event.keyval == Gdk.KEY_equal)
+        {
+            while (add_test_entry ());
+            user_list.add_entry ("*guest", _("Guest Session"));
+        }
+        if (event.keyval == Gdk.KEY_g)
+            user_list.remove_entry ("*guest");
+        if (event.keyval == Gdk.KEY_G)
+            user_list.add_entry ("*guest", _("Guest Session"));
+
+        return false;
+    }
+
+    private bool add_test_entry ()
+    {
+        var e = test_entries[n_test_entries];
+        if (e.username == "")
+            return false;
+
+        var background = e.background;
+        if (background == "*")
+        {
+            var background_index = 0;
+            for (var i = 0; i < n_test_entries; i++)
+            {
+                if (test_entries[i].background == "*")
+                    background_index++;
+            }
+            if (test_backgrounds.length () > 0)
+                background = test_backgrounds.nth_data (background_index % test_backgrounds.length ());
+        }
+        user_list.add_entry (e.username, e.real_name, background, make_layout_list (e.layouts), e.is_active, e.has_messages, e.session);
+        n_test_entries++;
+
+        return true;
+    }
+
+    private bool remove_test_entry ()
+    {
+        if (n_test_entries == 0)
+            return false;
+
+        user_list.remove_entry (test_entries[n_test_entries - 1].username);
+        n_test_entries--;
+
+        return true;
     }
 
     public static void add_style_class (Gtk.Widget widget)
@@ -255,10 +310,6 @@
         if (user.real_name == "")
             label = user.name;
 
-        var session = user.session;
-        if (session == null)
-            session = greeter.default_session_hint;
-
         var layouts = new List <LightDM.Layout> ();
         foreach (var name in user.get_layouts ())
         {
@@ -267,7 +318,7 @@
                 layouts.append (layout);
         }
 
-        user_list.add_entry (user.name, label, user.background, layouts, user.logged_in, user.has_messages, session);
+        user_list.add_entry (user.name, label, user.background, layouts, user.logged_in, user.has_messages, user.session);
     }
 
     private void user_removed_cb (LightDM.User user)
@@ -290,7 +341,14 @@
 
     private void show_prompt_cb (string text, LightDM.PromptType type)
     {
-        update_other_label ();
+        /* Notify the greeter on what user has been logged */
+        if (user_list.selected == "*other" && user_list.manual_username == null)
+        {
+            if (test_mode)
+                user_list.manual_username = test_username;
+            else
+                user_list.manual_username = greeter.authentication_user;
+        }
 
         prompted = true;
         if (text == "Password: ")
@@ -329,8 +387,6 @@
 
     private void authentication_complete_cb ()
     {
-        update_other_label ();
-
         bool is_authenticated;
         if (test_mode)
             is_authenticated = test_is_authenticated;
@@ -389,30 +445,13 @@
         start_authentication ();
     }
 
-    private void update_other_label ()
-    {
-        if (!greeter.hide_users_hint)
-            return;
-
-        var text = _("Other...");
-        if (test_mode)
-        {
-            if (user_list.selected == "*other" && test_username != null)
-                text = test_username;
-        }
-        else
-        {
-            if (user_list.selected == "*other" && greeter.authentication_user != null)
-                text = greeter.authentication_user;
-        }
-
-        user_list.add_entry ("*other", text, null, null, false, false, greeter.default_session_hint);
-    }
-
     private void start_authentication ()
     {
         prompted = false;
 
+        /* Reset manual username */
+        user_list.manual_username = null;
+
         if (test_mode)
         {
             test_username = null;
@@ -472,17 +511,12 @@
         else
         {
             if (user_list.selected == "*other")
-            {
-                greeter.authenticate (authenticate_user);
-                authenticate_user = null;
-            }
+                greeter.authenticate ();
             else if (user_list.selected == "*guest")
                 greeter.authenticate_as_guest ();
             else
                 greeter.authenticate (user_list.selected);
         }
-
-        update_other_label ();
     }
 
     private void respond_to_prompt_cb (string text)
@@ -792,3 +826,14 @@
         return Posix.EXIT_SUCCESS;
     }
 }
+
+private struct TestEntry
+{
+    string username;
+    string real_name;
+    string? background;
+    string? layouts;
+    bool is_active;
+    bool has_messages;
+    string? session;
+}

=== modified file 'src/user-list.vala'
--- src/user-list.vala	2012-03-05 05:12:26 +0000
+++ src/user-list.vala	2012-03-07 04:56:18 +0000
@@ -121,8 +121,34 @@
     {
         get { if (selected_entry == null) return null; return selected_entry.name; }
     }
+    
+    private string? _manual_username = null;
+    public string? manual_username
+    { 
+        get { return _manual_username; }
+        set
+        {
+            _manual_username = value;
+            if (find_entry ("*other") != null)
+                add_manual_entry ();
+        }
+    }
+    
+    public string default_session = "ubuntu";
 
-    public string? session {get; private set;}
+    private string? _session = null;
+    public string? session
+    {
+        get
+        {
+            return _session;
+        }
+        set
+        {
+            _session = value;
+            session_image.set_from_pixbuf (get_badge ());
+        }
+    }
 
     private Gdk.Pixbuf? last_session_badge = null;
 
@@ -169,7 +195,7 @@
         session_button = new Gtk.Button ();
         session_button.focus_on_click = false;
         session_button.get_accessible ().set_name (_("Session Options"));
-        session_image = new Gtk.Image.from_pixbuf (SessionChooser.get_badge (session));
+        session_image = new Gtk.Image.from_pixbuf (get_badge ());
         session_image.show ();
         session_button.relief = Gtk.ReliefStyle.NONE;
         session_button.add (session_image);
@@ -333,31 +359,76 @@
         return null;
     }
 
-    public void add_entry (string name, string label, string? background, List <LightDM.Layout>? keyboard_layouts, bool is_active = false, bool has_messages = false, string? session = null)
+    public void add_entry (string name, string label, string? background = null, List <LightDM.Layout>? keyboard_layouts = null, bool is_active = false, bool has_messages = false, string? session = null)
     {
         var e = find_entry (name);
         if (e == null)
         {
             e = new UserEntry ();
-            entries.append (e);
             e.name = name;
         }
+        else
+            entries.remove (e);
         e.layout = create_pango_layout (label);
         e.layout.set_font_description (Pango.FontDescription.from_string ("Ubuntu 16"));
         e.background = background;
         e.keyboard_layouts = keyboard_layouts.copy ();
         e.is_active = is_active;
         e.has_messages = has_messages;
-        e.session = (session == null) ? "ubuntu" : session;
+        e.session = session;
+        entries.insert_sorted (e, compare_entry);
 
         if (selected_entry == null)
             select_entry (e, 1.0);
         else
             select_entry (selected_entry, 1.0);
 
+        /* Remove manual option when have users */
+        if (have_users ())
+            remove_entry ("*other");
+
         redraw_user_list ();
     }
 
+    private static int compare_entry (UserEntry a, UserEntry b)
+    {
+        if (a.name.has_prefix ("*") || b.name.has_prefix ("*"))
+        {
+           /* Special entries go after normal ones */
+           if (!a.name.has_prefix ("*"))
+               return -1;
+           if (!b.name.has_prefix ("*"))
+               return 1;
+
+           /* Manual comes before guest */
+           if (a.name == "*other")
+               return -1;
+           if (a.name == "*guest")
+               return 1;
+        }
+
+        /* Alphabetical by label */
+        return a.layout.get_text ().ascii_casecmp (b.layout.get_text ());
+    }
+    
+    private bool have_users ()
+    {
+        foreach (var e in entries)
+        {
+            if (!e.name.has_prefix ("*"))
+                return true;
+        }
+        return false;
+    }
+    
+    private void add_manual_entry ()
+    {
+        var text = manual_username;
+        if (text == null)
+            text = _("Login");
+        add_entry ("*other", text);
+    }
+
     public void set_active_entry (string ?name)
     {
         var e = find_entry (name);
@@ -374,6 +445,11 @@
         var index = entries.index (entry);
         entries.remove (entry);
 
+        /* Show a manual login if no users */
+        if (!have_users ())
+            add_manual_entry ();
+
+        /* Select previous entry if the selected one was removed */
         if (entry == selected_entry)
         {
             if (index >= entries.length ())
@@ -404,12 +480,6 @@
         scroll_timer.reset (AnimateTimer.INSTANT);
     }
 
-    private void change_session (string session)
-    {
-        this.session = session;
-        session_image.set_from_pixbuf (SessionChooser.get_badge (session));
-    }
-
     private void session_clicked_cb (string? session)
     {
         return_if_fail (mode == Mode.SESSIONS);
@@ -418,7 +488,7 @@
         session_chooser.fade_out ();
 
         if (session != null)
-            change_session (session);
+            this.session = session;
     }
 
     private void session_fade_done_cb ()
@@ -546,13 +616,13 @@
 
     private void select_entry (UserEntry entry, double direction)
     {
-        last_session_badge = SessionChooser.get_badge (session);
+        last_session_badge = get_badge ();
 
         if (!get_realized ())
         {
             /* Just note it for the future if we haven't been realized yet */
             selected_entry = entry;
-            change_session (entry.session);
+            session = entry.session;
             return;
         }
 
@@ -600,13 +670,21 @@
         if (selected_entry != entry)
         {
             selected_entry = entry;
-            change_session (entry.session);
+            session = entry.session;
             user_selected (selected_entry.name);
 
             if (mode == Mode.LOGIN)
                 user_displayed (); /* didn't need to move, make sure we trigger side effects */
         }
     }
+    
+    private Gdk.Pixbuf? get_badge ()
+    {
+        if (session == null)
+            return SessionChooser.get_badge (default_session);
+        else
+            return SessionChooser.get_badge (session);    
+    }
 
     public override void realize ()
     {
@@ -839,7 +917,7 @@
 
                 var badge = last_session_badge;
                 if (entry == selected_entry)
-                    badge = SessionChooser.get_badge (session);
+                    badge = get_badge ();
 
                 if (position <= 0) /* top of box, normal pace */
                     draw_entry_at_position (c, entry, position, true, badge);