1200351 - [Camera] Unable to take picture with Hardware Buttons after taking screenshot in Camera

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System::System UI, defect, P2)

Description

[John Thomas [:Johnt]]

Attachment: logcat_20150831_1150.txt

Description:
If the user takes a screenshot while in the Camera app they will be unable to take a picture using the Aries hardware buttons.

Repro Steps:
1) Update a Aries to 20150826130914
2) Open Camera
3) Take a screenshot
4) Attempt to take a picture using hardware buttons.

Actual:
Unable to capture a picture with hardware buttons, volume level becomes adjustable.

Expected:
It is expected that the user will still be able to take photos using hardware buttons after taking a screenshot in camera app.

Environmental Variables:
Device: Aries 2.5 Kk
Build ID: 20150831112343
Gaia: 31e595f86f6bf159b3a9a46816a6ac00a55ca9f9
Gecko: ee6d6f239fb1ce935322504e0ea8de95e381fc1b
Gonk: 2916e2368074b5383c80bf5a0fba3fc83ba310bd
Version: 43.0a1 (2.5)
Firmware Version: D5803_23.1.A.1.28_NCB.ftf
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Repro frequency: 50%
See attached: Video & Logcat 
Video link: https://youtu.be/G0Z9K7_N0ZY

Note: In the video I did not mean to activate the power down menu while attempting to repro this bug.

Comment 1

[John Thomas [:Johnt]]

This issue does NOT occur on Flame 2.5
Result: Camera begins playing with volume hardware buttons.

Environmental Variables:
Device: Flame 2.5 Kk Fullflash (319mb)
Build ID: 20150830193027
Gaia: 31e595f86f6bf159b3a9a46816a6ac00a55ca9f9
Gecko: f2518b8a7b97b5bb477e94bc9131584007aac887
Gonk: c4779d6da0f85894b1f78f0351b43f2949e8decd
Version: 43.0a1 (2.5)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Comment 2

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

[Blocking Requested - why for this release]:

The end user will lose the ability to take pictures using the volume rocker so nominating this as a blocker.

Comment 3

[John Thomas [:Johnt]]

Correction: This issue DOES occur on the Flame 2.5 and Flame 2.2
Result: Unable to capture a picture with hardware buttons, volume level will become adjustable.

Environmental Variables:
Device: Flame 2.5 Kk Fullflash (319mb)
Build ID: 20150830193027
Gaia: 31e595f86f6bf159b3a9a46816a6ac00a55ca9f9
Gecko: f2518b8a7b97b5bb477e94bc9131584007aac887
Gonk: c4779d6da0f85894b1f78f0351b43f2949e8decd
Version: 43.0a1 (2.5)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Environmental Variables:
Device: Flame 2.2
Build ID: 20150827003009
Gaia: 335cd8e79c20f8d8e93a6efc9b97cc0ec17b5a46
Gecko: 16d864d163de
Gonk: bd9cb3af2a0354577a6903917bc826489050b40d
Version: 37.0 (2.2)
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Comment 4

[David Flanagan [:djf]]

I think this is probably a system app bug, not a camera bug.

Comment 5

[Hema Koka [:hema]]

Diego, 

Can you check this on the camera side and if it is not a camera bug, NI Gregor for System help and change component. 

Thanks
Hema

Comment 6

[Diego Marcos [:dmarcos]]

I confirm that this is not a camera bug. keydown events from the windows element are delivered normally on volume down presses until you take a screenshot (Power + volume down). After that you can use the volume keys to change the volume but camera doesn't receive keydown events anmyore. Should we move this to system?

Comment 7

[Hema Koka [:hema]]

Thanks Diego and David!

Gregor, could you please route this to the right dev for investigation. 

Thanks!
Hema

Comment 8

[:gerard-majax]

Maybe a long shot, but I also had issues on some other hardware button usecase: trying to trigger logshake was sometimes not functionnal. So maybe this is completely unrelated to Camera and Screenshot features.

Comment 9

[Gregor Wagner [:gwagner]]

I am a little bit confused about comment 3 here. The Flame doesn't have a hardware camera button. Do we have a button combination to simulate it?

Comment 10

[David Flanagan [:djf]]

Taking this to see if I can fix it, since I know something about hardawre_buttons.js in the system app.

