Merge lp:~robert-ancell/indicator-session/blank-username into lp:indicator-session/14.04

[Robert Ancell]

Use username as label if the real name is empty or whitespace

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:425
http://jenkins.qa.ubuntu.com/job/indicator-session-ci/114/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-session-trusty-amd64-ci/10
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-session-trusty-armhf-ci/10
        deb: http://jenkins.qa.ubuntu.com/job/indicator-session-trusty-armhf-ci/10/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-session-ci/114/rebuild

review: Approve (continuous-integration)

[Ted Gould (ted)]

What about using g_unichar_isspace() since we expect the real name to be
a UTF8 string?

[Robert Ancell (robert-ancell)]

I had a long look at what it takes to parse it as UTF-8, I'm not sure if it's worth the effort.
- We really can't be sure it is valid UTF-8 so we have to validate it at the top of the function
- We have to handle invalid UTF8 cases which makes the code more complex
- This only helps if there are only some obscure non-printable UTF8 characters used - it's completely valid to check for the ASCII characters as all the UTF8 will be ignored

So I'd say "don't bother", what do you think?

[Ted Gould (ted)]

I guess my concern was that the UTF-8 code would be a multibyte code, where one of those bytes is a space. So we'd improperly detect a space when really it was a valid visible UTF-8 character. But since what we're detecting is all spaces, that would require both bytes to be valid ASCII spaces, and the name to be a single multi-byte character in length. Which seems pretty rare.

review: Approve

=== modified file 'src/service.c'
--- src/service.c	2013-11-08 17:41:36 +0000
+++ src/service.c	2013-12-19 03:06:21 +0000
@@ -274,6 +274,19 @@
 }
 
 static const char *
+get_user_label (const IndicatorSessionUser * user)
+{
+  const char * c;
+
+  /* If blank or whitespace, use username instead */
+  for (c = user->real_name; *c != '\0' && g_ascii_isspace (*c); c++);
+  if (*c == '\0')
+    return user->user_name;
+
+  return user->real_name;
+}
+
+static const char *
 get_current_real_name (IndicatorSessionService * self)
 {
   GHashTableIter iter;
@@ -289,7 +302,7 @@
     {
       IndicatorSessionUser * user = value;
       if (user->is_current_user)
-        return user->real_name;
+        return get_user_label (user);
     }
 
   return "";
@@ -435,7 +448,7 @@
   const IndicatorSessionUser * a = *(const IndicatorSessionUser**)ga;
   const IndicatorSessionUser * b = *(const IndicatorSessionUser**)gb;
 
-  if ((i = g_strcmp0 (a->real_name, b->real_name)))
+  if ((i = g_strcmp0 (get_user_label (a), get_user_label (b))))
     return i;
 
   return g_strcmp0 (a->user_name, b->user_name);
@@ -536,7 +549,7 @@
       const IndicatorSessionUser * u = g_ptr_array_index (users, i);
       GVariant * serialized_icon;
 
-      item = g_menu_item_new (u->real_name, NULL);
+      item = g_menu_item_new (get_user_label (u), NULL);
       g_menu_item_set_action_and_target (item, "indicator.switch-to-user", "s", u->user_name);
       g_menu_item_set_attribute (item, "x-canonical-type", "s", "indicator.user-menu-item");
 

=== modified file 'tests/test-service.cc'
--- tests/test-service.cc	2013-11-08 19:06:20 +0000
+++ tests/test-service.cc	2013-12-19 03:06:21 +0000
@@ -819,3 +819,33 @@
   wait_for_signal (mock_settings, "changed::last-command");
   check_last_command_is ("switch-to-user::tbaker");
 }
+
+TEST_F (ServiceTest, UserLabels)
+{
+  int pos = 0;
+  GMenuModel * switch_menu = 0;
+
+  // Check label uses username when real name is blank
+  IndicatorSessionUser * u = g_new0 (IndicatorSessionUser, 1);
+  u->uid = 100;
+  u->user_name = g_strdup ("blank");
+  u->real_name = g_strdup ("");
+  indicator_session_users_mock_add_user (INDICATOR_SESSION_USERS_MOCK(mock_users), u);
+  wait_for_menu_resync ();
+  ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-screensaver", &switch_menu, &pos));
+  check_label ("blank", switch_menu, 2);
+  g_clear_object (&switch_menu);
+  indicator_session_users_mock_remove_user (INDICATOR_SESSION_USERS_MOCK(mock_users), 100);
+
+  // Check label uses username when real name is all whitespace
+  u = g_new0 (IndicatorSessionUser, 1);
+  u->uid = 100;
+  u->user_name = g_strdup ("whitespace");
+  u->real_name = g_strdup (" ");
+  indicator_session_users_mock_add_user (INDICATOR_SESSION_USERS_MOCK(mock_users), u);
+  wait_for_menu_resync ();
+  ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-screensaver", &switch_menu, &pos));
+  check_label ("whitespace", switch_menu, 2);
+  g_clear_object (&switch_menu);
+  indicator_session_users_mock_remove_user (INDICATOR_SESSION_USERS_MOCK(mock_users), 100);
+}