Closed Bug 1020917 Opened 10 years ago Closed 10 years ago

Lock screen no longer blocks home key press

Categories

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

x86
macOS
defect
Not set
normal

Tracking

(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 verified)

RESOLVED FIXED
2.0 S5 (4july)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.1 --- verified

People

(Reporter: timdream, Assigned: gweng)

References

Details

(Keywords: regression)

Attachments

(3 files, 3 obsolete files)

On master with Flame.

STR:

1. launch an app to foreground
2. Press sleep
3. Press sleep again, see lock screen
4. Press home
5. swipe and unlock

Expected:

1. See my foreground app

Actual:

1. See home screen

Note:

Let's find the regression window first to see the affect releases and the offending patch.
QA Contact: ddixon
blocking-b2g: 2.0? → 2.0+
Issue DOES occur in 1.3 and 1.4 Flame builds. 


B2G Inbound Regression Window (Buri device): 

Last Working:

Device: Buri 
Build ID: 20140403064233
Gaia: 90b62f2030c58e2207b2fa2c5892f8b51e178b52
Gecko: e2e287901e1f
Version: 31.0a1 (Master) MOZ
Firmware Version: v1.2device.cfg

First Broken:

Device: Buri 
Build ID: 20140403070634
Gaia: 680f2b0f22dac1eea680158c419a1c7d8a0ad70e
Gecko: d2c0858051bc
Version: 31.0a1 (Master) MOZ
Firmware Version: v1.2device.cfg

Last Working Gecko and First Broken Gaia
Issue DOES occur here. 
Gaia: 680f2b0f22dac1eea680158c419a1c7d8a0ad70e
Gecko: e2e287901e1f

Last Working Gaia and First Broken Gecko
Issue DOES NOT occur here. 
Gaia: 90b62f2030c58e2207b2fa2c5892f8b51e178b52
Gecko: d2c0858051bc


Gaia Pushlog: https://github.com/mozilla-b2g/gaia/compare/90b62f2030c58e2207b2fa2c5892f8b51e178b52...680f2b0f22dac1eea680158c419a1c7d8a0ad70e
Attached file Patch (obsolete) —
I've found this is because the previous assumption -- the lockscreen.js would block the event before AppWindowManager -- now is broken. So I remove it and let the block happen in AppWindowManager to solve this.
Attachment #8436611 - Flags: review?(alive)
Attached file Patch v2.0 (obsolete) —
Attachment #8436612 - Flags: review?(alive)
Comment on attachment 8436612 [details] [review]
Patch v2.0

I dislike AWM knows detail of lock state(although there might be for now). This makes AWM become an all mighty god module again.
Couldn't we block this event in LWM to swallow the home event and improve it later in homeEventDispatcher?
Comment on attachment 8436611 [details] [review]
Patch

clear before having conclusion
Attachment #8436611 - Flags: review?(alive)
I've tried to block it in LWM. However, it never worked, because (as I watched from log) AWM would receive the event *before* LWM. I've also tried to move the order of the initialization of LWM as the first component at the bootstrap.js, but AWM seems would be initialized even before the bootstrap.js' loading callback, it still not worked. So I can only provide solution like this. If you have any better idea, since you know more about System app & bootstrapping than me, I'm glad to fix the patch.

Another idea to solve this is to make a home button manager (or something like that), which would ask all registered auditors (like LWM) to see if the event can be really handled before it got received by other components. In this way we can centralize the event blocking things with a formal component. But this seems complex and may be not so necessary.
Flags: needinfo?(alive)
A proper fix is to make AppWindowManager instantiable and put off its start after lwm start.
But that will change a lot of modules/tests.
I am going to provide a simple workaround.
Flags: needinfo?(alive)
(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #7)
> A proper fix is to make AppWindowManager instantiable and put off its start
> after lwm start.
> But that will change a lot of modules/tests.
> I am going to provide a simple workaround.

Well, my proposal is moving AppWindowManager.init() to bootstrap.js, right after LWM.start();
Could you try this? You are still assignee and this shouldn't touch too many stuff.
Yes, I would take some time to solve this while I'm not on duty (MAE) according to your suggestion. Thank you.
Assignee: nobody → gweng
Target Milestone: --- → 2.0 S5 (4july)
(In reply to Greg Weng [:snowmantw][:gweng][:λ] from comment #9)
> Yes, I would take some time to solve this while I'm not on duty (MAE)
> according to your suggestion. Thank you.

Any update? We don't have any updates for a really long time here!
Flags: needinfo?(gweng)
Greg and Alive are PTO this week.
Flags: needinfo?(gweng)
Attached file Patch (obsolete) —
Waiting for tests to set review.
Attachment #8436611 - Attachment is obsolete: true
Attachment #8436612 - Attachment is obsolete: true
Comment on attachment 8448526 [details] [review]
Patch

Since TBPL hook now is down, I set the review flag first.
Attachment #8448526 - Flags: review?(alive)
Comment on attachment 8448526 [details] [review]
Patch

Please at least have a test in lockscreen_window_manager
Attachment #8448526 - Flags: review?(alive) → feedback+
Attached file Patch
^^ Done. Also fixed some errors.
Attachment #8448806 - Flags: review?(alive)
Attached file Patch for 2.0
Patch for 2.0, and obsoleted the old patch which has been deleted.
Attachment #8448526 - Attachment is obsolete: true
Attachment #8448806 - Flags: review?(alive) → review+
The failed tests now passed except one GI and GU failure which passed before:

https://tbpl.mozilla.org/?rev=12dec0d1ba57facac4082d4a1e65dc8e951ef300&tree=Gaia-Try
v2.0 patch also passed all tests except one known issue:

https://travis-ci.org/mozilla-b2g/gaia/jobs/29025557
v2.0: https://github.com/mozilla-b2g/gaia/commit/ee4cc647c7a599b210f9d8e76505f22926539d79
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
See Also: → 1028443
Attached video VIDEO0047.mp4
This issue has been successfully verified on Flame 2.1:
Gaia-Rev        1bdd49770e2cb7a7321e6202c9bf036ab5d8f200
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/db893274d9a6
Build-ID        20141125001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: