Merge lp:~dbarth/unity-greeter/remote-login-only into lp:unity-greeter

Proposed by David Barth
Status: Merged
Approved by: Michael Terry
Approved revision: 929
Merged at revision: 941
Proposed branch: lp:~dbarth/unity-greeter/remote-login-only
Merge into: lp:unity-greeter
Diff against target: 287 lines (+134/-35)
5 files modified
src/greeter-list.vala (+10/-6)
src/user-list.vala (+55/-24)
tests/test-list.vala (+1/-0)
tests/test.vala (+60/-1)
tests/unity-greeter.vala (+8/-4)
To merge this branch: bzr merge lp:~dbarth/unity-greeter/remote-login-only
Reviewer Review Type Date Requested Status
Michael Terry (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+178007@code.launchpad.net

Commit message

Let the greeter display only the remote login entry, without adding a fallback manual login option.

Description of the change

Let the greeter display only the remote login entry, without adding a fallback manual login option.

See the unit tests for the configuration details, ie:
greeter-show-manual-login=false
greeter-hide-users=true
greeter-allow-guest=false
greeter-show-remote-login=true

Note that if the remote login service becomes unavailable, the manual login entry will re-appear, and vice-versa.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
928. By David Barth

fix the test for gdk3 on saucy

929. By David Barth

indention fixes

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:929
http://jenkins.qa.ubuntu.com/job/unity-greeter-ci/3/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-greeter-saucy-amd64-ci/3

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-greeter-ci/3/rebuild

review: Approve (continuous-integration)
Revision history for this message
Michael Terry (mterry) wrote :

19 + else if (index < entries.length ())
20 + index++;

Why the change from selecting previous index to selecting next index after removing an entry?

162 +

This whitespace change in tests/test-list.vala can be dropped.

197 + /* don't go too fast, otherwise the lastest gdk3 will lose control... */
198 + Posix.sleep(1);

Ick. Did you try a call to process_events()? That might have the same effect, without an actual sleep.

211 + GLib.assert (list.num_entries() == 1);
212 + GLib.assert (list.selected_entry.id == "*remote_directory*http://crazyurl.com");

These lines (among others) seem differently indented than the surrounding area.

Besides those, seems fine and appropriate. Thanks!

review: Needs Fixing
Revision history for this message
David Barth (dbarth) wrote :

Le 02/08/2013 20:50, Michael Terry a écrit :
> Review: Needs Fixing
>
> 19 + else if (index < entries.length ())
> 20 + index++;
>
> Why the change from selecting previous index to selecting next index after removing an entry?
The comment explains it: we switch to either the previous line (by
default) if there exists a previous line, or if not, we change to the
next one. The code now works in cases where the line we remove is the
first line in the list, not just the last one as has been the case so far.

>
> 162 +
>
> This whitespace change in tests/test-list.vala can be dropped.
ok
>
> 197 + /* don't go too fast, otherwise the lastest gdk3 will lose control... */
> 198 + Posix.sleep(1);
>
> Ick. Did you try a call to process_events()? That might have the same effect, without an actual sleep.
I did, but gtk3/vala on saucy can get satisfied with just that :/
gtk3/vala on raring is fine without it... Otherwise, you get gtk3 to
complain about losing the last reference to an undeleted widget. I
assume this is a temporary regression in saucy.

> 211 + GLib.assert (list.num_entries() == 1);
> 212 + GLib.assert (list.selected_entry.id == "*remote_directory*http://crazyurl.com");
>
> These lines (among others) seem differently indented than the surrounding area.
I re-tabbed all of the lines I could see, hopefully for good now.

David

Revision history for this message
Michael Terry (mterry) wrote :

Sure, seems fine. Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/greeter-list.vala'
2--- src/greeter-list.vala 2013-07-08 11:35:39 +0000
3+++ src/greeter-list.vala 2013-08-01 15:02:04 +0000
4@@ -444,18 +444,22 @@
5 entry.destroy ();
6 entries.remove (entry);
7
8- /* Show a manual login if no users */
9- if (!have_entries ())
10- add_manual_entry ();
11-
12- /* Select previous entry if the selected one was removed */
13+ /* Select another entry if the selected one was removed */
14 if (entry == selected_entry)
15 {
16 if (index >= entries.length () && index > 0)
17 index--;
18- select_entry (entries.nth_data (index), -1.0);
19+ else if (index < entries.length ())
20+ index++;
21+
22+ if (entries.nth_data (index) != null)
23+ select_entry (entries.nth_data (index), -1.0);
24 }
25
26+ /* Show a manual login if no users and no remote login entry */
27+ if (!have_entries () && !UnityGreeter.singleton.show_remote_login_hint ())
28+ add_manual_entry ();
29+
30 queue_draw ();
31 }
32
33
34=== modified file 'src/user-list.vala'
35--- src/user-list.vala 2013-02-13 04:25:47 +0000
36+++ src/user-list.vala 2013-08-01 15:02:04 +0000
37@@ -124,8 +124,11 @@
38
39 fill_list ();
40
41- entry_selected.connect (entry_selected_cb);
42- entry_selected_cb (selected_entry.id);
43+ if (selected_entry != null)
44+ {
45+ entry_selected.connect (entry_selected_cb);
46+ entry_selected_cb (selected_entry.id);
47+ }
48
49 connect_to_lightdm ();
50
51@@ -150,6 +153,10 @@
52 {
53 remote_login_server_list = new List<RemoteServer?> ();
54 remove_entries_with_prefix ("*remote_login");
55+
56+ /* If we have no entries at all, we should show manual */
57+ if (!always_show_manual)
58+ add_manual_entry ();
59 }
60
61 private async void query_directory_servers ()
62@@ -249,6 +256,12 @@
63 it = it.next;
64 }
65 }
66+
67+ /* Remove manual option unless specified */
68+ if (remote_directory_server_list.length() > 0 && !always_show_manual) {
69+ debug ("removing manual login since we have a remote login entry");
70+ remove_entry ("*other");
71+ }
72 }
73
74 private PromptBox create_prompt_for_login_server (RemoteServer remote_server)
75@@ -341,6 +354,12 @@
76 {
77 remove_remote_servers ();
78 remote_login_service = null;
79+
80+ /* provide a fallback manual login option */
81+ if (UnityGreeter.singleton.hide_users_hint ()) {
82+ add_manual_entry();
83+ set_active_entry ("*other");
84+ }
85 }
86
87 private async void remote_directory_respond_cb ()
88@@ -1099,31 +1118,43 @@
89 {
90 }
91
92- while (add_test_entry ());
93- offer_guest = true;
94- always_show_manual = true;
95+ if (!UnityGreeter.singleton.hide_users_hint())
96+ while (add_test_entry ());
97+
98+ /* add a manual entry if the list of entries is empty initially */
99+ if (n_test_entries <= 0)
100+ {
101+ add_manual_entry();
102+ set_active_entry ("*other");
103+ n_test_entries++;
104+ }
105+
106+ offer_guest = UnityGreeter.singleton.has_guest_account_hint();
107+ always_show_manual = UnityGreeter.singleton.show_manual_login_hint();
108
109 key_press_event.connect (test_key_press_cb);
110
111- Timeout.add (1000, () =>
112- {
113- RemoteServer[] test_server_list = {};
114- RemoteServer remote_server = RemoteServer ();
115- remote_server.type = "uccs";
116- remote_server.name = "Remote Login";
117- remote_server.url = "http://crazyurl.com";
118- remote_server.last_used_server = false;
119- remote_server.fields = {};
120- RemoteServerField field1 = RemoteServerField ();
121- field1.type = "email";
122- RemoteServerField field2 = RemoteServerField ();
123- field2.type = "password";
124- remote_server.fields = {field1, field2};
125-
126- test_server_list += remote_server;
127- set_remote_directory_servers (test_server_list);
128- return false;
129- });
130+ if (UnityGreeter.singleton.show_remote_login_hint())
131+ Timeout.add (1000, () =>
132+ {
133+ RemoteServer[] test_server_list = {};
134+ RemoteServer remote_server = RemoteServer ();
135+ remote_server.type = "uccs";
136+ remote_server.name = "Remote Login";
137+ remote_server.url = "http://crazyurl.com";
138+ remote_server.last_used_server = false;
139+ remote_server.fields = {};
140+ RemoteServerField field1 = RemoteServerField ();
141+ field1.type = "email";
142+ RemoteServerField field2 = RemoteServerField ();
143+ field2.type = "password";
144+ remote_server.fields = {field1, field2};
145+
146+ test_server_list += remote_server;
147+ set_remote_directory_servers (test_server_list);
148+
149+ return false;
150+ });
151
152 var last_user = UnityGreeter.singleton.get_state ("last-user");
153 if (last_user != null)
154
155=== modified file 'tests/test-list.vala'
156--- tests/test-list.vala 2012-09-04 13:35:33 +0000
157+++ tests/test-list.vala 2013-08-01 15:02:04 +0000
158@@ -15,4 +15,5 @@
159 {
160 return mode == Mode.SCROLLING;
161 }
162+
163 }
164\ No newline at end of file
165
166=== modified file 'tests/test.vala'
167--- tests/test.vala 2013-06-20 18:17:28 +0000
168+++ tests/test.vala 2013-08-01 15:02:04 +0000
169@@ -145,7 +145,7 @@
170 // Wait until remote login appears
171 scroll_to_remote_login (list);
172
173- GLib.assert (list.num_entries() == 30);
174+ GLib.assert (list.num_entries() > 0);
175
176 // Make sure we are at the beginning of the list
177 do_scroll (list, GreeterList.ScrollTarget.START);
178@@ -688,6 +688,63 @@
179 mw.hide ();
180 }
181
182+ public static void remote_login_only ()
183+ {
184+ UnityGreeter.singleton.test_mode = true;
185+ UnityGreeter.singleton.session_started = false;
186+
187+ /* this configuration should result in the list containing only the remote login entry,
188+ without any fallback manual entry */
189+ UnityGreeter.singleton._hide_users_hint = true;
190+ UnityGreeter.singleton._show_remote_login_hint = true;
191+ UnityGreeter.singleton._has_guest_account_hint = false;
192+ UnityGreeter.singleton._show_manual_login_hint = false;
193+
194+ MainWindow mw = setup ();
195+ TestList list = mw.stack.top () as TestList;
196+
197+ /* don't go too fast, otherwise the lastest gdk3 will lose control... */
198+ Posix.sleep(1);
199+
200+ /* Wait for Remote Login to appear */
201+ bool rl_appeared = false;
202+ for (int i=0; i<100 && !rl_appeared; i++)
203+ {
204+ do_scroll (list, GreeterList.ScrollTarget.END);
205+ process_events ();
206+ if (list.selected_entry.id == "*remote_directory*http://crazyurl.com")
207+ rl_appeared = true;
208+ }
209+
210+ GLib.assert (rl_appeared);
211+ GLib.assert (list.num_entries() == 1);
212+ GLib.assert (list.selected_entry.id == "*remote_directory*http://crazyurl.com");
213+
214+ mw.hide ();
215+ }
216+
217+ public static void manual_login_fallback ()
218+ {
219+ UnityGreeter.singleton.test_mode = true;
220+ UnityGreeter.singleton.session_started = false;
221+
222+ /* this configuration should result in the list containing at least a manual entry */
223+ UnityGreeter.singleton._hide_users_hint = true;
224+ UnityGreeter.singleton._show_remote_login_hint = false;
225+ UnityGreeter.singleton._has_guest_account_hint = false;
226+ UnityGreeter.singleton._show_manual_login_hint = true;
227+
228+ MainWindow mw = setup ();
229+ TestList list = mw.stack.top () as TestList;
230+
231+ /* verify if the manual entry has been added as a fallback mechanism */
232+ GLib.assert (list.num_entries() == 1);
233+ GLib.assert (list.selected_entry.id == "*other");
234+
235+ mw.hide ();
236+ }
237+
238+
239 static void setup_gsettings()
240 {
241 try
242@@ -740,6 +797,8 @@
243 GLib.Test.add_func ("/Email Autocomplete", email_autocomplete);
244 GLib.Test.add_func ("/Greeter Communication", greeter_communcation);
245 GLib.Test.add_func ("/Unsupported server type", unsupported_server_type);
246+ GLib.Test.add_func ("/Remote Login Only", remote_login_only);
247+ GLib.Test.add_func ("/Manual Login Fallback", manual_login_fallback);
248
249 return GLib.Test.run();
250 }
251
252=== modified file 'tests/unity-greeter.vala'
253--- tests/unity-greeter.vala 2013-02-20 16:17:42 +0000
254+++ tests/unity-greeter.vala 2013-08-01 15:02:04 +0000
255@@ -73,24 +73,28 @@
256 return "";
257 }
258
259+ public bool _show_manual_login_hint = true;
260 public bool show_manual_login_hint ()
261 {
262- return false;
263+ return _show_manual_login_hint;
264 }
265
266+ public bool _show_remote_login_hint = true;
267 public bool show_remote_login_hint ()
268 {
269- return true;
270+ return _show_remote_login_hint;
271 }
272
273+ public bool _hide_users_hint = false;
274 public bool hide_users_hint ()
275 {
276- return false;
277+ return _hide_users_hint;
278 }
279
280+ public bool _has_guest_account_hint = true;
281 public bool has_guest_account_hint ()
282 {
283- return false;
284+ return _has_guest_account_hint;
285 }
286
287 public void start_session (string? session, Background bg)

Subscribers

People subscribed via source and target branches