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 on 2013-09-25

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 on 2013-09-25

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
1=== modified file 'debian/changelog'
2--- debian/changelog 2013-08-23 22:07:25 +0000
3+++ debian/changelog 2013-09-25 17:56:30 +0000
4@@ -1,3 +1,11 @@
5+indicator-session (12.10.5+13.10.20130823-0ubuntu2) UNRELEASED; urgency=low
6+
7+ * src/service.c: don't switch to greeter when locking screen, as that
8+ switches away from the user's audio, power preferences, etc.
9+ (LP: #1220201)
10+
11+ -- Marc Deslauriers <marc.deslauriers@ubuntu.com> Fri, 20 Sep 2013 14:28:32 -0400
12+
13 indicator-session (12.10.5+13.10.20130823-0ubuntu1) saucy; urgency=low
14
15 [ Charles Kerr ]
16
17=== modified file 'src/service.c'
18--- src/service.c 2013-08-23 17:37:31 +0000
19+++ src/service.c 2013-09-25 17:56:30 +0000
20@@ -464,16 +464,15 @@
21 item = g_menu_item_new (ellipsis ? _("Switch Account…")
22 : _("Switch Account"), action);
23 }
24- else if (g_hash_table_size (p->users) == 1)
25- {
26- const char * action = "indicator.switch-to-greeter";
27- item = g_menu_item_new (_("Lock"), action);
28- }
29 else
30 {
31- const char * action = "indicator.switch-to-greeter";
32- item = g_menu_item_new (ellipsis ? _("Lock/Switch Account…")
33- : _("Lock/Switch Account"), action);
34+ const char * action = "indicator.switch-to-screensaver";
35+
36+ if (g_hash_table_size (p->users) == 1)
37+ item = g_menu_item_new (_("Lock"), action);
38+ else
39+ item = g_menu_item_new (ellipsis ? _("Lock/Switch Account…")
40+ : _("Lock/Switch Account"), action);
41 }
42 str = g_settings_get_string (p->keybinding_settings, "screensaver");
43 g_menu_item_set_attribute (item, "accel", "s", str);
44
45=== modified file 'tests/test-service.cc'
46--- tests/test-service.cc 2013-08-12 17:45:35 +0000
47+++ tests/test-service.cc 2013-09-25 17:56:30 +0000
48@@ -504,7 +504,7 @@
49 bool confirm = confirm_supported && !confirm_disabled;
50
51 // confirm that the ellipsis are correct
52- ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.switch-to-greeter"));
53+ ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.switch-to-screensaver"));
54 ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.logout"));
55 if (action_menuitem_exists ("indicator.reboot"))
56 ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.reboot"));
57@@ -538,7 +538,7 @@
58 bool confirm = confirm_supported && !confirm_disabled;
59
60 // confirm that the ellipsis are correct
61- ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.switch-to-greeter"));
62+ ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.switch-to-screensaver"));
63 ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.logout"));
64 if (action_menuitem_exists ("indicator.reboot"))
65 ASSERT_EQ (confirm, action_menuitem_label_is_ellipsized ("indicator.reboot"));
66@@ -570,7 +570,7 @@
67 ASSERT_TRUE (find_menu_item_for_action ("indicator.about", NULL, NULL));
68 ASSERT_TRUE (find_menu_item_for_action ("indicator.help", NULL, NULL));
69 ASSERT_TRUE (find_menu_item_for_action ("indicator.settings", NULL, NULL));
70- ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-greeter", NULL, NULL));
71+ ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-screensaver", NULL, NULL));
72 ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-guest", NULL, NULL));
73 ASSERT_TRUE (find_menu_item_for_action ("indicator.logout", NULL, NULL));
74 ASSERT_TRUE (find_menu_item_for_action ("indicator.suspend", NULL, NULL));
75@@ -681,10 +681,10 @@
76 };
77
78 // Find the switcher menu model.
79- // In BackendMock's default setup, it will only two menuitems: greeter & guest
80+ // In BackendMock's default setup, it will only have two menuitems: lockswitch & guest
81 int pos = 0;
82 GMenuModel * switch_menu = 0;
83- ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-greeter", &switch_menu, &pos));
84+ ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-screensaver", &switch_menu, &pos));
85 ASSERT_EQ (0, pos);
86 ASSERT_EQ (2, g_menu_model_get_n_items (switch_menu));
87 g_clear_object (&switch_menu);
88@@ -709,7 +709,7 @@
89 wait_for_menu_resync ();
90
91 // now there should be 7 menuitems: greeter + guest + the five doctors
92- ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-greeter", &switch_menu, &pos));
93+ ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-screensaver", &switch_menu, &pos));
94 ASSERT_EQ (0, pos);
95 ASSERT_EQ (7, g_menu_model_get_n_items (switch_menu));
96 // confirm that the doctor names are sorted
97@@ -727,7 +727,7 @@
98 wait_for_menu_resync ();
99
100 // now there should be 5 menuitems: greeter + guest + the three doctors
101- ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-greeter", &switch_menu, &pos));
102+ ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-screensaver", &switch_menu, &pos));
103 ASSERT_EQ (0, pos);
104 ASSERT_EQ (5, g_menu_model_get_n_items (switch_menu));
105 // confirm that the doctor names are sorted
106@@ -744,7 +744,7 @@
107 wait_for_menu_resync ();
108
109 // now there should be 5 menuitems: greeter + guest + the three doctors
110- ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-greeter", &switch_menu, &pos));
111+ ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-screensaver", &switch_menu, &pos));
112 ASSERT_EQ (0, pos);
113 ASSERT_EQ (5, g_menu_model_get_n_items (switch_menu));
114 g_clear_object (&switch_menu);
115@@ -770,7 +770,7 @@
116 g_object_get (service, "max-users", &max_users, NULL);
117 ASSERT_EQ (2, max_users);
118 wait_for_menu_resync ();
119- ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-greeter", &switch_menu, &pos));
120+ ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-screensaver", &switch_menu, &pos));
121 ASSERT_EQ (0, pos);
122 ASSERT_EQ (4, g_menu_model_get_n_items (switch_menu));
123 check_label ("First Doctor", switch_menu, 2);
124@@ -800,7 +800,7 @@
125 users[10]->is_logged_in = true;
126 indicator_session_users_changed (mock_users, users[10]->uid);
127 wait_for_menu_resync ();
128- ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-greeter", &switch_menu, &pos));
129+ ASSERT_TRUE (find_menu_item_for_action ("indicator.switch-to-screensaver", &switch_menu, &pos));
130 ASSERT_EQ (0, pos);
131 ASSERT_EQ (9, g_menu_model_get_n_items (switch_menu));
132 check_label ("Eleventh Doctor", switch_menu, 2);

Subscribers

People subscribed via source and target branches