Merge lp://staging/~azzar1/unity/lockscreen into lp://staging/unity

Proposed by Andrea Azzarone
Status: Merged
Approved by: Marco Trevisan (Treviño)
Approved revision: no longer in the source branch.
Merged at revision: 3703
Proposed branch: lp://staging/~azzar1/unity/lockscreen
Merge into: lp://staging/unity
Diff against target: 5118 lines (+3653/-167)
80 files modified
CMakeLists.txt (+2/-0)
UnityCore/DBusIndicators.cpp (+7/-2)
UnityCore/DBusIndicators.h (+5/-1)
UnityCore/GnomeSessionManager.cpp (+30/-12)
UnityCore/GnomeSessionManager.h (+1/-0)
UnityCore/GnomeSessionManagerImpl.h (+1/-0)
UnityCore/SessionManager.h (+5/-0)
debian/control (+3/-5)
hud/HudView.cpp (+0/-2)
lockscreen/BackgroundSettings.cpp (+153/-0)
lockscreen/BackgroundSettings.h (+55/-0)
lockscreen/CMakeLists.txt (+33/-0)
lockscreen/CofView.cpp (+41/-0)
lockscreen/CofView.h (+42/-0)
lockscreen/LockScreenAbstractShield.h (+55/-0)
lockscreen/LockScreenController.cpp (+249/-0)
lockscreen/LockScreenController.h (+74/-0)
lockscreen/LockScreenPanel.cpp (+230/-0)
lockscreen/LockScreenPanel.h (+75/-0)
lockscreen/LockScreenSettings.cpp (+104/-0)
lockscreen/LockScreenSettings.h (+66/-0)
lockscreen/LockScreenShield.cpp (+200/-0)
lockscreen/LockScreenShield.h (+63/-0)
lockscreen/LockScreenShieldFactory.cpp (+34/-0)
lockscreen/LockScreenShieldFactory.h (+51/-0)
lockscreen/UserAuthenticator.h (+56/-0)
lockscreen/UserAuthenticatorPam.cpp (+171/-0)
lockscreen/UserAuthenticatorPam.h (+66/-0)
lockscreen/UserPromptView.cpp (+282/-0)
lockscreen/UserPromptView.h (+81/-0)
lockscreen/pch/lockscreen_pch.hh (+31/-0)
panel/PanelIndicatorEntryView.cpp (+1/-1)
panel/PanelIndicatorsView.cpp (+8/-0)
panel/PanelIndicatorsView.h (+1/-0)
panel/PanelMenuView.cpp (+0/-1)
panel/PanelMenuView.h (+0/-1)
panel/PanelView.cpp (+5/-10)
plugins/unityshell/CMakeLists.txt (+5/-1)
plugins/unityshell/src/nux-text-entry-accessible.cpp (+8/-19)
plugins/unityshell/src/unity-text-input-accessible.cpp (+90/-0)
plugins/unityshell/src/unity-text-input-accessible.h (+57/-0)
plugins/unityshell/src/unitya11y.cpp (+5/-0)
plugins/unityshell/src/unityshell.cpp (+52/-4)
plugins/unityshell/src/unityshell.h (+7/-0)
plugins/unityshell/unityshell.xml.in (+20/-0)
po/POTFILES.in (+1/-0)
services/CMakeLists.txt (+3/-0)
services/panel-main.c (+21/-2)
services/panel-service.c (+19/-5)
services/panel-service.h (+2/-0)
services/unity-panel-service-lockscreen.conf.in (+8/-0)
shutdown/CMakeLists.txt (+1/-0)
shutdown/SessionDBusManager.cpp (+180/-0)
shutdown/SessionDBusManager.h (+50/-0)
shutdown/StandaloneSession.cpp (+1/-0)
tests/CMakeLists.txt (+5/-0)
tests/autopilot/unity/tests/launcher/test_icon_behavior.py (+1/-1)
tests/autopilot/unity/tests/launcher/test_tooltips.py (+2/-3)
tests/autopilot/unity/tests/test_quicklist.py (+11/-6)
tests/autopilot/unity/tests/test_spread.py (+1/-1)
tests/data/external.gschema.xml (+24/-0)
tests/test_gnome_session_manager.cpp (+50/-40)
tests/test_lockscreen_controller.cpp (+335/-0)
tests/test_mock_session_manager.h (+1/-0)
tests/test_text_input.cpp (+4/-2)
tests/test_upstart_wrapper.cpp (+90/-0)
tests/test_user_authenticator_pam.cpp (+58/-0)
tests/test_utils.h (+1/-1)
unity-shared/CMakeLists.txt (+1/-0)
unity-shared/GtkTexture.h (+64/-0)
unity-shared/IMTextEntry.cpp (+3/-2)
unity-shared/IMTextEntry.h (+1/-1)
unity-shared/MockableBaseWindow.h (+0/-1)
unity-shared/TextInput.cpp (+46/-31)
unity-shared/TextInput.h (+12/-8)
unity-shared/UScreen.cpp (+3/-3)
unity-shared/UnityWindowView.cpp (+1/-1)
unity-shared/UpstartWrapper.cpp (+74/-0)
unity-shared/UpstartWrapper.h (+53/-0)
unity-shared/WindowManager.h (+1/-0)
To merge this branch: bzr merge lp://staging/~azzar1/unity/lockscreen
Reviewer Review Type Date Requested Status
Marco Trevisan (Treviño) Approve
PS Jenkins bot (community) continuous-integration Approve
Robert Ancell Approve
Sebastien Bacher Needs Information
Review via email: mp+206291@code.staging.launchpad.net

