887164 - [MMS-Gallery] BMP and WBMP display support from MMS issue.

Status: RESOLVED FIXED

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

Description

[Leo]

1. Title : BMP & WBMP display support from MMS. 
2. Precondition : MMS should be working & device should support BMP & WBMP.
3. Tester's Action : 
         a. Receive MMS message with BMP image attached. Message is displayed in message list. Click on the image to open it.
         b. Repeat the same procedure with WBMP image.
4. Detailed Symptom (ENG.) : BMP & WBMP images are not displayed hence not able to Save them.
5. Detailed Symptom (KOR.) : 
6. Expected : BMP & WBMP images should be displayed and able to save them.
7. Reproducibility: Y
8. Frequency Rate : 100%

Comment 1

[Leo]

Hi ,

Gallery has been changed to scan all the images, which Gecko/Platform can display. However Gallery Open activity manifest has not been modified to support all the images.

diff --git a/apps/gallery/manifest.webapp b/apps/gallery/manifest.webapp
index 5759143..13fe5a6 100755
--- a/apps/gallery/manifest.webapp
+++ b/apps/gallery/manifest.webapp
@@ -18,7 +18,10 @@
           "image/jpeg", 
           "image/png", 
           "image/gif", 
-          "image/bmp"
+          "image/bmp",
+          "image/vnd.wap.wbmp",
+          "image/x-ms-bmp",
+          "image/x-bmp"
         ]
       }
     }, 
--
After adding the above code in Gallery Manifest, I am able to display BMP and WBMP images when I click them from Message thread. I am able to save BMP, which was received in MMS message but WBMP image saving is failing.

Comment 2

[Leo]

Attachment: Test files are attached(BMP and WBMP)

Test files attached for reference

Comment 3

[Daniel Veditz [:dveditz]]

Not a security problem, unhiding bug.

Comment 4

[Steve Chung [:steveck]]

I will take some time to investigate why wbmp image is no savable.

Comment 5

[Steve Chung [:steveck]]

Hi SC, We need to add wbmp format support in gecko for saving wbmp image. Could you confirm that this gecko changes is allowed for v1.1? Thanks.

Comment 6

[Steve Chung [:steveck]]

I paste some log to http://pastebin.com/9yYYWsrQ. The main error log should be TypeMismatchError.

Comment 7

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

We need to add *.wbmp in devicestorage.properties, so that Device Storage can allow file with wbmp extension to be created in picture folder.
http://dxr.mozilla.org/mozilla-central/source/toolkit/content/devicestorage.properties#l2

Comment 8

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: Add *.wbmp in devicestorage.properties

We need to support *.wbmp so that Message app can save received WBMP image into picture folder.

Comment 9

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 770109 [details] [diff] [review]
Add *.wbmp in devicestorage.properties

I've filed bug 889746 for the gecko part modification.

Comment 10

[Steve Chung [:steveck]]

Hi, could we save the wbmp image as the PNG foramt? wbmp type is not in the storage white list now.

Comment 11

[Leo]

Hi Steve,

No,better not to change the original format of the image.

Also want to know, *.wbmp is not part of v1.1 ? (from comment #5 above)

Comment 12

[Steve Chung [:steveck]]

Hi yuren, Could you help this issue? The wbmp decoder is already implemented in https://bugzilla.mozilla.org/show_bug.cgi?id=882526#c3. We need to replace the wbmp attachment blob to PNG blob:
https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/smil.js#L250 and 
https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/smil.js#L227
The attachment content here should apply wbmp lib to replace with PNG blob. Please also notice the exitPoint here because it's a the conversion is async function:
https://github.com/mozilla-b2g/gaia/blob/master/apps/sms/js/smil.js#L170
We may also need to utilize activeReaders for checking the exitPoint here. Thanks!

Comment 13

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

My WIP here:
https://github.com/yurenju/gaia/commit/4c7e1dd430698ef772b5a6e5072cdb785b5e87f7

Steve, could you give some feedback for this? thanks!

Comment 14

[Steve Chung [:steveck]]

(In reply to Yuren Ju [:yurenju] from comment #13)
> My WIP here:
> https://github.com/yurenju/gaia/commit/
> 4c7e1dd430698ef772b5a6e5072cdb785b5e87f7
> 
> Steve, could you give some feedback for this? thanks!

Thanks for the great job! It look good except one part: We might also need to change the image name too(image.wbmp -> image.png), or it might return type mismatch error while saving the image because the extension and mimetype mismatch. Maybe the type checking will be ignored because gallery change the image saving from device storage to sd storage(sd storage saving will ignore the type checking), but saving the image with correct extension is still welcome. Please correct me if I'm wrong(and I will also test it later), thanks!

Comment 15

[Alex Keybl [:akeybl]]

This will not land for today, so fixing the milestone. Yuren, any updates?

Comment 16

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

I will send a pull request today.

Comment 17

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Attachment: PR: https://github.com/mozilla-b2g/gaia/pull/10996

here we go.

Steve, Mock messages for running sms app on desktop browser is broken because bug 893188, but I tested sms app on desktop browser by reverting commit of bug 893188 and it works.

let me know if any questions!

Comment 18

[Steve Chung [:steveck]]

Comment on attachment 776253 [details] [review]
PR: https://github.com/mozilla-b2g/gaia/pull/10996

I've left some comments on github, it's a minor fixing so r=me with these update. Thanks!

Comment 19

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Merged, thank you Steve :D

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

Comment 20

[Steve Chung [:steveck]]

Hi Yuren, since you set the slide.blob at fist in the new patch, there is no need to set in again if attachment is not wbmp. Please fire another patch for fixing this issue. Sorry I should discover this small bug soon :(  (And thanks Paul for the nice catch!)

Comment 21

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Steve, I landed a new commit for that. thanks Paul's eagle eye!

v1-train:
https://github.com/mozilla-b2g/gaia/commit/e6228f1

Comment 23

[Chris Lee [:clee]]

This is not a blocker for MMS for v1.1.  Does leo need to block on this?

Comment 24

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

v1.1.0hd: e6228f12357da9a1e04d39673485c82a925c66cd
v1.1.0hd: ce0fa1d52c1b44b29226cc472ff8381c2c165566
v1.1.0hd: e1f9a35ae70fe70189af8421ae52eadaa4e9da63