882526 - Remove Gecko support for WBMP

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Robert O'Callahan (:roc) (email my personal email if necessary)]

In bug 847310 we added support for WBMP images and in bug 852053 we limited it to FirefoxOS. However, my understanding is that WBMP is only needed for MMS (and possibly a few other apps like Gallery). If that's the case, we can easily write a JS library to decode WBMP and draw the results to a <canvas>, add that library to those apps, and remove WBMP support from Gecko. This will eliminate one difference in FirefoxOS vs desktop Gecko, and shrink Gecko a little --- worth doing.
 
Shih-Chiang, does this make sense to you? If so, can you do this? The sooner the better.

Comment 1

[Andreas Gal :gal]

Shih-Chiang, I am in Taipei this week. Lets chat. I can help with the approach if needed. We have done this before for pdf.js.

Comment 2

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

No problem, I think wbmp.js is a good example about how we add a non-web-standard feature in to FirefoxOS.

Comment 3

[Andreas Gal :gal]

I made a JS library you can use:

https://github.com/andreasgal/wbmp.js

Comment 4

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

I suppose that wbmp.js will be used to inline render the <img> in HTML documents. In this case, the following 3 use cases will need to take into consideration:
1. dynamically add/remove <img> point to WBMP file.
2. access <img> attributes in javascript, like get height/width.
3. manipulate <img> attributes via javascript or css, like fade in/fade out.

These might be the hardest part of this bug.
@roc and @gal, how do you think the scope of wbmp.js?

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I suggest extending wbmp.js to create a <canvas> element with the correct size and draw the image into it. Then the <canvas> will behave very much like a regular <img> element --- the things you mentioned should work.

Comment 6

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

Looping @schung and @djf since there are only two apps, Message and Gallery, need to display WBMP image currently.

Using wbmp.js on <img> will lose the original img.src and mimetype. Per discussion with @schung, losing src and mimetype information seems not a problem to Message app.

As for the Gallery app, displaying WBMP is still under discussion.

Comment 7

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

For MMS use case like bug 887164, user can save a WBMP image to device storage. We will need a JS API to add supported file types in order to save the original WBMP binary file in to device storage.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Can we save it as a PNG instead?

Comment 9

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

Save as a PNG is ok for the purpose of displaying images, but user will not be able to re-distribute the same binary file. I'll definitely want a solution that can extend the full support of an unknown file/mimetype.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Wouldn't it be better to redistribute the image as a PNG? A lot more software can display PNGs than WBMPs.

Comment 11

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

I'm saying if we want to use the same technique for providing tiff.js/raw.js, keep the original binary file is a must-have feature IMO. It could be a follow-up bug for supporting additional file format.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Those formats are separate issues.

TIFF is a complex format. I don't know what subset of TIFF you'd want to support. It seems likely to me most TIFF usage could be converted to PNG or JPEG, no problem.

For RAW, I think you're probably right that we'd want to be able to save it locally and use a JS library to load it into specific apps. However, I suggest not worrying about until RAW support becomes important.

Comment 13

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

Attachment: Remove WBMP support from gecko

Bug 887164 landed a patch for using wbmp.js for decoding WBMP image in Gaia, we can now remove the WBMP code from Gecko.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 779074 [details] [diff] [review]
Remove WBMP support from gecko

Review of attachment 779074 [details] [diff] [review]:
-----------------------------------------------------------------

Great, thanks!

Comment 15

[Joe Drew (not getting mail)]

Comment on attachment 779074 [details] [diff] [review]
Remove WBMP support from gecko

Review of attachment 779074 [details] [diff] [review]:
-----------------------------------------------------------------

r=hooray

Comment 16

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

This patch should be also checked in to b2g18 for syncing the capability between different release of Firefox OS, asking for triage.

Comment 17

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

https://tbpl.mozilla.org/?tree=Try&rev=abf4e9b25f30

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d33324fd2251

Comment 19

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d33324fd2251

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Needs a branch-specific patch for uplift.

Comment 21

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

Attachment: remove-wbmp-b2g18.patch

patch for uplift

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/91fa9bf5e638

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/91fa9bf5e638