1081132 - Possible to get lock screen into landscape mode

Status: VERIFIED FIXED

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

Description

[Ben Francis [:benfrancis]]

I have only seen this happen once on a 2.1 build and I haven't been able to reproduce it since, but as it's such a potentially serious bug I wanted to file it.

The STR were:
* Load a web page in a browser window
* Turn off the screen
* Move the device into a landscape position
* Turn the screen on
* Move the device into a portait position

Expected:
* Lockscreen never gets into landscape mode and never exposes content

Actual:
* When the screen was turned on the lock screen was displayed in landscape mode. When I turned the device into portrait mode the lockscreen only covered half the screen, exposing content behind it.

This was on a fairly recent 2.1 user build updated via OTA on my dogfooding device.

Comment 1

[Ben Francis [:benfrancis]]

Attachment: Screenshot of lockscreen in landscape mode

Comment 2

[Ben Francis [:benfrancis]]

Attachment: Screenshot of lockscreen exposing content

Comment 3

[Ben Francis [:benfrancis]]

Attachment: Screenshot of lockscreen not covering browser window

On the same build I also noticed that once the lockscreen was not covering a browser window when I unlocked the device.

STR:
* Load content in a browser window
* Turn off the screen
* Turn on the screen

Expected:
* Lockscreen displayed

Actual:
* Browser window (but not content) displayed instead, can't be interacted with

I couldn't reproduce this again after a reboot.

Comment 4

[Ben Francis [:benfrancis]]

This might benefit from some exploratory testing by QA?

Comment 5

[Duane Dixon [:ddixon]]

Ben, I'm unable to reproduce this issue after installing a 2.1 build using OTA.  

Repro attempts: 0/25

If you could list what build you were on when you saw this issue, that may be helpful.  Thanks.

Also, this issue you're seeing might be related to: https://bugzilla.mozilla.org/show_bug.cgi?id=1074029  which has already been addressed. 

Environmental Variables:

Device: Flame 2.1
Build ID: 20141003000203
Gaia: 9861c61ec302fb0316c753a2e1c0f592180515fd
Gecko: da68900d1c66
Version: 34.0a2 
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 6

[Ben Francis [:benfrancis]]

Sorry, I think I've received an OTA update then so I'm not sure what build it was.

I'm now on

Build identifier: 20141008000201
Update channel: Aurora
Git commit info: 2014-10-08 02:27:46 d71f8804

I haven't yet seen the landscape lockscreen problem, but I have seen a browser window appear in front of the lockscreen again as in https://bug1081132.bugzilla.mozilla.org/attachment.cgi?id=8503179

I'm afraid I still haven't found reliable STR

Comment 7

[Candice Serran (:cserran)]

Just wanted to make sure this was on your radar Tim...it came up in our sprint planning

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Greg, have you ever saw this?

This maybe an window mgmt bug or lockscreen bug. We don't know until we get a window.

Comment 9

[Greg Weng [:snowmantw][:gweng][:λ]]

I have seen this, but I don't have reliable STR, at least when I try to reproduce it intentionally it disappear. And it seems irrelevant to browser window, because this could occur with only Homescreen & LockScreen.

One thing I've noticed is the last time I saw that, the phone slept a long while (several hours). And it even came with some graphic broken. But I don't know whether this is parts of the STR.

Comment 10

[Ben Francis [:benfrancis]]

Attachment: Screenshot of incoming call screen failing to display

I saw the bug again today where the lockscreen was covered by a browser window when turning on the screen as in https://bug1081132.bugzilla.mozilla.org/attachment.cgi?id=8503179

I also saw this new problem (new attachment) when receiving a phone call this was what I saw instead of the incoming call screen! It prevented me from receiving the call and I had to reboot the device to get out of it.

I can't tell whether this is related but it seems that it might be.

Comment 11

[Ben Francis [:benfrancis]]

This seems serious enough to nominate for 2.1+, I'm sorry I still haven't found more reliable STR.

[Blocking Requested - why for this release]: Seriously broken UI with lockscreen covered by content, prevents the phone from being unlocked.

Comment 12

[Greg Weng [:snowmantw][:gweng][:λ]]

