GNOME Bugzilla – Bug 692560
Always use the shield
Last modified: 2013-01-31 14:24:07 UTC
After the idle-delay is passed, and the lock delay isn't passed yet, the shield should be down, with the desktop underneath (as it does on LiveCDs).
Created attachment 234534 [details] [review] ScreenShield: lower the shield when the user is idle but before locking In time span between idle and lock the shield should behave like autologin, but should prevent accidental reactivation (for example when using a touch screen) by showing the curtain.
Review of attachment 234534 [details] [review]: Full review after the question is answered. ::: js/ui/screenShield.js @@ +670,3 @@ if (this._becameActiveId == 0) this._becameActiveId = this.idleMonitor.connect('became-active', Lang.bind(this, this._onUserBecameActive)); Is this on top of something? I don't have this in master. @@ +757,3 @@ _hideLockScreen: function(animate, velocity) { + if (this._lockScreenState == MessageTray.State.HIDDEN) + return; This is a separate bugfix for a double lower when you hit "Lock" IIRC, so please split it out.
(In reply to comment #2) > Review of attachment 234534 [details] [review]: > > Full review after the question is answered. > > ::: js/ui/screenShield.js > @@ +670,3 @@ > if (this._becameActiveId == 0) > this._becameActiveId = this.idleMonitor.connect('became-active', > Lang.bind(this, > this._onUserBecameActive)); > > Is this on top of something? I don't have this in master. Probably on top of bug 691964 (which itself is on top of 689106).
Yeah, the dependency is a bit convoluted, but I don't want to risk a rebase. The sequence is: - the first patch from bug 691964 - this patch - the two other patches from bug 691964
(In reply to comment #2) > Review of attachment 234534 [details] [review]: > @@ +757,3 @@ > _hideLockScreen: function(animate, velocity) { > + if (this._lockScreenState == MessageTray.State.HIDDEN) > + return; > > This is a separate bugfix for a double lower when you hit "Lock" IIRC, so > please split it out. No, this is _hideLockScreen, so it's about raising the shield. Please review again, so I can land this and the other bug.
Review of attachment 234534 [details] [review]: ::: js/ui/screenShield.js @@ +495,3 @@ + _liftShield: function(onPrimary, velocity) { + if (this._isLocked) { if (this._isLocked) { } else { this.unlock(); } seems weird. Renames of some methods or an added comment would be appreciated. @@ +1082,3 @@ + if (!this._becomeModal()) { + Main.notifyError(_("Failed to activate screen lock"), + _("The screen lock is inhibited by an application capturing the keyboard.")); This needs to be part of the Try harder patch. I'm not convinced it's a good idea quite yet.
Created attachment 234911 [details] [review] ScreenShield: lower the shield when the user is idle but before locking In time span between idle and lock the shield should behave like autologin, but should prevent accidental reactivation (for example when using a touch screen) by showing the curtain.
Review of attachment 234911 [details] [review]: Much better.
Attachment 234911 [details] pushed as 9a25224 - ScreenShield: lower the shield when the user is idle but before locking