865177 - [Gallery] Different image format support in Gallery

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Gallery, defect, P2)

Description

[Leo]

1. Title : Doesn't Support images other than JPEG and PNG formats
2. Precondition : 
		1. Gallery app should be running in background.
3. Tester's Action :  When user download's an GIF or BMP image from browser, the image is shown in Gallery 
4. Detailed Symptom (ENG.) :
		1. Open a web browser and visit any web page, which has GIF and BMP images.
		2. Long press on BMP or GIF image to get download option. 
		3. Use download images option to download GIF/BMP image.
		4. Long press Home key, switch to Gallery 
		5. Downloaded images are shown in Gallery
		6. Minimize Gallery application by pressing home key
		7. Long press Home key, to get card view.
		8. Kill Gallery App.
		9. Launch Gallery , downloaded BMP/GIF images are not listed		
5. Expected : Gallery should support different image formats(Eg: PNG,JPEG/JPG,BMP,GIF,TIFF)
6.Reproducibility: Y
	1)Frequency Rate : 100%
7.Gaia Master/v1-train : Reproduced
8.Gaia Revision: b97dddebcb755978041b5f3f146b04ea493e50b0
9.Personal email id:  [e-mail]

Comment 1

[Leo]

Hi djf,

Adding User impact to this defect,
User may want to see various file types in Gallery like JPEG, PNG, GIF and BMP.

1. Can We enable all of these MIME types in Gallery? Gallery.js currently while creating MediaDB instance image/jpeg and image/png is passed as type argument. Is it possible to change type to image/* to support different formats.

2. By adding the MIME types or changing it to image/* as mentioned above,will there any side effects on functionality or performance of gallery application?

3. Gallery is able to show the images of BMP and GIF because when an image is downloaded from browser it will be added to MediaDB without checking for the MIME type, whereas while scanning only image/jpeg and image/png are scanned. Is this the reason for the gallery behaviour as mentioned in this defect? Don't we have to add check for the MIME type even when a new file is getting added to MediaDB after download (in this cased from browser).

please suggest

Comment 2

[David Flanagan [:djf]]

It was hard to understand the bug report because it didn't say that Gallery had to be running when the image was saved.  But if I do that, I can reproduce.

Yes, that is a bug. We should either reject unsupported image times all the time or support them all the time.

This comes up a lot, but I've still never seen product requirements or UX guidelines that say that the Gallery app should display other image formats.

It should be an easy change to make, but I'd like UX input before I make it.

Now that browser can save images and says "saved to gallery", should the gallery app be changed to be able to display those images?  

It seems like a no-brainer, but the reason I'm a little resistant to the idea is that early on we decided that the gallery should not have separate folders to keep photos, screenshots and downloads (e.g.) separate.  I think that mixing random gifs from the web with photos that the user has taken is a bad idea without the ability to segregate them into separate folders.

The Gallery app was designed as a photo gallery, not as the system image viewer, which is what it is turning into.

Casey, can you tell me what to do here?

Comment 3

[Casey Yee]

(In reply to David Flanagan [:djf] from comment #2)

> This comes up a lot, but I've still never seen product requirements or UX
> guidelines that say that the Gallery app should display other image formats.

If there are ill effects (like performance) with adding GIF and BMP as supported image formats, I don't see why we wouldn't enable it.

> 
> Now that browser can save images and says "saved to gallery", should the
> gallery app be changed to be able to display those images?  

I don't see why not.   If a user wants to save the image, they will want to be able to access and view them.   Currently the gallery is the only way to do this.

> 
> It seems like a no-brainer, but the reason I'm a little resistant to the
> idea is that early on we decided that the gallery should not have separate
> folders to keep photos, screenshots and downloads (e.g.) separate.  I think
> that mixing random gifs from the web with photos that the user has taken is
> a bad idea without the ability to segregate them into separate folders.

It's not ideal and certainly having the ability to organize, sort and filter photos within the app are being considered as part of future upgrades to the app.   For now downloaded images should display inline with all the other photos in your gallery.

From a file system perspective we should continue to separate screen shots into a /Screenshots folder, camera photos into /DCIM and perhaps downloads (images or otherwise) from the browser into /Downloads.   
 
> The Gallery app was designed as a photo gallery, not as the system image
> viewer, which is what it is turning into.

Even later when we put in a document manager the gallery will be the main go-to application for viewing images.

Comment 4

[Alex Keybl [:akeybl]]

Please coordinate with the product team if you'd like to ask for this enhancement request to be prioritized for v1.x.

Comment 5

[David Flanagan [:djf]]

Alex,

This isn't enhancement request. There is a bug real here: gallery sometimes displays gifs and bmp images and sometimes it doesn't.  (I can't understand comment 0, but comment 1 explains it better.)

In order to fix this, I need to always allow gifs and bmp images or never allow those image types.  Now that we have feedback from Casey, I'd like to always allow those image types.

The fix will be trivial. (In fact, it would be harder to never allow gifs and bmp images.)  Yes, this ends up enhancing the app, but it is required to fix the bug.

I'm re-nominating and removing the enhancement request tag from the whiteboard.

Comment 6

[Lukas Blakk [:lsblakk] use ?needinfo]

Unless we're going to consider supporting GIF and BMP formats a blocking feature for v1.1 we don't need to block on this to accept a trivial, hopefully low-risk fix.  Tracking will suffice.

Comment 7

[David Flanagan [:djf]]

Attachment: link to patch on github

Dominic,

This patch does two things:

1) It modifies gallery so that it no longers filters images by type. If device storage recognizes it as an image, and if gecko can display it, then Gallery will display it.

2) It fixes a bug in MediaDB where files found by scanning were filtered by mime type test but files found via a device storage notification were not filtered in the same way.

The second issue only occurs when an app uses the mimeTypes option to MediaDB. I think Gallery may have been the only app that did that.  And in the fix for part 1, I remove the mimeTypes array, so you can't actually test the fix for part 2 with this patch.

Comment 8

[David Flanagan [:djf]]

Partha,

Would you verify that the attached patch fixes the bug for you?

Comment 9

[Dominic Kuo [:dkuo]]

Comment on attachment 748239 [details]
link to patch on github

David,

This is a trivial fix and looks good as it is. I can see gif and bmp in gallery app after I applied this patch. If people want this like they said in the previous comments, then it should be no problem.

r+ on commit: 92188a90d64fcca7e48c4512dcf6d72e549d6ea2

Comment 10

[David Flanagan [:djf]]

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

Comment 11

[David Flanagan [:djf]]

Comment on attachment 748239 [details]
link to patch on github

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 865177
User impact if declined: uses will sometimes see gifs in gallery and then they will disappear when gallery is restarted.
Testing completed: locally
Risk to taking this patch (and alternatives if risky): very low risk
String or UUID changes made by this patch: none

Comment 12

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 3173aaca027a625ceab417f49f8aab2b08ed33a9 to:
v1-train: 2c6dac1b0be9b95020000c0175208596e81deea4

Comment 13

[Partha]

Hi djf,

I tested the patch , it works for png,jpeg,gif and bmp formats.

Thanks,