While it's totally justicial to nominate it, without STR we can do nothing but continuously watching if there is any procedure causes to the symptom. Could or should we set qawanted again? Or this would be a really serious but hard to debug 2.1(+) bug.

Comment 13

[Joshua Mitchell (Inactive)]

(In reply to Greg Weng [:snowmantw][:gweng][:λ] from comment #12)
> While it's totally justicial to nominate it, without STR we can do nothing
> but continuously watching if there is any procedure causes to the symptom.
> Could or should we set qawanted again? Or this would be a really serious but
> hard to debug 2.1(+) bug.

No need to set a QA-Wanted keyword, We see this in our Queues because of the Steps-Wanted tag. We've also been looking for STR but no luck over here either.

Comment 14

[KTucker [:KTucker][Inactive 3/4/2016]]

Here are my steps:

1. Slide to camera from the lockscreen.
2. Take a picture with the camera.
3. Tap on the picture preview.
4. Rotate the phone to landscape while viewing the picture.
5. Lock the device.

The lockscreen will be in landscape mode. 

I'm pretty sure this is written elsewhere.

Comment 15

[KTucker [:KTucker][Inactive 3/4/2016]]

I forgot you must have passcode enabled as well.

Comment 16

[Joshua Mitchell (Inactive)]

I can confirm those steps on 2.1 - will attach a screenshot similar to reporter's in next comment.

Swapping Steps-Wanted for QA-Wanted for the rest of the Branch Check

Comment 17

[Joshua Mitchell (Inactive)]

Attachment: 2014-10-14-19-13-19.png

Lockscreen in Landscape mode

Comment 18

[Joshua Mitchell (Inactive)]

Using those STR I find that this issue repro's in 2.2 and 2.1 but does NOT repro in 2.0

Issue DOES repro in 2.2 and 2.1 - KK Builds - Full Flash - 512 mem

Environmental Variables:
Device: Flame 2.1
Build ID: 20141014001201
Gaia: 7e2e65a9668123b54c8cce5dacfdba6f4bd4672b
Gecko: 2325da834971
Version: 34.0 
Firmware Version: v184
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Device: Flame 2.2
Build ID: 20141014040203
Gaia: 4f86c631e0465c0e56ccebeb1324fd28be9ea32f
Gecko: 54217864bae9
Version: 36.0a1 (2.2)
Firmware Version: v184
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0
--------------------------------------------------------------------------------------

Issue does NOT repro in 2.0 - KK Builds - Full Flash - 512 mem

Actual Results: 2.0 does not use the slider, but has the sim pin pad automatically up on lockscreen 

Environmental Variables:
Device: Flame 2.0
Build ID: 20141014000201
Gaia: 21fc294d6c9b78997142153fc32c3175b4835a89
Gecko: 93530962cca3
Version: 32.0 (2.0)
Firmware Version: v184
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 19

[Joshua Mitchell (Inactive)]

Bug 1066741 is a similar issue

Comment 20

[Andrew Overholt [:overholt]]

Regression (with clear STR! :) + user experience of being unable to unlock = blocker.

Comment 21

[Greg Weng [:snowmantw][:gweng][:λ]]

OK, thanks a million for the STR. I would start to debug this.

Comment 22

[Greg Weng [:snowmantw][:gweng][:λ]]

From Comment 14 and Comment 15, it seems this is related to secure camera and the previewer. I'll try it first.

Comment 23

[Greg Weng [:snowmantw][:gweng][:λ]]

Yes, confirmed that this is broken with secure camera's preview mode. Without the passcode and using the ordinary camera via sliding to unlock, it works well. I think LWM/SWM many need to lock the orientation when the foreground secure window get killed.

Comment 24

[Greg Weng [:snowmantw][:gweng][:λ]]

My original thought is to lock the orientation again while the screen is off. But I've found this not work because when it locked, the orientation locking function would not work (return false). An alternative is to lock it at the screen turning on moment, but I suspect that user would unavoidable see the screen rotating transiently.

Comment 25

[Greg Weng [:snowmantw][:gweng][:λ]]

One tricky way to reduce the impact of UX is when the screen is on, do not show LockScreen immediately, but covered it with a whole black screen just like the screen is still off, and we can remove the cover after we successfully lock the orientation. But the risk is user may feel a little delay if the screen is expected to be turned on immediately after he/she press the power button.

Comment 26

[Greg Weng [:snowmantw][:gweng][:λ]]

Attachment: Demo patch

For this issue I submit this demo patch, it:

1. When screen turn off, hide the whole screen for hide LockScreen and the statusbar

2. When screen turn on, try to lock the orientation to portrait, with the setInterval trick, since it may fail at the exact moment the event received

3. After we successfully lock the orientation, display the screen again

I know this patch is dirty, but it demo the possible solution of this bug.

BTW, I really hate to modify ScreenManager according to it's current status...

Comment 27

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8506628 [details] [review]
Demo patch

Alive: I set feedback for the possible proper solution you can provide. I may submit a new patch according to them, thanks.

Comment 28

[(inactive after 6/18) Alive Kuo [:alive]]

Read through the comments I think we are able to get a regression window?

Comment 29

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8506628 [details] [review]
Demo patch

Please try to use visibilitychange instead of spamming lock in screenchange.
Please try not to hide the body to prevent additional bugs.

Comment 30

[Greg Weng [:snowmantw][:gweng][:λ]]

Additional offline comments from Alive:

Open LW while screen off/on is reasonable, while to close it is not.

Comment 31

[Chris Kreinbring [:CKreinbring]]

Regression window
Last working
BuildID: 20140731150609
Gaia: 04ea7e1a4034a50d4a7a4f5b95a04a2ed8313908
Gecko: 104254bd1fc8
Platform Version: 34.0a1
Firmware Version: V123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

First broken
BuildID: 20140801070614
Gaia: 9689218473b6fc4dd927ad6aa7b06c05f0843824
Gecko: 8970589d6505
Platform Version: 34.0a1
Firmware Version: V123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Working Gaia / Broken Gecko = No repro
Gaia: 04ea7e1a4034a50d4a7a4f5b95a04a2ed8313908
Gecko: 8970589d6505
Broken Gaia / Working Gecko = Repro
Gaia: 9689218473b6fc4dd927ad6aa7b06c05f0843824
Gecko: 104254bd1fc8
Gaia pushlog: https://github.com/mozilla-b2g/gaia/compare/04ea7e1a4034a50d4a7a4f5b95a04a2ed8313908...9689218473b6fc4dd927ad6aa7b06c05f0843824

B2G Inbound
Last working
BuildID: 20140731195311
Gaia: f740793dc3f7e59df800c6392209ab0a2486d4c9
Gecko: 4c2e31b74bc4
Platform Version: 34.0a1
Firmware Version: V123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

First broken
BuildID: 20140731202712
Gaia: d1e2bb26d5433a07782c3cccf3e9e49c3f3eb838
Gecko: 102dee00cce2
Platform Version: 34.0a1
Firmware Version: V123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Working Gaia / Broken Gecko = No repro
Gaia: f740793dc3f7e59df800c6392209ab0a2486d4c9
Gecko: 102dee00cce2
Broken Gaia / Working Gecko = Repro
Gaia: d1e2bb26d5433a07782c3cccf3e9e49c3f3eb838
Gecko: 4c2e31b74bc4
Gaia pushlog: https://github.com/mozilla-b2g/gaia/compare/f740793dc3f7e59df800c6392209ab0a2486d4c9...d1e2bb26d5433a07782c3cccf3e9e49c3f3eb838

Comment 32

[Greg Weng [:snowmantw][:gweng][:λ]]

Since 'visibilitychange' would fail to fire when user press the power button quickly, I can only keep the 'screenchange' + setInterval solution.

Comment 33

[Joshua Mitchell (Inactive)]

caused by bug 1041889

Comment 34

[Greg Weng [:snowmantw][:gweng][:λ]]

Attachment: Patch

While I don't like to put the orientation back to the lockscreen.js, I submit the patch we discussed.

The Gaia-Try only failed at those everybody failed tests:

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

I'll submit a v2.1 version later, and after reviewing I would fire a bug to hide LockScreen while the orientation is locking to prevent user see the strange rotating, which may not be 2.1+.

Comment 35

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8509268 [details] [review]
Patch

Let's move the logic into LockscreenWindow instead of in Manager.

Comment 36

[Greg Weng [:snowmantw][:gweng][:λ]]

Alive: for your comment of the patch:

The original version of |setOrientation| in AppWindow return nothing, and it proxy the |lockOrientation| which also return nothing. Should I invent my version of |setOrientation| and do the locking action and omit |lockOrientation|? Since you said LWM should proxy the |setOrientation| and re-try according to it's return value.

Comment 37

[(inactive after 6/18) Alive Kuo [:alive]]

LWM should not care about how the lockscreen window locks orientation. in the future maybe we want a landscape lockscreen, so it's lockscreen window's responsibility to keep the orientation (by reading from manifest and manifest is different for different lockscreen app).

For the lockscreenWindow we could try to 
1. When screen is turned on, let LWM call LW.lockOrientation().
2. Internally check the lock result; if failed the LW should check lock again until succeed.

Comment 38

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8509268 [details] [review]
Patch

Updated the patch and it only failed at some intermittent tests, so I set the review flag.

Comment 39

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8509268 [details] [review]
Patch

r+ with nits

Comment 40

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8509268 [details] [review]
Patch

1. LWM would check if it's active, so I don't think lockOrientation needs to check it again. Unless someone isn't LWM would call it as well, but this won't be a reasonable path to use it.

2. I've found I missed one case: when we press homekey to kill secure app, it would need to lock the orientation, too. So I now set the feedback for this part.

Comment 41

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8509268 [details] [review]
Patch

WFM

Comment 42

[Greg Weng [:snowmantw][:gweng][:λ]]

Attachment: Patch v2.1

Comment 43

[Greg Weng [:snowmantw][:gweng][:λ]]

Gaia-Try is green for master patch:

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

Comment 44

[Greg Weng [:snowmantw][:gweng][:λ]]

master: https://github.com/mozilla-b2g/gaia/commit/82aa9612bc8d05c6ff28a0b02f1107b9c8989af8

Comment 45

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8514056 [details] [review]
Patch v2.1

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): The orientation API design has some flaws make apps can aggressively lock the orientation while we're back from SecureApp or ordinary app. This patch can fix the part Gaia is able to do.
[User impact] if declined: Error occurs
[Testing completed]: Gaia-Try is green except intermittent tests and one Gij failure which failed even after I revert my patch completely, like:

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

