Closed Bug 1069879 Opened 8 years ago Closed 8 years ago

Only three dots shown when entering passcode

Categories

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

defect
Not set
normal

Tracking

(tracking-b2g:backlog, b2g-v2.1 verified, b2g-v2.2 fixed)

RESOLVED FIXED
2.1 S7 (24Oct)
tracking-b2g backlog
Tracking Status
b2g-v2.1 --- verified
b2g-v2.2 --- fixed

People

(Reporter: olof.wickstrom, Assigned: gweng)

References

Details

(Whiteboard: [Tako_Blocker])

Attachments

(3 files)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:32.0) Gecko/20100101 Firefox/32.0
Build ID: 20140911151253

Steps to reproduce:

Phone has Lock Screen and Passcode lock enabled
Go to Lock Screen
Slide to right; Passcode field is shown
Enter first three digits of the correct passcode. For each digit a black dot is shown in the white areas above the key-board
Press the fourth correct digit

MUT HW: FLAME
MUT SW: Boot2Gecko2.2.0.0-prerelease Buildidentifier: 20140918160202



Actual results:

Transition to home screen starts without any black dot shown in the fourth white field.


Expected results:

A black dot should have been showed in the fourth white field concurrent with the transition to home screen. 
During the transition the keyboard and the white areas moves downward so the user has time to see the empty field. This may create and uncertainty whether all  four digits are needed or entered correctly.
Flags: needinfo?(firefoxos-ux-bugzilla)
Noting for 2.2 UX backlog but cannot reproduce on current builds.
ux-b2g: --- → 2.2
Flags: needinfo?(firefoxos-ux-bugzilla)
Please see attached movie from a Flame running Build Identifier 20140929040202.
As you can see the fourth dot is shown but not until quite late. It is not shown when the user press the fourth correct digit. It is first shown when the downward animation starts.
Please fix for 2.1
Video was too big to attach. See this link instead: http://youtu.be/ziAh7G_ELQc
Flags: needinfo?(swilkes)
Please fix for 2.1
Impacts large number of users
"Security" in a wide sense is one Firefox OS key selling points.
This bug confuses the user in a area related to protection of all the personal data the user stores on the device. Is three digits enough? Does it matter what fourth digit I press?
blocking-b2g: --- → 2.1?
this doesnt break existing functionality, just visual appearance.  not blocking for 2.1.
blocking-b2g: 2.1? → -
[Blocking Requested - why for this release]:

