Code review comment for lp:~lukas-kde/unity8/sessionIndicatorForDevices

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

+ Connections {
+ target: DBusUnitySessionService
+ onLockRequested: lightDM.greeter.showGreeter()
+ }

I think this would better fit in Greeter.qml. We try to consolidate all the greeter logic in there, so that it's more easily ignorable if we're running without a greeter (which some split greeter scenarios need).

+ PromptLock();

Huh... org.freedesktop.DisplayManager does not seem to emit a signal when we ask it to lock... I guess the intention was for lightdm to switch VTs. And then the desktop session would notice the vt switch? I'd have to check how we know to lock the session in such cases.

But I guess this is OK for now. We might have to confirm everything works as expected here in the split greeter branches though.

+ if ((new_indicator->identifier() == QStringLiteral("indicator-session"))

Can we not just drop that check and enable the correct profile for indicator-session? Does it still not have profile support? It would be more correct to fix that in the indicator than patch its menu items in unity8...

I haven't run this yet, this was just looking at code.

review: Needs Fixing

« Back to merge proposal