I now submit two commits to show that. One is the patch and another is the reverting patch. And it should show that no matter my patch get reverted or not, the test still fails.

[Risk to taking this patch] (and alternatives if risky): No obvious risks
[String changes made]: No

Comment 46

[Greg Weng [:snowmantw][:gweng][:λ]]

The Gij now only failed at one irrelevant Browser assertion failure, which not failed at previous runs:

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

Comment 47

[Greg Weng [:snowmantw][:gweng][:λ]]

Comment on attachment 8514056 [details] [review]
Patch v2.1

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): The orientation API design has some flaws make apps can aggressively lock the orientation while we're back from SecureApp or ordinary app. This patch can fix the part Gaia is able to do.
[User impact] if declined: Error occurs
[Testing completed]: Gaia-Try is green except intermittent tests as the above comment described.
[Risk to taking this patch] (and alternatives if risky): No obvious risks
[String changes made]: No

Comment 48

[Greg Weng [:snowmantw][:gweng][:λ]]

Yes, the test now is passed:

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

Comment 49

[Greg Weng [:snowmantw][:gweng][:λ]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/8e8a50d6256dfef7c90b45a1370b2305d2fbc09e

Comment 50

[Yeojin Chung [:YeojinC]]

This issue is verified fixed on Flame 2.1 and 2.2.

Result: The lockscreen is in portrait mode following STRs on Comment 14. 

Device: Flame 2.1 (319mb)(Kitkat Base)(Full Flash)
BuildID: 20141104001202
Gaia: 8b0cf889ae0d48a9eb7ecdcb9b67590de45cc5e5
Gecko: 388b03efe92d
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: 20141104040207
Gaia: 3c50520982560ccba301474d1ac43706138fc851
Gecko: 54d05732f29b
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