[Gallery] Image info should show more Exif properties like aperture, speed, focal length and flash.

RESOLVED FIXED in 2.6 S7 - 2/12

Status

Firefox OS
Gaia::Gallery
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: hub, Assigned: hub)

Tracking

unspecified
2.6 S7 - 2/12
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
Image info should show more Exif properties like aperture, speed, focal length and flash.
(Assignee)

Updated

3 years ago
Blocks: 1179570
(Assignee)

Updated

3 years ago
Depends on: 1184155
Created attachment 8699591 [details] [review]
[gaia] hfiguiere:bug1179601-exif-info-ng > mozilla-b2g:master
(Assignee)

Comment 2

3 years ago
This bug is stand alone. But pegged on bug 1178812
Assignee: nobody → hub
Depends on: 1178812
No longer depends on: 1184155
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 3

3 years ago
Comment on attachment 8699591 [details] [review]
[gaia] hfiguiere:bug1179601-exif-info-ng > mozilla-b2g:master

This is based on the other patch that is pending review. We load the Exif on demand when displaying the info.
Attachment #8699591 - Flags: review?(dflanagan)
(Assignee)

Comment 4

3 years ago
it no longer depends on bug 1184155 because we don't really need it for that.
Comment on attachment 8699591 [details] [review]
[gaia] hfiguiere:bug1179601-exif-info-ng > mozilla-b2g:master

This looks like it will be a really nice new feature. Thanks for working on it.

r- because I think you're assuming that all files will have the EXIF data you want to display, and I think you'll get exceptions thrown for files that don't have them.

Also, can you localize the flash strings using the new l20n techniques in MediaUtils.js intead of using get() in info.js?
Attachment #8699591 - Flags: review?(dflanagan) → review-
(Assignee)

Comment 6

3 years ago
Comment on attachment 8699591 [details] [review]
[gaia] hfiguiere:bug1179601-exif-info-ng > mozilla-b2g:master

Rebased on the other changes. Now use the new l20n.js API as well.
Attachment #8699591 - Flags: review- → review?(dflanagan)
(Assignee)

Updated

3 years ago
No longer blocks: 1179570
(Assignee)

Comment 7

2 years ago
Comment on attachment 8699591 [details] [review]
[gaia] hfiguiere:bug1179601-exif-info-ng > mozilla-b2g:master

Punam, this is the second pass review. I addressed all of David's comments. Thanks,
Attachment #8699591 - Flags: review?(dflanagan) → review?(pdahiya)
Comment on attachment 8699591 [details] [review]
[gaia] hfiguiere:bug1179601-exif-info-ng > mozilla-b2g:master

Hi Hub

Great to see EXIF details shutter speed, ISO, Flash on info page for images taken with Flame. I am giving r- because of error seen in logs when metadata.ApertureValue is undefined (see github).

Also on info screen there's an extra line at the end of last info field, if an image doesn't have EXIF e.g. screen shots

Thanks
Attachment #8699591 - Flags: review?(pdahiya) → review-
(Assignee)

Comment 9

2 years ago
Comment on attachment 8699591 [details] [review]
[gaia] hfiguiere:bug1179601-exif-info-ng > mozilla-b2g:master

Fixed the issue.

I'll squash the commits before merging.

Thanks!
Attachment #8699591 - Flags: review- → review?(pdahiya)
Comment on attachment 8699591 [details] [review]
[gaia] hfiguiere:bug1179601-exif-info-ng > mozilla-b2g:master

Hi Hub

Thanks for working on this patch and updating with feedback. I have left last two comments in github and with those addressed it looks good.
Attachment #8699591 - Flags: review?(pdahiya) → review+
(Assignee)

Comment 11

2 years ago
Dealt with the comments.

Merged
https://github.com/mozilla-b2g/gaia/commit/469efe2a2b9d736de78fecaa714a5d8ea5ddfa8f


Thanks !
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.6 S7 - 2/12
You need to log in before you can comment on or make changes to this bug.