Code review comment for lp:~unity-team/unity/phablet-pinlock

I haven't looked at the new code yet, but here's a review of some of the touch points with existing code:

670 + LightDM.Greeter.authenticate("has-pin")

Testing code?

715 + } else if (authenticationUser == "has-pin"){
716 + Q_EMIT q->showPrompt("Please enter your PIN:", Greeter::PromptTypeSecret);

Please make that just "PIN". That's what the pam_pin module does. And Shell.qml should check for an exact match, not an indexOf. In the UI, you can check for the string and present a nicer one if needed.

723 - Q_EMIT q->showPrompt("Password: ", Greeter::PromptTypeSecret);
724 + Q_EMIT q->showPrompt("Please enter your password:", Greeter::PromptTypeSecret);

PAM will likely just give us "Password:", so we should keep our mock close to that. Plus, you'll have to change some of the qmltests that check for "Password" being passed back. You can, however, check in the UI if the string is "Password" and present something nicer. It's not translated from PAM. (Which reminds me, we should pass it through gettext in LoginList.qml)

732 - authenticated = (response == "password");
733 + if (authenticationUser == "has-password") {
734 + authenticated = (response == "password");

Actually, not only has-password has a password. All the users are assumed to have "password" as their password unless otherwise specified. (And no-password users never get to the handleRespond() function) So please just check for the pin user and fallback to checking for "password".

751 + { "has-pin", "Has PIN", 0, 0, false, false, 0, 0 },

I see you added the user to the "full" mock lightdm. I think it would be more useful to add a new mock liblightdm that only has the pin user. That way we can get a more phone-like experience.

See https://code.launchpad.net/~mterry/unity/phablet-greeter-single-user/+merge/166296 which will make the phone UI only active if there's just one user. And it shows how to add a new mock liblightdm (and corrects a couple bugs with our current multiple mock support).