Partner is requesting this work or 2.1.   re'noming, and cc'ing lockscreen team.   can you guys give an assessment if the patch for this can be done quickly and at low risk?
Status: UNCONFIRMED → NEW
blocking-b2g: - → 2.1?
Ever confirmed: true
Flags: needinfo?(alive)
(In reply to Tony Chung [:tchung] from comment #6)
> [Blocking Requested - why for this release]:
> 
> Partner is requesting this work or 2.1.   re'noming, and cc'ing lockscreen
> team.   can you guys give an assessment if the patch for this can be done
> quickly and at low risk?

is :snowmantw the right person to assess?
Flags: needinfo?(gweng)
Yes

(In reply to Tony Chung [:tchung] from comment #7)
> (In reply to Tony Chung [:tchung] from comment #6)
> > [Blocking Requested - why for this release]:
> > 
> > Partner is requesting this work or 2.1.   re'noming, and cc'ing lockscreen
> > team.   can you guys give an assessment if the patch for this can be done
> > quickly and at low risk?
> 
> is :snowmantw the right person to assess?
Flags: needinfo?(alive)
I can't reproduce it from slower transition. So I think the issue is at the moment the fourth code has been typed:

1. lockscreen.js try to add the black dot at the slot
2. lockscreen_state_manager.js try to perform the sliding animation

1. & 2. are triggered maybe at the same time, so we don't guarantee that 2. would happen *after* 1. get done. In fact, from the slow motion part of the video, since the dot is still get drew in the slot after all, I believe this is the root cause. You can see the video I attached that shows this issue doesn't occur with slow transition:

https://www.youtube.com/watch?v=Pg6IjDWVOWQ

We can fix this, but I don't think we can handle this in schedule.
Flags: needinfo?(gweng)
Whiteboard: [Tako_Blocker]
ux-b2g: 2.2 → ---
Flags: needinfo?(swilkes)
Triage group reviewed and it still doesn't meet the blocking criteria: https://wiki.mozilla.org/B2G/Triage#Blocker_Triage_Guidelines

If a patch is available while 2.1 is still stabilizing, you can ask for uplift approval on the patch.
blocking-b2g: 2.1? → backlog
Assignee: nobody → gweng
The video of the patched v2.1:

http://youtu.be/x31XBNVZvzE 

Please note the square icon is a known issue, and we've discussed it with graphic team.
Flags: needinfo?(bhuang)
Hi Greg,

Is there an ETA of when this will land on master?
Flags: needinfo?(gweng)
If you mean the complete solution, no, since I'm still working on other issues and this is not a trivial problem. But if you want to land the workarounds, I can land it immediately. 

If we need to solve this for partner, we may fire another bug for the complete solution and close this first.
Flags: needinfo?(gweng)
After discussion with Bruce, I would submit review and approval for the v2.1 workaround patch. And maybe fire another bug for animation control.
Attachment #8503871 - Flags: review?(timdream)
Comment on attachment 8503871 [details] [review]
Patch (workaround, 2.1)

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Legacy code manage animation without robust mechanism to ensure these animations perform sequentially.
[User impact] if declined: 3 dots only when it's unlocking
[Testing completed]: Gaia-Try
[Risk to taking this patch] (and alternatives if risky): Animation would be 100ms slower, but partner say it's acceptable from the video
[String changes made]: No
Attachment #8503871 - Flags: approval-gaia-v2.1?(fabrice)
Comment on attachment 8503871 [details] [review]
Patch (workaround, 2.1)

It would be better if we could implement the delay in CSS animations w/o a setTimeout() in the code.

Every time there is a setTimeout without saving the timer, we create a new state which we can't escape from.
Attachment #8503871 - Flags: review?(timdream)
Attachment #8503871 - Flags: approval-gaia-v2.1?(fabrice)
While you're right about the setTimeout issue, it's much difficult to put such CSS transition delay properly since we manage panel animation via some old & tricky class status + state machine. So compare to submitting a pure CSS solution that was a mess from my trying, I put the styling change JS code in the keypadHide state. This may be more close to what you want.
Comment on attachment 8503871 [details] [review]
Patch (workaround, 2.1)

Pure CSS solution is difficult to be done, at least that's what I've tried with the current stylesheet, which mixed lots of code that I'm now trying to refactor at another patch.
Attachment #8503871 - Flags: review?(timdream)
Comment on attachment 8503871 [details] [review]
Patch (workaround, 2.1)

Yeah this is good for the time being.
Attachment #8503871 - Flags: review?(timdream) → review+
master: https://github.com/mozilla-b2g/gaia/commit/381821d774a203686b0455ce0b5a10bb21f883ab

Now submit v2.1 approval.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Comment on attachment 8503871 [details] [review]
Patch (workaround, 2.1)

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Legacy code manage animation without robust mechanism to ensure these animations perform sequentially.
[User impact] if declined: 3 dots only when it's unlocking
[Testing completed]: Gaia-Try
[Risk to taking this patch] (and alternatives if risky): Animation would be 100ms slower, but partner say it's acceptable from the video
[String changes made]: No
Attachment #8503871 - Flags: approval-gaia-v2.1?(fabrice)
sorry had to revert for gaia unit test failures like https://treeherder.mozilla.org/ui/logviewer.html#?job_id=662319&repo=b2g-inbound
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 8503871 [details] [review]
Patch (workaround, 2.1)

OK I would fix the test. I'm very sorry that I missed the test.
Attachment #8503871 - Flags: approval-gaia-v2.1?(fabrice)
Fixed the failure.
All waiting CI.
master: https://github.com/mozilla-b2g/gaia/commit/e5cacda2a8bbb4bf88a18e8a91c990190177674d
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
On 2.1: even I reverted my patch on 2.1 it still failed at the same Gij tests:

https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=713f6c631dd6

And my patch is really irrelevant to these parts. So I now submit the approval for v2.1.
Comment on attachment 8503871 [details] [review]
Patch (workaround, 2.1)

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Legacy code manage animation without robust mechanism to ensure these animations perform sequentially.
[User impact] if declined: 3 dots only when it's unlocking
[Testing completed]: Gaia-Try with intermittent failures. See the above comments.
[Risk to taking this patch] (and alternatives if risky): Animation would be 100ms slower, but partner say it's acceptable from the video
[String changes made]: No
Attachment #8503871 - Flags: approval-gaia-v2.1?(fabrice)
Attachment #8503871 - Flags: approval-gaia-v2.1?(fabrice) → approval-gaia-v2.1+
This issue is NOT fixed on Flame 2.1 and 2.2.

Result: The dot on the 4th field never appears when the user presses the number key and during the animation.

On 2.0, the dot does not appear when the user presses the number key, but appears during the animation, same as the video on Comment 3.

Device: Flame 2.0 (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141031000201
Gaia: 7b8df9941700c1f6d6d51ff464f0c8ae32008cd2
Gecko: 82a6ed695964
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 32.0 (2.0) 
Firmware Version: v188
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141031001201
Gaia: f89c7b12c36572262c9ea76058694a139b1a8634
Gecko: 50d48f8a04c7
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141031061804
Gaia: a07994714f0552f89801d6097982308d8b0a1ee1
Gecko: 6bd2071b373f
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 36.0a1 (2.2) 
Firmware Version: v188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0
QA Whiteboard: [QAnalyst-Triage?][QAnalyst-verify-]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?][QAnalyst-verify-] → [QAnalyst-Triage?][failed-verification]
This bug failed to verified is because when I want to show that the intermittent failures is irrelevant to my patch, I did a commit that revert my work on the branch, and Ryan kindly help me to check in when I didn't notice that. So I think it would be ready if I revert the reverting commit on v2.1 branch.
This is the video shows the reverted result on v2.1:

http://youtu.be/ztgISWPopxE
Depends on: 1092884
And since now I fired the Bug 1092884 as regression to handle this on master, could QA verify it again on 2.1, and mark it as VERFIED?
Flags: needinfo?(ychung)
This issue is verified on Flame 2.1.

Result: All 4 dots are displayed when the user enters the lockscreen passcode.

Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141103001220
Gaia: 027a7de0c95320cea0579bfd1a4ceef3e9038f34
Gecko: ffecb2be228b
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 34.0 (2.1) 
Firmware Version: v188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
===================================================

This issue STILL reproduces on Flame 2.2.

Result: The dot on the 4th field never appears when the user presses the number key and during the animation.

Device: Flame 2.2 Master (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141103040202
Gaia: bc168c17474dabbcceaa349e9bc7c95654435aec
Gecko: 5999e92e89ff
Gonk: 48835395daa6a49b281db62c50805bd6ca24077e
Version: 36.0a1 (2.2) 
Firmware Version: v188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0
Flags: needinfo?(ychung)
QA Whiteboard: [QAnalyst-Triage?][failed-verification] → [QAnalyst-Triage+][failed-verification]
Flags: needinfo?(ktucker)
Depends on: 1102002
Flags: needinfo?(bhuang)
blocking-b2g: backlog → ---
You need to log in before you can comment on or make changes to this bug.