Closed
Bug 995158
Opened 11 years ago
Closed 10 years ago
lock screen will be shrunk while 2 phones enabling NFC are approaching
Categories
(Firefox OS Graveyard :: Gaia::System::Lockscreen, defect)
Tracking
(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)
People
(Reporter: allstars.chh, Assigned: timdream)
References
Details
(Whiteboard: [p=1])
Attachments
(1 file, 1 obsolete file)
From nfc_manager I found the value of lockScreen.locked is false when boot up. STR: 1. Ensure Screen lock is enabled. 2. Reboot phone First 'screen-change' event will come, at this time, lockScreen.locked is false. Second Settings got the value of nfc.enabled, at this time, lockScreen.locked is still false. Then 'lock' event comes, at this time, lockScreen.locked is still false. Which is strange here, because we already received the 'lock' event, however the value of lockScreen.locked hasn't been updated. Wait for a moment, the screen will go off, 'screen-change' will come, now lockScreen.locked will be true now. Then we unlock the screen, and press power key to lock the screen again, now when the 'lock' event comes, now lockScreen.locked will be true. So there is a different result here, I think the when the first 'lock' event is received by nfc_manager, the lockScreen.enabled hasn't been updated.
Comment 1•11 years ago
|
||
Yoshi, Please clearly point out the user impact so that we can better judge the importance.
Flags: needinfo?(allstars.chh)
Reporter | ||
Comment 2•11 years ago
|
||
NFC is still enabled even when screen is locked.
Flags: needinfo?(allstars.chh)
Comment 3•11 years ago
|
||
Yoshi, in this cases, that power consumption will be high because NFC is not disabled. But for users, they don't see anything changing when this bug is happened. right?
Updated•11 years ago
|
blocking-b2g: --- → 2.0?
Updated•11 years ago
|
blocking-b2g: 2.0? → backlog
Reporter | ||
Comment 5•11 years ago
|
||
After discussed with Greg, we should listen to 'lock' and 'unlock' event, and we shouldn't check lockScreen.enabled, I'll change the components back to NFC and fix it.
blocking-b2g: backlog → 2.0?
Component: Gaia::System::Lockscreen → NFC
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → ARM
Reporter | ||
Updated•11 years ago
|
Assignee: nobody → allstars.chh
Summary: [LockScreen] lockScreen.enabled is false event when 'lock' event is received when boot up. → NFC: Don't use lockScreen.enabled to check screen lock in nfc_manager.js
Updated•11 years ago
|
blocking-b2g: 2.0? → 2.0+
Comment 7•11 years ago
|
||
Comment on attachment 8411548 [details]
Pull Request
The main problem is you should use lockscreen.locked to replace lockscreen.enabled.
Attachment #8411548 -
Flags: review?(alive) → review-
Reporter | ||
Updated•11 years ago
|
Summary: NFC: Don't use lockScreen.enabled to check screen lock in nfc_manager.js → NFC: Don't use lockScreen.locked to check screen lock in nfc_manager.js
Reporter | ||
Comment 8•11 years ago
|
||
There's a typo in the first place, it should be 'lockScreen.locked', I mistyped it as lockScreen.enabled'. Just discussed with Alive, he thinks LockScreen should still provide a way to query current lock status in case other object (like NfcManager) is too late to receive the 'lock' event. So this is still a bug in LockScreen component. Moving Summary and Components back and deassign myself.
Assignee: allstars.chh → nobody
Component: NFC → Gaia::System::Lockscreen
Summary: NFC: Don't use lockScreen.locked to check screen lock in nfc_manager.js → [LockScreen] lockScreen.locked is false event when 'lock' event is received when boot up.
Reporter | ||
Updated•11 years ago
|
Attachment #8411548 -
Attachment is obsolete: true
Comment 9•11 years ago
|
||
I've solved this in my LockScreen as app patch. So if that patch can be landed in schedule we can eliminate both bugs at the same time.
Assignee | ||
Comment 10•11 years ago
|
||
Greg, I am assign this to you so you could close this when that patch lands.
Assignee: nobody → gweng
Comment 11•11 years ago
|
||
(In reply to Yoshi Huang[:allstars.chh] from comment #8) > Just discussed with Alive, he thinks LockScreen should still provide a way > to query current lock status in case other object (like NfcManager) is too > late to receive the 'lock' event. > So this is still a bug in LockScreen component. The main concern is we always need an object to keep the state - no matter it's lockscreen.locked or system.locked or lockscreenWindowManager.locked - for the lately-initialized module to query, unless we WANT to init lockscreen at the end of system bootstrap process --- but this doesn't make sense. We will have more lazily-loaded modules in the future.
Comment 13•10 years ago
|
||
Ken, can you please help understand why this is blocking ? To me I see this as a liveable use-case and may be a nice-to-fix but not a release blocking issue. We would need numbers here on how much the battery drain is to really block on the issue or any other information for this to make a blocking case.
blocking-b2g: 2.0+ → 2.0?
Flags: needinfo?(kchang)
Comment 14•10 years ago
|
||
(In reply to bhavana bajaj [:bajaj] from comment #13) > Ken, can you please help understand why this is blocking ? To me I see this > as a liveable use-case and may be a nice-to-fix but not a release blocking > issue. We would need numbers here on how much the battery drain is to really > block on the issue or any other information for this to make a blocking case. In short, Even if devices are in lock screen mode, the lock screen will be shrank while 2 phones enabling NFC are approaching. It is very strange for user. I think this bug should be fixed before FC date.
Flags: needinfo?(kchang)
Assignee | ||
Comment 15•10 years ago
|
||
(In reply to Ken Chang[:ken] from comment #14) > In short, Even if devices are in lock screen mode, the lock screen will be > shrank while 2 phones enabling NFC are approaching. It is very strange for > user. I think this bug should be fixed before FC date. Triage: blocking based on scenario above. Also we feel that for 2.0 we should address this in NFCManager instead, maybe by checking the DOM className instead of locked property. the lockScreen.locked can still be considered as a valid bug but not locking (and we should file another bug to track it).
Assignee: gweng → nobody
blocking-b2g: 2.0? → 2.0+
Component: Gaia::System::Lockscreen → Gaia::System
Flags: needinfo?(kchang)
Summary: [LockScreen] lockScreen.locked is false event when 'lock' event is received when boot up. → lock screen will be shrank while 2 phones enabling NFC are approaching
Comment 16•10 years ago
|
||
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #15) > (In reply to Ken Chang[:ken] from comment #14) > ... we feel that for 2.0 we > should address this in NFCManager instead, maybe by checking the DOM > className instead of locked property. I think this method is more clear and better for fixing this bug. Garner, what do you think?
Flags: needinfo?(kchang) → needinfo?(dgarnerlee)
Comment 17•10 years ago
|
||
Since LockedScreen.locked is stale, maybe assume locked status via state var, until the actual lock status events update arrive, then update that status on an incoming tech event try to cover the race condition later on. I'll check.
Flags: needinfo?(dgarnerlee)
Updated•10 years ago
|
Assignee: nobody → dgarnerlee
Comment 18•10 years ago
|
||
https://bugzilla.mozilla.org/show_bug.cgi?id=1017485#c5 (and c6): ---------------------------------------------------- Ken Chang[:ken] 2014-05-30 00:19:33 PDT NFC isn't allowed to p2p as well when screen is on and locked. ---------------------------------------------------- So I don't get something changed against spec, can someone re-link the full expected UX again (as the bug indicates, this isn't what's happening right now...it used to).
Flags: needinfo?(kchang)
Comment 19•10 years ago
|
||
BTW, my locked state seems correct whenever queried (this difference might be I have today's master gaia and gecko code as opposed to a month ago). I'm using adb reboot and gui reboot. If this.hwState is the default off, after reset, P2P is still sent up, but Tag reading is off. If you allow the NFC power config to happen anyway, it only sends discovery on and discovery off states, so sending anything at all enables both P2P and Tag reading. I/GeckoDump( 177): [DEBUG] SYSTEM NFC: XXXXXXXXXXXX handleEvent type: screenchange I/GeckoDump( 177): [DEBUG] SYSTEM NFC: XXXXXXXXXXXXXXXXXXX isScreenUnlockAndEnabled: I/GeckoDump( 177): [DEBUG] SYSTEM NFC: XXXXXXXXX lockScreen.locked: true I/GeckoDump( 177): [DEBUG] SYSTEM NFC: XXXXXXXXX ScreenManager.screenEnabled: true I/GeckoDump( 177): [DEBUG] SYSTEM NFC: ---> true It seems the nfcd has a bad power on default?
Updated•10 years ago
|
Flags: needinfo?(dlee)
Comment 20•10 years ago
|
||
This will be fixed in bug 1017485. The is a follow up bug after supporting card emulation mode when screen off or screen on, locked.
Flags: needinfo?(dlee)
Comment 21•10 years ago
|
||
(In reply to Garner Lee from comment #18) > https://bugzilla.mozilla.org/show_bug.cgi?id=1017485#c5 (and c6): > > ---------------------------------------------------- > Ken Chang[:ken] 2014-05-30 00:19:33 PDT > > NFC isn't allowed to p2p as well when screen is on and locked. > ---------------------------------------------------- > > So I don't get something changed against spec, can someone re-link the full > expected UX again (as the bug indicates, this isn't what's happening right > now...it used to). I don't think we have a specific UX for this and can not find anything related to this from user stories. ni? Juwei.
Flags: needinfo?(kchang) → needinfo?(jhuang)
Reporter | ||
Updated•10 years ago
|
Summary: lock screen will be shrank while 2 phones enabling NFC are approaching → lock screen will be shrunk while 2 phones enabling NFC are approaching
Assignee | ||
Comment 23•10 years ago
|
||
I figured out the problem: the 'lock' event was dispatched before the object is properly assign to window.lockScreen property. This shouldn't be hard unless we have some quirky issue.
Assignee: dgarnerlee → timdream
Status: NEW → ASSIGNED
Component: Gaia::System → Gaia::System::Lockscreen
Whiteboard: [p=1]
Comment 24•10 years ago
|
||
Assignee | ||
Comment 25•10 years ago
|
||
Comment on attachment 8436853 [details]
mozilla-b2g:master PR#20223
Greg, could you rubberstamp this trivial patch we discussed offline? I manually test this on Frame and lock screen still works.
John, in case Greg have not review this tonight and OOO tomorrow, could you review this?
Attachment #8436853 -
Flags: review?(jlu)
Attachment #8436853 -
Flags: review?(gweng)
Assignee | ||
Updated•10 years ago
|
Target Milestone: --- → 2.0 S4 (20june)
Comment 26•10 years ago
|
||
Comment on attachment 8436853 [details]
mozilla-b2g:master PR#20223
Yep, patch looks good and tests have been modified accordingly (Y)
Attachment #8436853 -
Flags: review?(jlu) → review+
Assignee | ||
Updated•10 years ago
|
Attachment #8436853 -
Flags: review?(gweng)
Assignee | ||
Comment 27•10 years ago
|
||
master: https://github.com/mozilla-b2g/gaia/commit/9424afff2d4bf9cd966e89f0a682a8bef85c0246
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
status-b2g-v2.0:
--- → affected
status-b2g-v2.1:
--- → fixed
Resolution: --- → FIXED
Comment 28•10 years ago
|
||
v2.0: https://github.com/mozilla-b2g/gaia/commit/7f671b6facef20e54846c0bffcbb5ee297c6175a
You need to log in
before you can comment on or make changes to this bug.
Description
•