Commit message

Implement the lockscreen in Unity that looks like unity-greeter. Also allow to fallback to lightdm + greeter (tty switching) in case you need more pam power.

Description of the change

== General ==
Implement the lockscreen in Unity that looks like unity-greeter. Also allow to fallback to lightdm + greeter (tty switching) in case you need more pam power.

https://bugs.launchpad.net/ayatana-design/+bug/878836/+attachment/3980171/+files/Screenshot%20from%202014-02-13%2023%3A40%3A36.png

== Known issues ==
1. The panel is not 50% transparent (FIXED)
2. No way to switch user from the lockscreen (FIXED lp:~andyrock/indicator-session/lockscreen/+merge/207591)
3. alt+f10 shortcut does not work in the lockscreen (FIXED in lp:~3v1n0/unity/lockscreen-panel)
4. Accessibilty not fully implemented (keyboard and screen-reader work, high-contrast need to be implemented)
   See: lp:~andyrock/nux/view-added/+merge/207763
5. Not full pam support (FIXED)

Remaining issues can be fixed just falling back to lightdm (there is an option in ccsm).

== Notes ==
When testing make sure to run the correct panel service.
At the moment you can lock the screen just using the shortcut (Super+L) or the command:
sudo loginctl lock-sessions

That's because session-menu->Lock does call gnome-screensaver methods to lock the screen.
I'll propose a branch as soon as possible.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Robert Ancell (robert-ancell) wrote :

