Merge lp:~mdeslaur/indicator-session/lp1220201 into lp:indicator-session/13.10

Proposed by Marc Deslauriers
Status: Merged
Approved by: Charles Kerr
Approved revision: 414
Merged at revision: 412
Proposed branch: lp:~mdeslaur/indicator-session/lp1220201
Merge into: lp:indicator-session/13.10
Diff against target: 132 lines (+25/-18)
3 files modified
debian/changelog (+8/-0)
src/service.c (+7/-8)
tests/test-service.cc (+10/-10)
To merge this branch: bzr merge lp:~mdeslaur/indicator-session/lp1220201
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Charles Kerr (community) Approve
Marc Deslauriers Needs Resubmitting
Review via email: mp+186853@code.launchpad.net

Description of the change

  * src/service.c: don't switch to greeter when locking screen, as that
    switches away from the user's audio, power preferences, etc.
    (LP: #1220201)

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Charles Kerr (charlesk) wrote :

Marc, this is a great point, but we should probably apply this when there's >1 user on the system as well.

What about merging this else clause and the next one this way:

$ bzr diff
=== modified file 'src/service.c'
--- src/service.c 2013-09-05 19:44:55 +0000
+++ src/service.c 2013-09-25 14:49:19 +0000
@@ -463,16 +463,15 @@
       item = g_menu_item_new (ellipsis ? _("Switch Account…")
                                        : _("Switch Account"), action);
     }
- else if (g_hash_table_size (p->users) == 1)
- {
- const char * action = "indicator.switch-to-greeter";
- item = g_menu_item_new (_("Lock"), action);
- }
   else
     {
- const char * action = "indicator.switch-to-greeter";
- item = g_menu_item_new (ellipsis ? _("Lock/Switch Account…")
- : _("Lock/Switch Account"), action);
+ const char * action = "indicator.switch-to-screensaver";
+
+ if (g_hash_table_size (p->users) == 1)
+ item = g_menu_item_new (_("Lock"), action);
+ else
+ item = g_menu_item_new (ellipsis ? _("Lock/Switch Account…")
+ : _("Lock/Switch Account"), action);
     }
   str = g_settings_get_string (p->keybinding_settings, "screensaver");
   g_menu_item_set_attribute (item, "accel", "s", str);

413. By Marc Deslauriers

src/service.c: also switch to screensaver if there is more than one user

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

Ah, yes, I didn't notice the multiuser case. Updated, thanks.

review: Needs Resubmitting
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
414. By Marc Deslauriers

tests/test-service.cc: adjust test suite for screen lock changes

Revision history for this message
Marc Deslauriers (mdeslaur) :
review: Needs Resubmitting
Revision history for this message
Charles Kerr (charlesk) wrote :