Gregor, to answer comment #9: on flame we use use the volume keys to trigger the shutter, and I can confirm that that breaks after taking a screenshot.

Comment 11

[David Flanagan [:djf]]

On the Aries, we use the volume up and down keys to take a picture. The actual camera button is (by default, anyway) configured to cause the app to refocus.  Possibly related to this bug, taking a screenshot seems harder than it should be in the camera app.  On my first attempt I often end up taking a picture instead of taking a screenshot.

Comment 12

[David Flanagan [:djf]]

I've verified Diego's comment #6.  Before the screenshot the camera app gets keydown events for the volume keys. After the screenshot, it does not.

Comment 13

[David Flanagan [:djf]]

The bug is not specific to camera. If I add a keydown event handler to Gallery, I can see volume up and volumedown key events until I take a screenshot, then they stop getting sent.

I don't think we have any other apps that actually care about the volume keys, so Camera is probably the only app actually affected by the bug.

Comment 14

[David Flanagan [:djf]]

Long pressing both volume keys at once to capture system logs does not cause the same bug. The volume key events continue to be sent in that case.

Also, note that returning to the homescreen and then re-opening hte already running app cures the problem, and key events are sent correctly again after that.  (This makes me worry that the issue is not in the hardware_buttons.js state machine and is instead deeper in gecko.)

Comment 15

[David Flanagan [:djf]]

This is getting more interersting.  I think the screenshot is a red-herring.

If I press and hold the power key to bring up the power menu and then click cancel, I'll find that the volume keys no longer send key events properly.

In the gallery app, if I do the screenshot timing perfectly so that the power menu never comes up, then I still get key events.

In the camera app, it seems to be impossible to take a screenshot without bringing up the power menu the first time you try. That breaks the volume keys, and then after that first failed screenshot, you can always take a screenshot after that since the volume keys are no longer being sent to the app.

This means that if we fix whatever issue in the power menu is breaking the volume keys, then we will probably never be able to take a screenshot in the camera. (I actually saw someone complain about that on a mailing list the other day, so maybe there is a recent change here.)

Also, if you're running camera and press and hold the power key to bring up the menu, you can still use the volume keys to take pictures even though the camera app is not visible.  So that is probably a different system app bug.  It means, however, that the volume keys are not being broken until after the power menu is dismissed.  (I wonder if the system app starts dispatching key events to that menu after we dismiss it or something.)

Comment 16

[David Flanagan [:djf]]

Pulling down the notification tray does not break the volume keys. The power menu is the only thing I've found that does that.

When the volume keys are not being sent to the camera app, I can get them to work again by: 

- going to the homescreen and coming back
- previewing a photo and comign back 
- opening the camera menu and then closing it
- switching from front to rear camera or back
- pulling down the notification tray and closing it again
- changing the flash setting

Strangely, switching from still photos to video mode or back does not fix things.

Things are similar in Gallery. If I break the volume keys with the power menu, they start working again when I switch from thumbnail mode to select mode or to single photo mode.  From single photo mode, I can make them work again by displyaing the info dialog or the share menu or the delete menu or the photo editor ui, or by playing a video. The only thing in gallery that does not fix the arrow keys again is swiping sideways from one photo to the next.

The fact that minor internal things like displaying a new view or changing the flash setting fixes the volume keys says to me that this is not a system app bug, but something weirder and deeper going on in gecko, and I'm worried that I won't be able to fix this.

But I will investigate a bit more in hardware_buttons.js and in the code that displays the power menu to see if I can get any further to the bottom of this.

Comment 17

[David Flanagan [:djf]]

Adding logging to system/js/hardware_buttons.js, I see that when an app is in this broken state, the mozbrowser[before|after]key[up|down] events are not arriving.

So that makes me thing that the bug is in gecko where the key events are dispatched.

Comment 18

[David Flanagan [:djf]]

I wonder if this is a focus management problem. When the sleep dialog is dismissed, do we need to call focus() on the app window?  I should grep for existing focus() and/or blur() calls in the system app and see if we're already doing any kind of focus management...

Comment 19

[David Flanagan [:djf]]

Ah ha! After I break the volume keys with the power menu, all it takes to restore them to working again is one touch on the app.  So most of the things I was listing in comment #16 that restore the keys are red herrings: it is just that I was touching the screen.  

