Unity 8

Merge lp:~mterry/unity8/fix-greeter-retry into lp:unity8

fix-greeter-retry
Merge into trunk

Proposed by Michael Terry on 2016-03-17

Status:	Merged
Approved by:	Michał Sawicz on 2016-03-25
Approved revision:	2280
Merged at revision:	2314
Proposed branch:	lp:~mterry/unity8/fix-greeter-retry
Merge into:	lp:unity8
Diff against target:	39 lines (+5/-2) 3 files modified qml/Greeter/Greeter.qml (+1/-1) tests/qmltests/Greeter/tst_Greeter.qml (+1/-0) tests/qmltests/tst_Shell.qml (+3/-1)
To merge this branch:	bzr merge lp:~mterry/unity8/fix-greeter-retry
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Michał Sawicz			Approve on 2016-03-25
Unity8 CI Bot	continuous-integration	2016-03-17	Needs Fixing on 2016-03-24
Albert Astals Cid (community)		2016-03-17	Approve on 2016-03-22
Review via email: mp+289317@code.launchpad.net

This proposal supersedes a proposal from 2016-03-16.

Commit Message

Fix showing "Retry" in password prompt when turning on screen in tablet mode.

Description of the Change

When told to show the greeter (power button press or dbus request), we forcibly reset greeter state. This lets us catch when passwords change or the like by resetting the PAM stack.

In one case, we weren't resetting state hard enough and the password prompt thought it hadn't been prompted yet by PAM but still wasn't authorized. So it showed a "Retry" button instead of the normal password prompt.

This fixes that by going through the proper channels for a reset.

= To reproduce =

- Use tablet mode
- Log in
- Lock screen
- Turn screen on
- Turn screen off
- Turn screen on

You should now see a "Retry" button in the greeter password field.

= Checklist =

* Are there any related MPs required for this MP to build/function as expected? Please list.
No

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

* Did you make sure that your branch does not contain spurious tags?
yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
NA

* If you changed the UI, has there been a design review?
NA

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-16: Posted in a previous version of this proposal

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/761/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-17:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/762/rebuild

review: Needs Fixing (continuous-integration)

Albert Astals Cid (aacid) wrote on 2016-03-21:

Can this be autotested?

review: Needs Information

Albert Astals Cid (aacid) wrote on 2016-03-21:

Shell::test_greeterLoginsAutomaticallyWhenNoPasswordSet seems to have regressed because of this. Can you have a look?

review: Needs Fixing

Michael Terry (mterry) wrote on 2016-03-22:

OK, fixed test_greeterLoginsAutomaticallyWhenNoPasswordSet. It was assuming a precise "sessionStarted" signal count, which is no longer true. It *should* be true once Josh's split greeter branches land, which clean up the usage of that signal. But as it stands now, that signal doesn't indicate what you think, and is more of a "successful authentication" signal.

And I added a check to a tst_Greeter.qml check to confirm this MP's fix (that we actually trigger a fresh authentication when calling forceShow.

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-22:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/789/rebuild

review: Needs Fixing (continuous-integration)

Albert Astals Cid (aacid) wrote on 2016-03-22:

* Did you perform an exploratory manual test run of the code change and any related functionality?
Yes

* Did CI run pass?
Yes (except broken autopilot)

* Did you make sure that the branch does not contain spurious tags?
Yes

review: Approve

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-23:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/815/rebuild

review: Needs Fixing (continuous-integration)

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-03-24:

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/823/rebuild

review: Needs Fixing (continuous-integration)

Michał Sawicz (saviq) wrote on 2016-03-25:

IIUC what I *should* see is "Password", not "Retry"? But I still see "Retry" both on first boot and after your steps?

review: Needs Information

Michał Sawicz (saviq) wrote on 2016-03-25:

OK my testing was bogus, did not have the new unity8 installed :/

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Michael Terry

Mohammad Rakibul Hasan

Unity Team

1	=== modified file 'qml/Greeter/Greeter.qml'
2	--- qml/Greeter/Greeter.qml 2016-01-11 17:37:02 +0000
3	+++ qml/Greeter/Greeter.qml 2016-03-22 13:00:09 +0000
4	@@ -60,7 +60,7 @@
5
6	function forceShow() {
7	showNow();
8	- loader.item.reset();
9	+ d.selectUser(d.currentIndex, true);
10	}
11
12	function notifyAppFocused(appId) {
13
14	=== modified file 'tests/qmltests/Greeter/tst_Greeter.qml'
15	--- tests/qmltests/Greeter/tst_Greeter.qml 2016-01-11 17:37:02 +0000
16	+++ tests/qmltests/Greeter/tst_Greeter.qml 2016-03-22 13:00:09 +0000
17	@@ -545,6 +545,7 @@
18
19	LightDM.Greeter.showGreeter();
20	compare(viewResetSpy.count, 1);
21	+ tryCompare(viewShowPromptSpy, "count", 1);
22	}
23	}
24	}
25
26	=== modified file 'tests/qmltests/tst_Shell.qml'
27	--- tests/qmltests/tst_Shell.qml 2016-03-16 16:55:24 +0000
28	+++ tests/qmltests/tst_Shell.qml 2016-03-22 13:00:09 +0000
29	@@ -1082,7 +1082,9 @@
30
31	showGreeter();
32
33	- tryCompare(sessionSpy, "count", 1);
34	+ var greeter = findChild(shell, "greeter");
35	+ verify(!greeter.locked);
36	+ verify(sessionSpy.count > 0);
37	}
38
39	function test_fullscreen() {