974730 - [Camera] Camera Library has to listen for the onPreviewStateChange event

Status: RESOLVED FIXED

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

Description

[Diego Marcos [:dmarcos]]

We have to listen for the onPreviewStateChange event of the gecko camera API and disable the UI elements that trigger camera reconfigurations. We're not allowed to change the state of the camera if the preview is not ready. There's parallel development to improve the robustness of the API and this might no be necessary in the future.

Comment 1

[Diego Marcos [:dmarcos]]

Attachment: Pull Request

Comment 2

[Wilson Page [:wilsonpage]]

Comment on attachment 8378728 [details] [review]
Pull Request

Patch looks good. Could you clarify exactly what `previewStateChange` is?

Comment 3

[David Flanagan [:djf]]

Without this patch, I can crash the app on my nexus 4 by tapping repeatedly and quickly on the front/rear camera toggle button. 

I would expect this patch to disable the button and prevent that crash. But it does not prevent it, so I think this needs further work and I have not landed it.

Investigating further, the problem is that bug 972804 changed hud.css to use .hud_btn instead of just plain a tags, but it missed the disabled buttons case at line 63.

This patch fixes a crash and really needs to land to master.  Please go ahead and land it, Diego, after you change this line in hud.css:

 .hud[buttons-enabled=false] a {

In my testing, changing that line to:

 .hud[buttons-enabled=false] .hud_btn {

makes the patch work.

Comment 4

[Diego Marcos [:dmarcos]]

Attachment: Pull Request on camera new features

Comment 5

[Diego Marcos [:dmarcos]]

Attachment: Pull Request on Master

Comment 6

[Diego Marcos [:dmarcos]]

I let David decide if we land this on master or camera new features

Comment 7

[singhashish1887]

Attachment: Pull Request (camera-dev) UT Test

Comment 8

[Diego Marcos [:dmarcos]]

Is this PR attached to the proper bug?

Comment 9

[David Flanagan [:djf]]

Diego: r+ to land your patches on master and camera-new-features.  (I'd already given r+ for it in comment #3.)  This fixes a crash on master, so we need to land it there.

Comment 10

[Diego Marcos [:dmarcos]]

Landed in master:

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

Comment 11

[Diego Marcos [:dmarcos]]

Can you add those tests in a separate bug?

Comment 12

[Wilson Page [:wilsonpage]]

Comment on attachment 8385797 [details] [review]
Pull Request (camera-dev) UT Test

r- due to:

1. Introducing global mock files loaded with requireApp API. Which none of the other test suites use.
2. Your branch hasn't been rabased on top of dmarco/camera-dev-rebased for a while. Meaning it's out of date, and will not land cleanly.

Comment 13

[Diego Marcos [:dmarcos]]

I'm confused. Why are these tests attached to this bug? 

https://bugzilla.mozilla.org/attachment.cgi?id=8385797

Should we move this to a different bug?