Closed Bug 974730 Opened 8 years ago Closed 8 years ago

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


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

Gonk (Firefox OS)
Not set


(Not tracked)



(Reporter: dmarcos, Assigned: dmarcos, NeedInfo)



(4 files)

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.
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
Attached file Pull Request
Attachment #8378728 - Flags: review?(wilsonpage)
Assignee: nobody → dmarcos
Comment on attachment 8378728 [details] [review]
Pull Request

Patch looks good. Could you clarify exactly what `previewStateChange` is?
Attachment #8378728 - Flags: review?(wilsonpage) → review+
Flags: needinfo?(dmarcos)
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.
Flags: needinfo?(dmarcos)
Attachment #8385233 - Flags: review?(dflanagan)
Attached file Pull Request on Master
Attachment #8385234 - Flags: review?(dflanagan)
I let David decide if we land this on master or camera new features
Attachment #8385797 - Flags: review?(wilsonpage)
Is this PR attached to the proper bug?
Flags: needinfo?(singhashish1887)
Attachment #8385233 - Flags: review?(dflanagan) → review+
Attachment #8385234 - Flags: review?(dflanagan) → review+
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.
Landed in master:
Flags: needinfo?(singhashish1887)
Can you add those tests in a separate bug?
Flags: needinfo?(singhashish1887)
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.
Attachment #8385797 - Flags: review?(wilsonpage) → review-
I'm confused. Why are these tests attached to this bug?

Should we move this to a different bug?
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.