Closed
Bug 1186124
Opened 9 years ago
Closed 9 years ago
Add "Power button locks instantly" setting for lockscreen
Categories
(Firefox OS Graveyard :: Gaia::System::Lockscreen, defect)
Tracking
(Not tracked)
RESOLVED
INCOMPLETE
People
(Reporter: cr, Assigned: cr)
References
Details
(Keywords: feature, Whiteboard: ux-wanted)
Attachments
(2 files)
(deleted),
text/x-github-pull-request
|
gweng
:
review+
harly
:
ui-review+
|
Details |
(deleted),
application/pdf
|
Details |
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.
Assignee | ||
Comment 1•9 years ago
|
||
This depends on resolving bug 1186100 that prevents screen lock from working as expected.
Assignee | ||
Comment 2•9 years ago
|
||
Created https://github.com/mozilla-b2g/gaia/pull/31061 to get the ball rolling. PR also includes fix for bug 1186100.
Comment 3•9 years ago
|
||
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•9 years ago
|
||
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•9 years ago
|
||
I've leave some opinions on the PR. We could have further discussion on that, thanks.
Comment 6•9 years ago
|
||
Assignee | ||
Comment 7•9 years ago
|
||
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?
Whiteboard: ux-wanted
Comment 9•9 years ago
|
||
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.
Flags: needinfo?(firefoxos-ux-bugzilla) → needinfo?(hhsu)
Assignee | ||
Comment 10•9 years ago
|
||
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?
Assignee | ||
Comment 11•9 years ago
|
||
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•9 years ago
|
||
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•9 years ago
|
||
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.
Flags: needinfo?(hhsu)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → cr
Assignee | ||
Comment 14•9 years ago
|
||
Comment on attachment 8637969 [details]
[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.
Attachment #8637969 -
Flags: ui-review?(hhsu)
Attachment #8637969 -
Flags: review?(gweng)
Comment 15•9 years ago
|
||
Comment on attachment 8637969 [details]
[gaia] cr:powerlock > mozilla-b2g:master
LGTM, thank you :)
Attachment #8637969 -
Flags: ui-review?(hhsu) → ui-review+
Comment 16•9 years ago
|
||
Comment on attachment 8637969 [details]
[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.
Attachment #8637969 -
Flags: review?(gweng)
Comment 17•9 years ago
|
||
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.
Flags: needinfo?(hhsu)
Comment 18•9 years ago
|
||
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?
Flags: needinfo?(hhsu)
Assignee | ||
Comment 19•9 years ago
|
||
(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.
Assignee | ||
Updated•9 years ago
|
Assignee | ||
Comment 20•9 years ago
|
||
(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•9 years ago
|
||
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.
Flags: needinfo?(timdream)
Comment 22•9 years ago
|
||
Comment on attachment 8637969 [details]
[gaia] cr:powerlock > mozilla-b2g:master
Okay, then, I think the patch is good to me as :cr's comment explained.
Attachment #8637969 -
Flags: review+
Comment 23•9 years ago
|
||
(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.
Flags: needinfo?(timdream)
Comment 25•9 years ago
|
||
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.
Flags: needinfo?(thsieh)
Assignee | ||
Comment 26•9 years ago
|
||
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•9 years ago
|
||
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".
Assignee | ||
Comment 28•9 years ago
|
||
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.
Flags: needinfo?(thsieh)
Comment 29•9 years ago
|
||
(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.
Flags: needinfo?(thsieh)
Assignee | ||
Comment 30•9 years ago
|
||
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.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → INCOMPLETE
You need to log in
before you can comment on or make changes to this bug.
Description
•