This reinforces my thought that this is a focus bug. Maybe the system app just needs to call focus() on the foreground app.

Comment 20

[David Flanagan [:djf]]

It is a focus bug, and I can fix it with one line in app_window_manager.js:

diff --git a/apps/system/js/app_window_manager.js b/apps/system/js/app_window_manager.js
index 5a90280..ccd4ae7 100644
--- a/apps/system/js/app_window_manager.js
+++ b/apps/system/js/app_window_manager.js
@@ -609,6 +609,7 @@
 
     '_handle_reset-orientation': function() {
       this._activeApp && this._activeApp.setOrientation();
+      this._activeApp && this._activeApp.broadcast('focus');
     },
 
     _handle_appopening: function(evt) {

There is already special case code in that file for when the permission dialog is hidden. Here we're doing the same thing for the sleep menu.  Ideally we'd have a more generic way of doing this, but for a 2.5+ blocker, a special case single-line fix seems good enough.

Unfortunately, as noted earlier in this bug, fixing this means that it is never possible to take a screenshot of the camera app.  We can probably fix that in hardware_buttons.js, but it will harder. And I'm not sure whether screenshots in the camera app is something we would block the release on or not.  We should probably file a new bug on that.

Comment 21

[David Flanagan [:djf]]

I've got a better patch than the one above. Coming soon.

Comment 22

[GitHub Autolander Bot]

Attachment: [gaia] davidflanagan:bug1200351 > mozilla-b2g:master

Comment 23

[David Flanagan [:djf]]

Comment on attachment 8675227 [details] [review]
[gaia] davidflanagan:bug1200351 > mozilla-b2g:master

Etienne: would you review this 2.5+ patch please?

Note that after applying the patch, it will be impossible to take a screenshot of the camera app. I'll find an existing bug or file a new one for that.  Note that this bug didn't actually have anything to do with screenshots bug with the sleep menu.  So you can test that bringing up the sleep menu and then pressing cancel stops the volume keys from taking a picture.  This patch fixes that.

Comment 24

[David Flanagan [:djf]]

The issue of not being able to take a screenshot in camera is covered by bug 1198932.  I'll add notes there that the problem is about to get worse.

Comment 25

[Etienne Segonzac (:etienne)]

Comment on attachment 8675227 [details] [review]
[gaia] davidflanagan:bug1200351 > mozilla-b2g:master

r=me with a test similar to this one [1] added

[1] https://github.com/mozilla-b2g/gaia/blob/b86bffe236928abe156397d3582ae00d4699ac2e/apps/system/test/unit/app_window_manager_test.js#L338-L344

Comment 26

[David Flanagan [:djf]]

Thanks, Etienne. Test added, just waiting now to get the tests to run.

Comment 27

[David Flanagan [:djf]]

Landed on master: https://github.com/mozilla-b2g/gaia/commit/fe82461fb410de261a77318847704863aec46403

Comment 28

[Shally Li（leave from Mozilla）]

This bug has been verified as "pass" on the latest build of Flame KK 2.5 and Aires KK 2.5 by the STR in comment 0.

Actual results: Users can be able to take picture with Hardware Buttons after taking screenshot in Camera.
See attachment: verified_Aries_v2.5.3gp
Reproduce rate: 0/10


Device: Flame KK 2.5 (Pass)
Build ID               20151020150236
Gaia Revision          11eb5d4cb2675d359d277ae17772bc75f7ccedbc
Gaia Date              2015-10-20 16:22:28
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/f397034950304b845175f8dab8fccbdd0e8bf995
Gecko Version          44.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20151020.182846
Firmware Date          Tue Oct 20 18:28:58 EDT 2015
Firmware Version       v18D v4
Bootloader             L1TC000118D0

Device: Aries KK 2.5 (Pass)
Build ID               20151020225607
Gaia Revision          11eb5d4cb2675d359d277ae17772bc75f7ccedbc
Gaia Date              2015-10-20 16:22:28
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/f397034950304b845175f8dab8fccbdd0e8bf995
Gecko Version          44.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20151020.221418
Firmware Date          Tue Oct 20 22:14:26 UTC 2015
Bootloader             s1

Comment 29

[Shally Li（leave from Mozilla）]

Attachment: verified_Aries_v2.5.3gp