Closed Bug 882526 Opened 6 years ago Closed 6 years ago

Remove Gecko support for WBMP

Categories

(Core :: ImageLib, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25
blocking-b2g leo+
Tracking Status
firefox23 --- wontfix
firefox24 --- wontfix
firefox25 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix
b2g-v1.1hd --- fixed

People

(Reporter: roc, Assigned: schien)

References

Details

(Whiteboard: [LeoVB+])

Attachments

(2 files)

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.
Assignee: nobody → schien
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.
No problem, I think wbmp.js is a good example about how we add a non-web-standard feature in to FirefoxOS.
I made a JS library you can use:

https://github.com/andreasgal/wbmp.js
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?
Flags: needinfo?(roc)
Flags: needinfo?(gal)
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.
Flags: needinfo?(roc)
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.
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.
Can we save it as a PNG instead?
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.
Wouldn't it be better to redistribute the image as a PNG? A lot more software can display PNGs than WBMPs.
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.
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.
Bug 887164 landed a patch for using wbmp.js for decoding WBMP image in Gaia, we can now remove the WBMP code from Gecko.
Attachment #779074 - Flags: review?(roc)
Attachment #779074 - Flags: review?(joe)
Comment on attachment 779074 [details] [diff] [review]
Remove WBMP support from gecko

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

Great, thanks!
Attachment #779074 - Flags: review?(roc) → review+
Comment on attachment 779074 [details] [diff] [review]
Remove WBMP support from gecko

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

r=hooray
Attachment #779074 - Flags: review?(joe) → review+
This patch should be also checked in to b2g18 for syncing the capability between different release of Firefox OS, asking for triage.
blocking-b2g: --- → leo?
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d33324fd2251
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
blocking-b2g: leo? → leo+
Depends on: 887164
Needs a branch-specific patch for uplift.
patch for uplift
Flags: needinfo?(schien)
Whiteboard: [LeoVB+]
Flags: needinfo?(gal)
You need to log in before you can comment on or make changes to this bug.