Thanks Marc!

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/changelog'
--- debian/changelog 2013-08-23 22:07:25 +0000
+++ debian/changelog 2013-09-25 17:56:30 +0000
@@ -1,3 +1,11 @@
1indicator-session (12.10.5+13.10.20130823-0ubuntu2) UNRELEASED; urgency=low
2
3 * src/service.c: don't switch to greeter when locking screen, as that
4 switches away from the user's audio, power preferences, etc.
5 (LP: #1220201)
6
7 -- Marc Deslauriers <marc.deslauriers@ubuntu.com> Fri, 20 Sep 2013 14:28:32 -0400
8
1indicator-session (12.10.5+13.10.20130823-0ubuntu1) saucy; urgency=low9indicator-session (12.10.5+13.10.20130823-0ubuntu1) saucy; urgency=low
210
3 [ Charles Kerr ]11 [ Charles Kerr ]
412
=== modified file 'src/service.c'
--- src/service.c 2013-08-23 17:37:31 +0000
+++ src/service.c 2013-09-25 17:56:30 +0000
@@ -464,16 +464,15 @@
464 item = g_menu_item_new (ellipsis ? _("Switch Account…")464 item = g_menu_item_new (ellipsis ? _("Switch Account…")
465 : _("Switch Account"), action);465 : _("Switch Account"), action);
466 }466 }
467 else if (g_hash_table_size (p->users) == 1)
468 {
469 const char * action = "indicator.switch-to-greeter";
470 item = g_menu_item_new (_("Lock"), action);
471 }
472 else467 else
473 {468 {
474 const char * action = "indicator.switch-to-greeter";469 const char * action = "indicator.switch-to-screensaver";
475 item = g_menu_item_new (ellipsis ? _("Lock/Switch Account…")470
476 : _("Lock/Switch Account"), action);471 if (g_hash_table_size (p->users) == 1)
472 item = g_menu_item_new (_("Lock"), action);
473 else
474 item = g_menu_item_new (ellipsis ? _("Lock/Switch Account…")
475 : _("Lock/Switch Account"), action);
477 }476 }
478 str = g_settings_get_string (p->keybinding_settings, "screensaver");477 str = g_settings_get_string (p->keybinding_settings, "screensaver");
479 g_menu_item_set_attribute (item, "accel", "s", str);478 g_menu_item_set_attribute (item, "accel", "s", str);
480479
=== modified file 'tests/test-service.cc'
--- tests/test-service.cc 2013-08-12 17:45:35 +0000
+++ tests/test-service.cc 2013-09-25 17:56:30 +0000
@@ -504,7 +504,7 @@
504 bool confirm = confirm_supported && !confirm_disabled;504 bool confirm = confirm_supported && !confirm_disabled;
505505
506 // confirm that the ellipsis are correct506 // confirm that the ellipsis are correct
507 ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.switch-to-greeter"));507 ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.switch-to-screensaver"));
508 ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.logout"));508 ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.logout"));
509 if (action_menuitem_exists ("indicator.reboot"))509 if (action_menuitem_exists ("indicator.reboot"))
510 ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.reboot"));510 ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.reboot"));
@@ -538,7 +538,7 @@
538 bool confirm = confirm_supported && !confirm_disabled;538 bool confirm = confirm_supported && !confirm_disabled;
539539
540 // confirm that the ellipsis are correct540 // confirm that the ellipsis are correct
541 ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.switch-to-greeter"));541 ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.switch-to-screensaver"));
542 ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.logout"));542 ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.logout"));
543 if (action_menuitem_exists ("indicator.reboot"))543 if (action_menuitem_exists ("indicator.reboot"))
544 ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.reboot"));544 ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.reboot"));
@@ -570,7 +570,7 @@
570 ASSERT_TRUE (find_menu_item_for_action ("indicator.about", NULL, NULL));570 ASSERT_TRUE (find_menu_item_for_action ("indicator.about", NULL, NULL));
571 ASSERT_TRUE (find_menu_item_for_action ("indicator.help", NULL, NULL));571 ASSERT_TRUE (find_menu_item_for_action ("indicator.help", NULL, NULL));
572 ASSERT_TRUE (find_menu_item_for_action ("indicator.settings", NULL, NULL));572 ASSERT_TRUE (find_menu_item_for_action ("indicator.settings", NULL, NULL));
573 ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-greeter", NULL, NULL));573 ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-screensaver", NULL, NULL));
574 ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-guest", NULL, NULL));574 ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-guest", NULL, NULL));
575 ASSERT_TRUE (find_menu_item_for_action ("indicator.logout", NULL, NULL));575 ASSERT_TRUE (find_menu_item_for_action ("indicator.logout", NULL, NULL));
576 ASSERT_TRUE (find_menu_item_for_action ("indicator.suspend", NULL, NULL));576 ASSERT_TRUE (find_menu_item_for_action ("indicator.suspend", NULL, NULL));
@@ -681,10 +681,10 @@
681 };681 };
682682
683 // Find the switcher menu model.683 // Find the switcher menu model.
684 // In BackendMock's default setup, it will only two menuitems: greeter & guest684 // In BackendMock's default setup, it will only have two menuitems: lockswitch & guest
685 int pos = 0;685 int pos = 0;
686 GMenuModel * switch_menu = 0;686 GMenuModel * switch_menu = 0;
687 ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-greeter", &switch_menu, &pos));687 ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-screensaver", &switch_menu, &pos));
688 ASSERT_EQ (0, pos);688 ASSERT_EQ (0, pos);
689 ASSERT_EQ (2, g_menu_model_get_n_items (switch_menu));689 ASSERT_EQ (2, g_menu_model_get_n_items (switch_menu));
690 g_clear_object (&switch_menu);690 g_clear_object (&switch_menu);
@@ -709,7 +709,7 @@
709 wait_for_menu_resync ();709 wait_for_menu_resync ();
710710
711 // now there should be 7 menuitems: greeter + guest + the five doctors711 // now there should be 7 menuitems: greeter + guest + the five doctors
712 ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-greeter", &switch_menu, &pos));712 ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-screensaver", &switch_menu, &pos));
713 ASSERT_EQ (0, pos);713 ASSERT_EQ (0, pos);
714 ASSERT_EQ (7, g_menu_model_get_n_items (switch_menu));714 ASSERT_EQ (7, g_menu_model_get_n_items (switch_menu));
715 // confirm that the doctor names are sorted715 // confirm that the doctor names are sorted
@@ -727,7 +727,7 @@
727 wait_for_menu_resync ();727 wait_for_menu_resync ();
728728
729 // now there should be 5 menuitems: greeter + guest + the three doctors729 // now there should be 5 menuitems: greeter + guest + the three doctors
730 ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-greeter", &switch_menu, &pos));730 ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-screensaver", &switch_menu, &pos));
731 ASSERT_EQ (0, pos);731 ASSERT_EQ (0, pos);
732 ASSERT_EQ (5, g_menu_model_get_n_items (switch_menu));732 ASSERT_EQ (5, g_menu_model_get_n_items (switch_menu));
733 // confirm that the doctor names are sorted733 // confirm that the doctor names are sorted
@@ -744,7 +744,7 @@
744 wait_for_menu_resync ();744 wait_for_menu_resync ();
745745
746 // now there should be 5 menuitems: greeter + guest + the three doctors746 // now there should be 5 menuitems: greeter + guest + the three doctors
747 ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-greeter", &switch_menu, &pos));747 ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-screensaver", &switch_menu, &pos));
748 ASSERT_EQ (0, pos);748 ASSERT_EQ (0, pos);
749 ASSERT_EQ (5, g_menu_model_get_n_items (switch_menu));749 ASSERT_EQ (5, g_menu_model_get_n_items (switch_menu));
750 g_clear_object (&switch_menu);750 g_clear_object (&switch_menu);
@@ -770,7 +770,7 @@
770 g_object_get (service, "max-users", &max_users, NULL);770 g_object_get (service, "max-users", &max_users, NULL);
771 ASSERT_EQ (2, max_users);771 ASSERT_EQ (2, max_users);
772 wait_for_menu_resync ();772 wait_for_menu_resync ();
773 ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-greeter", &switch_menu, &pos));773 ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-screensaver", &switch_menu, &pos));
774 ASSERT_EQ (0, pos);774 ASSERT_EQ (0, pos);
775 ASSERT_EQ (4, g_menu_model_get_n_items (switch_menu));775 ASSERT_EQ (4, g_menu_model_get_n_items (switch_menu));
776 check_label ("First Doctor", switch_menu, 2);776 check_label ("First Doctor", switch_menu, 2);
@@ -800,7 +800,7 @@
800 users[10]->is_logged_in = true;800 users[10]->is_logged_in = true;
801 indicator_session_users_changed (mock_users, users[10]->uid);801 indicator_session_users_changed (mock_users, users[10]->uid);
802 wait_for_menu_resync ();802 wait_for_menu_resync ();
803 ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-greeter", &switch_menu, &pos));803 ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-screensaver", &switch_menu, &pos));
804 ASSERT_EQ (0, pos);804 ASSERT_EQ (0, pos);
805 ASSERT_EQ (9, g_menu_model_get_n_items (switch_menu));805 ASSERT_EQ (9, g_menu_model_get_n_items (switch_menu));
806 check_label ("Eleventh Doctor", switch_menu, 2);806 check_label ("Eleventh Doctor", switch_menu, 2);

Subscribers

People subscribed via source and target branches