587 +void Controller::LockScreenUsingDisplayManager()
588 +{
589 + // TODO (andy) Move to a different class (DisplayManagerWrapper)
590 + const char* session_path_raw = g_getenv("XDG_SESSION_PATH");
591 + std::string session_path = session_path_raw ? session_path_raw : "";

You should just give up if $XDG_SESSION_PATH is undefined - you can't call Lock() without it.

review: Needs Fixing
Revision history for this message
Robert Ancell (robert-ancell) wrote :

I wouldn't worry too much about handling PAM not starting. If this occurs then Real Bad Things (TM) have happened to your system. It's probably best to leave the session locked and get a sysadmin to log into a text terminal to fix it (if PAM is truly broken then they might have a hard time doing that).

1357 + // FIXME (andy) would be nice to support a fallback in case PAM
1358 + // is not available.
1359 + if (!InitPam())
1360 + return false;

Revision history for this message
Robert Ancell (robert-ancell) wrote :

I think you've covered this in point 5, but the PAM support is not complete enough for production. :) I'll re-review when it is.

Revision history for this message
Robert Ancell (robert-ancell) wrote :

+ // TODO (andy) fallback to CK. But we need to figure out how to do that.

Do you need to support ConsoleKit? I don't think you do on Ubuntu, are there other Unity consumers who need it?

If you need to, the following seems to work for LightDM:
http://bazaar.launchpad.net/~lightdm-team/lightdm/trunk/view/head:/liblightdm-gobject/power.c#L91

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Please update the build dependencies in debian as well

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

PanelView: I guess you don't want to add a tray also in lockscreen mode, right? :)
I'd like also if you'd use a struct to initialize more than a bool.

LockScreenSettings: why don't you use the same approach I used for decoration::Settings? It might also help testing...
Also you miss an #include <NuxCore/Property.h>

You miss a lot of includes (I'm not using pch here), and so I get a lot of errors:

#include <NuxGraphics/GLTextureResourceManager.h> // for nux::BaseTexture
Why not a forward declaration?

Anyway, in general it doesn't compile here... I'm getting tons of errors without pch enabled. :(

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Andrea Azzarone (azzar1) wrote :

> PanelView: I guess you don't want to add a tray also in lockscreen mode,
> right? :)
> I'd like also if you'd use a struct to initialize more than a bool.
>

I can't because I need the value of lockscreen_mode_ in the constructor of panel::PanelView.

>
> LockScreenSettings: why don't you use the same approach I used for
> decoration::Settings? It might also help testing...

decoration::Settings?

> Also you miss an #include <NuxCore/Property.h>
>
> You miss a lot of includes (I'm not using pch here), and so I get a lot of
> errors:
>
> #include <NuxGraphics/GLTextureResourceManager.h> // for nux::BaseTexture
> Why not a forward declaration?
>
> Anyway, in general it doesn't compile here... I'm getting tons of errors
> without pch enabled. :(

Ok fixed. Forward declaration with nux is almost impossible :D

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Sebastien Bacher (seb128) wrote :

Thanks for the work, some questions:

- > Also allow to fallback to lightdm + greeter (tty switching) in case you need more pam power.

How do you fallback? Do you plan to put a widget on the lock screen that sends you to the greeter?

- the pam integration doesn't seem a small detail to handle. Robert, do you think it's possible to copy the one from lightdm? (I don't know how much that's a separate part of code that can be copied/reused)

- the lock_screen_type options has none/lightdm/custom, what does "custom" mean there? would that allow use to keep using gnome-screensaver instead of unity if needed?

review: Needs Information
Revision history for this message
Andrea Azzarone (azzar1) wrote :

> Thanks for the work, some questions:
>
> - > Also allow to fallback to lightdm + greeter (tty switching) in case you
> need more pam power.
>
> How do you fallback? Do you plan to put a widget on the lock screen that sends
> you to the greeter?
>

Yeah the branch already does that.

>
> - the pam integration doesn't seem a small detail to handle. Robert, do you
> think it's possible to copy the one from lightdm? (I don't know how much
> that's a separate part of code that can be copied/reused)

I don't think it's possible.

>
> - the lock_screen_type options has none/lightdm/custom, what does "custom"
> mean there? would that allow use to keep using gnome-screensaver instead of
> unity if needed?

None allow to uses gnome-screensaver or whatever you want, custom it's just unity.
I should change name.

Revision history for this message
Robert Ancell (robert-ancell) wrote :

> - the pam integration doesn't seem a small detail to handle. Robert, do you
> think it's possible to copy the one from lightdm? (I don't know how much
> that's a separate part of code that can be copied/reused)

No, but it's not enormously complicated to implement - you just need to be able to handle all the prompts that PAM throws at you. The current code just handles a single username and password prompt (in a very hacky way).

Revision history for this message
Andrea Azzarone (azzar1) wrote :

I'm already working on it.

2014-02-16 22:28 GMT+01:00 Robert Ancell <email address hidden>:

> > - the pam integration doesn't seem a small detail to handle. Robert, do
> you
> > think it's possible to copy the one from lightdm? (I don't know how much
> > that's a separate part of code that can be copied/reused)
>
> No, but it's not enormously complicated to implement - you just need to be
> able to handle all the prompts that PAM throws at you. The current code
> just handles a single username and password prompt (in a very hacky way).
> --
> https://code.launchpad.net/~andyrock/unity/lockscreen/+merge/206291
> You are the owner of lp:~andyrock/unity/lockscreen.
>

--
Andrea Azzarone
http://launchpad.net/~andyrock
http://wiki.ubuntu.com/AndreaAzzarone

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Andrea Azzarone (azzar1) wrote :

Robert can you give a look now?

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Sebastien Bacher (seb128) wrote :

looking to the screenshot on https://launchpadlibrarian.net/166229067/Screenshot%20from%202014-02-13%2023%3A40%3A36.png I see no widget/obvious way to go back to the greeter, is that an available feature? what's the user interaction?

review: Needs Information
Revision history for this message
Robert Ancell (robert-ancell) wrote :

The PAM support looks good to me now.

Revision history for this message
Andrea Azzarone (azzar1) wrote :

> looking to the screenshot on https://launchpadlibrarian.net/166229067/Screensh
> ot%20from%202014-02-13%2023%3A40%3A36.png I see no widget/obvious way to go
> back to the greeter, is that an available feature? what's the user
> interaction?

I'm going to propose a branch against session indicator to add a "Switch user..." item.

Revision history for this message
Sebastien Bacher (seb128) wrote :

> I'm going to propose a branch against session indicator to add a "Switch user..." item.

That seems suboptimal/not obvious, we should have something on the screen for that imho (widget, button)...

Revision history for this message
Andrea Azzarone (azzar1) wrote :

> > I'm going to propose a branch against session indicator to add a "Switch
> user..." item.
>
> That seems suboptimal/not obvious, we should have something on the screen for
> that imho (widget, button)...

Not sure, it's the same location of the "lock/switch account..." in the desktop. John Lea what do you think?

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Andrea Azzarone (azzar1) wrote :

-- package 'gnome-desktop-3.0' not found

What?

Revision history for this message
Sebastien Bacher (seb128) wrote :

> -- package 'gnome-desktop-3.0' not found

you need a build-depends on the library...

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Sebastien Bacher (seb128) wrote :

I've built that locally to give it some testing, it works mostly fine, good work!

Some small comments:

- the default config was to still use gnome-screensaver, is that wanted?

- even after changing the config, using indicator-session or ctrl-alt-l calls gnome-screensaver, do we need to land changes for those at the same time?

- the sound indicator lists e.g rhythmbox for me and let you start it in session, is that wanted?

- the password prompt displays "*" chars, lightdm and our other desktop prompts use dots instead

review: Needs Information
Revision history for this message
Andrea Azzarone (azzar1) wrote :

> I've built that locally to give it some testing, it works mostly fine, good
> work!
>
> Some small comments:
>
> - the default config was to still use gnome-screensaver, is that wanted?

Actually gnome-screensaver sould be disabled by default, at least the lockscreen.

>
> - even after changing the config, using indicator-session or ctrl-alt-l calls
> gnome-screensaver, do we need to land changes for those at the same time?
>

super-l sould work. We need to patch also the session indicator.

> - the sound indicator lists e.g rhythmbox for me and let you start it in
> session, is that wanted?

Nope, but it's an indicator bug.

>
> - the password prompt displays "*" chars, lightdm and our other desktop
> prompts use dots instead

Well nux does not allow us to use dots :/

Revision history for this message
Sebastien Bacher (seb128) wrote :

> Actually gnome-screensaver sould be disabled by default, at least the lockscreen.

hum, in fact I think I tried 'ctrl-alt-l" and got screensaver and though the config was wrong, super-L works as you pointing it

> Nope, but it's an indicator bug.

Ok

> Well nux does not allow us to use dots :/

you mean it doesn't allow unicode chars? :-(

Revision history for this message
Andrea Azzarone (azzar1) wrote :

2014-02-25 13:07 GMT+01:00 Sebastien Bacher <email address hidden>:

> > Actually gnome-screensaver sould be disabled by default, at least the
> lockscreen.
>
> hum, in fact I think I tried 'ctrl-alt-l" and got screensaver and though
> the config was wrong, super-L works as you pointing it
>
> > Nope, but it's an indicator bug.
>
> Ok
>
> > Well nux does not allow us to use dots :/
>
> you mean it doesn't allow unicode chars? :-(
>

Indeed.

> --
> https://code.launchpad.net/~andyrock/unity/lockscreen/+merge/206291
> You are the owner of lp:~andyrock/unity/lockscreen.
>

--
Andrea Azzarone
http://launchpad.net/~andyrock
http://wiki.ubuntu.com/AndreaAzzarone

Revision history for this message
Andrea Azzarone (azzar1) wrote :

Seb I added lignome-desktop-dev but it's not working. http://bazaar.launchpad.net/~andyrock/unity/lockscreen/revision/4091

Revision history for this message
Sebastien Bacher (seb128) wrote :

> lignome-desktop-dev but it's not working

that's the old gtk2 version, you want "libgnome-desktop-3-dev" (run "dpkg -S gnome-desktop-3.0.pc" on your install to know what binaries ships the file you need ;-)

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Robert Ancell (robert-ancell) wrote :

I'm happy with this as long as the remaining issues are fixed.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

You should revert your changes to po/*.po files, but the lockscreen code is great and works well.

I've introduced some random fixes in lp:~3v1n0/unity/lockscreen-review and lp:~3v1n0/unity/lockscreen-panel to improve multi-monitor and settings support and to cleanup a few things.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.