Lock screen no longer blocks home key press

RESOLVED FIXED in Firefox OS v2.0

Status

Firefox OS
Gaia::System::Lockscreen
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: timdream, Assigned: snowmantw)

Tracking

({regression})

unspecified
2.0 S5 (4july)
x86
Mac OS X
regression

Firefox Tracking Flags

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

Details

Attachments

(3 attachments, 3 obsolete attachments)

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.

Updated

4 years ago
QA Contact: ddixon

Updated

4 years ago
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
Keywords: regressionwindow-wanted

Updated

4 years ago
Blocks: 937442
(Assignee)

Comment 2

4 years ago
Created attachment 8436611 [details] [review]
Patch

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)
(Assignee)

Comment 3

4 years ago
Created attachment 8436612 [details] [review]
Patch v2.0
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)
Attachment #8436612 - Flags: review?(alive)
(Assignee)

Comment 6

4 years ago
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.
(Assignee)

Comment 9

4 years ago
Yes, I would take some time to solve this while I'm not on duty (MAE) according to your suggestion. Thank you.
Duplicate of this bug: 1021599

Updated

4 years ago
Assignee: nobody → gweng

Updated

4 years ago
Target Milestone: --- → 2.0 S5 (4july)

Comment 11

3 years ago
(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)
(Assignee)

Comment 13

3 years ago
Created attachment 8448526 [details] [review]
Patch

Waiting for tests to set review.
Attachment #8436611 - Attachment is obsolete: true
Attachment #8436612 - Attachment is obsolete: true
(Assignee)

Comment 14

3 years ago
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+
(Assignee)

Comment 16

3 years ago
Created attachment 8448806 [details] [review]
Patch

^^ Done. Also fixed some errors.
Attachment #8448806 - Flags: review?(alive)
(Assignee)

Comment 17

3 years ago
Created attachment 8448831 [details] [review]
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+
(Assignee)

Comment 18

3 years ago
The failed tests now passed except one GI and GU failure which passed before:

https://tbpl.mozilla.org/?rev=12dec0d1ba57facac4082d4a1e65dc8e951ef300&tree=Gaia-Try
(Assignee)

Comment 19

3 years ago
master: https://github.com/mozilla-b2g/gaia/commit/0f27ffb69182a2483a4d0028e3dde50c608fb9d0
(Assignee)

Comment 20

3 years ago
v2.0 patch also passed all tests except one known issue:

https://travis-ci.org/mozilla-b2g/gaia/jobs/29025557
(Assignee)

Comment 21

3 years ago
v2.0: https://github.com/mozilla-b2g/gaia/commit/ee4cc647c7a599b210f9d8e76505f22926539d79
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Updated

3 years ago
status-b2g-v2.0: --- → fixed
Reverted for Gaia unit test perma-fail on TBPL.
v2.0: https://github.com/mozilla-b2g/gaia/commit/26f82bf95d237272e66955bbca3323ccf8fcb74a

https://tbpl.mozilla.org/php/getParsedLog.php?id=43021020&tree=Mozilla-Aurora
status-b2g-v2.0: fixed → affected
status-b2g-v2.1: --- → fixed
(Assignee)

Comment 23

3 years ago
Fixed the error and let's try it again.

v2.0: https://github.com/mozilla-b2g/gaia/commit/d7d1b769ac5e826ce49b443a36ca283d3f74f239
(Assignee)

Updated

3 years ago
status-b2g-v2.0: affected → fixed
See Also: → bug 1028443

Updated

3 years ago
status-b2g-v2.1: fixed → verified

Comment 24

3 years ago
Created attachment 8528869 [details]
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.