1186124 - Add "Power button locks instantly" setting for lockscreen

Status: RESOLVED INCOMPLETE

(Firefox OS Graveyard :: Gaia::System::Lockscreen, defect)

Description

[Christiane Ruetten [INACTIVE]]

I propose implementing a setting for lockscreen called "Power button locks instantly" that allows the user take manual control of pass code locking while still being able to rely on a long timeout as fallback in case he or she forgets to lock the phone. This is especially useful for people doing regular development work on a personal device. Android for example already has an option for this.

Comment 1

[Christiane Ruetten [INACTIVE]]

This depends on resolving bug 1186100 that prevents screen lock from working as expected.

Comment 2

[Christiane Ruetten [INACTIVE]]

Created https://github.com/mozilla-b2g/gaia/pull/31061 to get the ball rolling. PR also includes fix for bug 1186100.

Comment 3

[Greg Weng [:snowmantw][:gweng][:λ]]

Hi Christiane: I think your patch of Bug 11186100 should be considered and landed individually. Since it's small and simple enough, you can still easily rebase this patch on that. So that we could focus on discussing this feature without considering and syncing the both bugs at the same time.

Comment 4

[Greg Weng [:snowmantw][:gweng][:λ]]

And please invite UX or post their specs here: for a formal feature we need UX to grant it with all the details. Without that, we can't even make sure what's the correct behavior in the future, not to mention to pick up the regressions.

Comment 5

[Greg Weng [:snowmantw][:gweng][:λ]]

I've leave some opinions on the PR. We could have further discussion on that, thanks.

Comment 6

[GitHub Autolander Bot]

Attachment: [gaia] cr:powerlock > mozilla-b2g:master

Comment 7

[Christiane Ruetten [INACTIVE]]

Please clarify what you mean by "post their specs here". Is it sufficient to note "ux-wanted" on the whiteboard, or where can I look up the procedure? Do you have names to CC?

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Just needinfo :fxosux on this.

Comment 9

[Tiffanie Shakespeare [:tif]]

Hi Christiane! I'm not totally following what you're wanting to do. 

If you have either passcode/lock screen enabled or just lock screen, both will immediately lock the device when pressing power or when the screen has timed out (which can be changed in the display setting). Passcode has the additional option of requiring the user to enter the passcode again either immediately upon lock or after a specific amount of time.

If this isn't the behaviour you are seeing, then it's a bug that should be fixed. Please lmk if I have misunderstood your comments.

NI'ing Harly as an FYI.

Comment 10

[Christiane Ruetten [INACTIVE]]

Hi Tiffanie, it's not a bug, but a feature. ;) The feature is to introduce a differentiation of the power button behavior:

1. Power button engages screen lock (so far that's the only option)
2. Power button engages pass code lock (and screen lock implicitly) (new)

The user story behind this is:

I want to be able to completely control pass code locking with the power button, and yet have both a fallback (usually half an hour or an hour) in case I forget to pass code lock, and still enjoy battery saving from a short screen timeout. The two major use cases this covers:

- When I use my device regularly, I push the power button and can be 100% sure my device is pass code locked before it goes into my pocket.
- When I know I will use it again very soon (like during dev work), I can just leave it on my table and let it run into the screen timeout without having to keep it awake by interaction or enter the pass code every time I fail to.

Android has offered this option for ages, and it worked out nicely for me. It gives me a good balance between long development and testing streaks and everyday use. Setting pass code locking timeout to immediately can't provide this. Currently I must decide between one of:

a) Immediate lock, having to enter my pass code dozens of times during a dev streak.
b) Long pass code timeout, resulting in the device being unlocked for long time after everyday use.
c) Reconfigure pass code timeout every time I switch between everyday use and dev streaks.

Makes sense?

Comment 11

[Christiane Ruetten [INACTIVE]]

I should perhaps add that I think that ideally we would have three staged timeouts: screen off, screen lock (after screen off), and passcode lock (after screen lock), but that's a different story.

Comment 12

[Tiffanie Shakespeare [:tif]]

Got it and thank you so much for your quick and through follow up! :)

Harly can follow up with you in regards to UX designs.

Thanks for pinging the UX team!

Comment 13

[Harly Hsu[:harly]]

Yes, I think we can add an item with toggle switch at the bottom of Screen Lock settings call "Power button locks instantly". If user enable this, it will lock the screen instantly when user press the power button, and will require the enter pin to unlock the phone. User can also choose to disable this, and will not lock the screen until the require passcode time is due.

Comment 14

[Christiane Ruetten [INACTIVE]]

Comment on attachment 8637969 [details] [review]
[gaia] cr:powerlock > mozilla-b2g:master

Rebased to master, implemented requested UI change (move to last item in menu), finished tests, ready for final review round.

Comment 15

[Harly Hsu[:harly]]

Comment on attachment 8637969 [details] [review]
[gaia] cr:powerlock > mozilla-b2g:master

LGTM, thank you :)

Comment 16

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8637969 [details] [review]
[gaia] cr:powerlock > mozilla-b2g:master

It's good to me except one change about listening the immediate-lock value: in my memory it need to ignore if the LockScreen is enabled, or our FMD function will be broken since a theft can easily bypass that by disabling LockScreen in the Settings app. If this now works not well we may need to fire another bug to fix it (and obviously we have some missing integration tests about FMD in such case), but to change the conditional statements as the patch may break the feature.

Comment 17

[Greg Weng [:snowmantw][:gweng][:λ]]

Harly: it's a new feature; and I think we need new specs (specifically, the diagram you usually post in  bugs) for new features, right? Or the next time people want to ask if there are any spec we will get into trouble. And you know some undefined behaviors has been identified as "regressions" because there are no specs to make sure what the thing should be.

Comment 18

[Harly Hsu[:harly]]

Hi Greg,
I can definitely help with the spec, but may need to wait as we have other stuff on hand for 2.5.
Also, you brought up a good question. Previously, it will be decided by PM/EPM if a new feature will be added to the release. So I am wondering has this feature been approved by anyone?

Comment 19

[Christiane Ruetten [INACTIVE]]

(In reply to Greg Weng [:snowmantw][:gweng][:λ] from comment #16)
> It's good to me except one change about listening the immediate-lock value:
> in my memory it need to ignore if the LockScreen is enabled, or our FMD
> function will be broken since a theft can easily bypass that by disabling
> LockScreen in the Settings app.

No. FMD's lock command sets both 'lockscreen.enabled' and 'lockscreen.passcode-lock.enabled' before it sets 'lockscreen.lock-immediately' to true in https://mxr.mozilla.org/gaia/source/apps/findmydevice/js/commands.js#148 . Given that, I don't see a way how an attacker can bypass pass code locking.

Note that the order of setting the values is absolutely crucial. I tested the current code and the settings observer callback sequence works as expected. There are, however, no guarantees that it will stay that way in the future. This is a completely separate issue, though. I'll file a follow-up bug on this with FMD.

Comment 20

[Christiane Ruetten [INACTIVE]]

(In reply to Harly Hsu[:harly] from comment #18)
> So I am wondering has
> this feature been approved by anyone?

No. I was missing this feature quite badly (because Android has it), implemented it for myself, and now here it is, ready to grab. Who will make the call?

Comment 21

[Dietrich Ayala (:dietrich)]

Ni? to Tim for guidance from a Gaia module owner on new features (or refinements of existing features) that aren't on the official roadmap.

Comment 22

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8637969 [details] [review]
[gaia] cr:powerlock > mozilla-b2g:master

Okay, then, I think the patch is good to me as :cr's comment explained.

Comment 23

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Dietrich Ayala (:dietrich) from comment #21)
> Ni? to Tim for guidance from a Gaia module owner on new features (or
> refinements of existing features) that aren't on the official roadmap.

I'll say we land it behind a flag but apparently it's not my call anymore.

Comment 24

[Harly Hsu[:harly]]

NI Tina to help providing the UX spec.

Comment 25

[Tina Hsieh[:Tina_Hsieh] UX]

Hello Christiane,

Can you please tell me the reason of your choosing a value selector instead of a toggle for this Power Button Behavior setting? I consider that a toggle for this may looks more intuitive for users.

Comment 26

[Christiane Ruetten [INACTIVE]]

I prototyped it with a checkbox option called "Power button locks instantly", but I encountered two problems with that:

1. It doesn't fit one line, making it visually awkward, and more importantly,

2. it doesn't convey what it really does. If you wanted to be clear about this, you'd also need to somehow convey the technical distinction between screen locking and passcode locking, but that would even turn it into a three-liner.

Turning this into a value chooser solved both issues rather elegantly. It's now a one-liner that clearly differentiates both behaviors by explicit juxtaposition.

Comment 27

[Tina Hsieh[:Tina_Hsieh] UX]

Attachment: [Settings] Screen Lock v1.0.pdf

Hi Christiane,
Thanks for your comment. I've attached the Screen Lock spec and please note that since the "Require Passcode" will influence the power button behavior, there are some differences between our spec and patch:

1. When "Require Passcode" is set to be "immediately", the screen will lock by passcode instantly both in "Instant screen lock" and "Instant passcode lock" situations. It may confuse users when they set the "Power Button Behavior" to "Instant screen lock", but in the end the power button instantly locks by passcode.
Therefore, I've added a gray-out case for "Power Button Behavior".

2. The screen lock setting list has been reordered because of the relationship between "Require Passcode" and "Power Button Behavior".

Comment 28

[Christiane Ruetten [INACTIVE]]

Hi Tina,

I went about implementing the changes you requested. It makes sense to visually deactivate the behavior setting when when timeout is 0, but why did you request that the behavior setting should be reset and not retain its value? In my view, once the user decided on a settings value, changing a different one shouldn't needlessly meddle with that decision.

On a different note, would hiding the behavior setting instead of graying it out also be an option? It requires much fewer code changes, and we're already doing the same with the other pin settings when lockscreen is disabled.

Comment 29

[Tina Hsieh[:Tina_Hsieh] UX]

(In reply to Christiane Ruetten [:cr] from comment #28)
> Hi Tina,
> 
> I went about implementing the changes you requested. It makes sense to
> visually deactivate the behavior setting when when timeout is 0, but why did
> you request that the behavior setting should be reset and not retain its
> value? In my view, once the user decided on a settings value, changing a
> different one shouldn't needlessly meddle with that decision.

I agree with you. 
Therefore I decided NOT to change the power button behavior automatically in the second time (as we know that it was automatically changed to "instant passcode lock" in the first time). I believe that retaining in "Instant passcode lock" is understandable.

> On a different note, would hiding the behavior setting instead of graying it
> out also be an option? It requires much fewer code changes, and we're
> already doing the same with the other pin settings when lockscreen is
> disabled.

Unfortunately, it's our new UX rule that things shouldn't disappear by no reason (it starts to implement in FxOS recently). If the functions are not available in certain situation, they should be grayed out.

Comment 30

[Christiane Ruetten [INACTIVE]]

Closing. This one never made it past UX review and landing it requires substantial UX changes which will not happen given the change in focus.