835193 - [Camera] camera Preview screen do not go to sleep mode even after screen timeout

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Camera, defect, P1)

Description

[Leo]

1. Title : camera Preview screen do not go to sleep mode even after screen timeout
2. Precondition : Screen timeout option should be set
3. Tester's Action : Open camera app
4. Detailed Symptom (ENG.) : camera Preview screen do not go to sleep mode even after screen timeout                                                                                
5. Gaia Revision # :   "7b8dea7e7ec377a2143151bbe5e9998d87f7b36d"
6. Expected :  camera Preview screen should go to sleep mode along with screen timeout
7.Reproducibility: Yes
                1)Frequency Rate : 100%
8.Comparison Results : 
                1)Model Comparing : 
9. Attached files: 
                1)Log : 
                2)Test Contents : 
                3)Video file :

Comment 1

[Leo]

Attachment: patch URL to pull request 8268

Comment 2

[David Flanagan [:djf]]

Comment on attachment 717087 [details]
patch URL to pull request 8268

Technically, this looks like a good fix, so I'm giving r+.

But see github for some changes I'd like you to make before landing this. 

Also, let's wait for feedback from Dale before landing, since he is the primary author of this app.

Dale: does this seem like a reasonable change to you? Is it something we need to clear with UX? I suppose if we had a short screen timeout in settings it could be frustrating trying to compose a shot or wait for action and have the screen go blank...

I wonder if we could use the motion sensors and hold a screen lock if the phone is recording or if it has moved recently?

Comment 3

[Dale Harvey (:daleharvey)]

Comment on attachment 717087 [details]
patch URL to pull request 8268

I dont think this should land, the not going to sleep was explicit

The screen timeout is configurable, right now the minimum is a minute but I wouldnt be surprised to see it go lower. 

I wouldnt expect my camera to turn off at any time while taking a photo, this is currently what android and ios implements.

Will close as invalid, can reopen if there is a reason for doing this

Comment 4

[Leo]

Because of the wake-lock created in Camera app, phone is not going to sleep mode during preview of images/videos from the filmstrip.

So, I am reopening this issue. 

Please check this conflict and provide a solution.

Comment 5

[Alex Keybl [:akeybl]]

Jaime/Product - what's the desired behavior here?

Comment 6

[jachen]

As Dale said in Comment 3, while in camera or video mode, the phone should not go to sleep mode.

However, in the preview mode for the camera, the phone should go to sleep.

I'm not sure if its feasible to implement like this, but that would be the recommended UX.

Comment 7

[Wilfred Mathanaraj [:WDM]]

Agreed with Jamie - this would be the behavior Product would expect.

Comment 8

[Dale Harvey (:daleharvey)]

Yeh going to sleep during preview makes sense, and its very feasible to do so, I will pick it up if noone else has by the time I am done with current bugs. 

It isnt really a conflict with the current patch though, its just a different patch.

Comment 9

[Leo]

Attachment: Pointer to pull request

Added sleep timer for filmstrip preview.

Please review the code.

Comment 10

[Dale Harvey (:daleharvey)]

Comment on attachment 739435 [details]
Pointer to pull request

Patch works great, there were a bunch of nits in there around the code style (using consistent camelCasing mostly) but with those fixed its good to go, thanks.

Comment 11

[Leo]

code will be updated as per coding guidelines soon.

Comment 12

[Leo]

Updated code as per the coding guideline. Please check.

Comment 13

[Dale Harvey (:daleharvey)]

Hey, this needs rebased but looks good aside from that (youll need to readd that r?)

Cheers

Comment 14

[Leo]

Comment on attachment 739435 [details]
Pointer to pull request

Rebase is done. Please check the code and let me know if I need to anything more. Thank you.

Comment 15

[Dale Harvey (:daleharvey)]

Comment on attachment 739435 [details]
Pointer to pull request

I had to edit the commit message on the way in to reference the bug number (and reviewer), so in future those need to be kept in mind

https://github.com/mozilla-b2g/gaia/commit/a24785ad5efd220d14bfda8b4c6a48cb5d8a2e38

Also there seemed to be a filemode change to the files that definitely shouldnt be there (but I just applied the patch as opposed to merging the PR)

The code was fine however, thanks

Comment 16

[James Lal [:lightsofapollo]]

v1-train: 38ca512

Comment 17

[Parul Mathur]

Executed test case in MozTrap https://moztrap.mozilla.org/manage/case/8558/ and verified fixed on:
Device: Leo phone
Build Identifier: 20130612070210
Update channel: no-update-commercial-ril
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/b4f8f0a288ca
Gaia: 547db47241459b5944ba40bacc94b15977d8a02e1371033420
Git commit info: 2013-06-12 10:37:00
OS version: 1.1.0.0-prerelease

Also verified that the phone does not go to sleep in Camera or Video mode.