Closed Bug 931054 Opened 9 years ago Closed 9 years ago

[Camera] filmstrip.js tries to access undefined preview metadata


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

Gonk (Firefox OS)
Not set


(blocking-b2g:hd+, b2g-v1.1hd fixed, b2g-v1.2 fixed)

blocking-b2g hd+
Tracking Status
b2g-v1.1hd --- fixed
b2g-v1.2 --- fixed


(Reporter: dmarcos, Assigned: dmarcos)




(2 files, 2 obsolete files)

Filmstrip hash to call parseJPEGMetadata on the preview to retrieve its metadata.
Blocks: 928614
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
Summary: [Camera] filmstrip.js access undefined preview metadata → [Camera] filmstrip.js tries to access undefined preview metadata
Should be hd+ because it blocks an hd+ bug.
blocking-b2g: --- → hd?
Assignee: nobody → dmarcos
Attached file Github Pull Request (obsolete) —
Attachment #822412 - Flags: review?(dflanagan)
Moving to hd+ since the bug that this blocks is a hd blocker
blocking-b2g: hd? → hd+
Comment on attachment 822412 [details] [review]
Github Pull Request

r+, but see my comments on github:

1) Use + instead of \ for long strings

2) File another hd+ bug for adding devicePixelRatio in the various places it will be needed.  Once we get the embedded preview image size right, photos will look blurry on Helix unless we add devicePixelRatio support, so that new bug will have to block the same bug this one does.
Attachment #822412 - Flags: review?(dflanagan) → review+
Attached file Pull Request
Attachment #822412 - Attachment is obsolete: true
Attaching a patch that applies on v1.1hd. I need some help. I don't have a device to test it. lecky, can you give it a try?
Flags: needinfo?(lecky.wanglei)
Attached patch Patch for v1.1hd (obsolete) — Splinter Review
Flags: needinfo?(lecky.wanglei)
I tested the patch for v.1.1hd and it works fine to me. I'm sending this for review.
Attachment #822693 - Attachment is obsolete: true
Attachment #825442 - Flags: review?(dflanagan)
Attached pull request specific to v.1.1.0hd. Need review for if
Setting the tracking flags, so this can be uplifted to v1.2 as well.

Diego: are there merge conflicts or can we just let a sheriff uplift this to the hd branch?

Also, comment 12 is incomplete... What do you want me to review for?
Flags: needinfo?(dmarcos)
What's a sheriff uplift? The patch I created for master doesn't apply on v.1.1.0hd. The code has diverged.
Flags: needinfo?(dflanagan)
I just need a review for the patch for v.1.1.0hd
Flags: needinfo?(dmarcos)
Comment on attachment 825442 [details] [review]
Pull request for v.1.1.0hd

The patch looks good to me. AFAIK, you don't generally need to request reviews for uplifts. I assume you've tried it out on the hd branch... And if it works, then go ahead and land.
Attachment #825442 - Flags: review?(dflanagan) → review+
Flags: needinfo?(dflanagan)
Closed: 9 years ago
Resolution: --- → FIXED
Still needs merging to the v1.2 branch.
You need to log in before you can comment on or make changes to this bug.