Restrict the WBMP support to Firefox OS only

RESOLVED FIXED in Firefox 22

Status

()

Core
ImageLib
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: schien, Assigned: schien)

Tracking

unspecified
mozilla22
Other
Gonk (Firefox OS)
Points:
---
Bug Flags:
in-testsuite -
in-moztrap -

Firefox Tracking Flags

(blocking-b2g:leo+, firefox20 wontfix, firefox21 wontfix, firefox22 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix)

Details

Attachments

(1 attachment, 2 obsolete attachments)

follow-up bug for bug 847310, WBMP should be supported only on FirefoxOS, see https://bugzilla.mozilla.org/show_bug.cgi?id=847310#c20
Created attachment 726098 [details] [diff] [review]
Support WBMP on FirefoxOS only

Nexus 7 can display WBMP image in both default browser and gallery app. I suggest that we could generally support WBMP on FirefoxOS.
Test page: http://people.mozilla.org/~schien/test_wbmp.html

try server result:
https://tbpl.mozilla.org/?tree=Try&rev=b0324bdfdffe
Attachment #726098 - Flags: review?(joe)
Attachment #726098 - Flags: feedback?(roc)
Attachment #726098 - Flags: review?(joe) → review+
Comment on attachment 726098 [details] [diff] [review]
Support WBMP on FirefoxOS only

We definitely shouldn't make the list of formats supported by the B2G browser different to what we support on desktop. So r- on those grounds.

As I see it our three options are:
1) Support WBMP in all circumstances on all platforms.
2) Support WBMP for FirefoxOS apps only (possibly only privileged apps, or apps with the MMS permission).
3) Don't support WBMP in Gecko at all and have the MMS app decode WBMP itself in JS.

I don't think we should take option 1 without a wider discussion in dev.media or dev.platform. Personally, I'm against it on the grounds that no-one really wants to start using WBMP on the Web --- PNG or GIF are much better and already universally supported. I prefer option 3.
Attachment #726098 - Flags: review-
Attachment #726098 - Flags: feedback?(roc)
Attachment #726098 - Flags: feedback-
I'd be fine with #2 or #3.
Created attachment 727061 [details] [diff] [review]
Support WBMP on FirefoxOS privileged/certified app only

This patch is for option #2. 
I add nsIPrincipal check at imgRequest since I cannot get and instance of nsIPricipal in |imgLoader::GetMimeTypeFromContent| and |Image::GetDecoderType|. In this patch, if we open a URL pointed to a WBMP file in browser app on FirefoxOS, e.g. http://people.mozilla.org/~schien/test-wbmp.wbmp, the content view will show "The image cannot be displayed because it contains errors" instead of show nothing.
@joe, do you have any suggestion on how to make it fail more gracefully?
Attachment #726098 - Attachment is obsolete: true
Attachment #727061 - Flags: review?(roc)
Attachment #727061 - Flags: review?(joe)
Comment on attachment 727061 [details] [diff] [review]
Support WBMP on FirefoxOS privileged/certified app only

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

::: image/src/imgRequest.cpp
@@ +727,5 @@
> +#ifdef MOZ_WBMP
> +#ifdef MOZ_WIDGET_GONK
> +    // Only support WBMP in privileged app and certified app, do not support in browser app.
> +    if (newType.EqualsLiteral(IMAGE_WBMP) &&
> +        (!mLoadingPrincipal || mLoadingPrincipal->GetAppStatus() < nsIPrincipal::APP_STATUS_PRIVILEGED)) {

It's too bad we don't get the WBMP mime type until now; it'd be really nice to refuse to load these images altogether.
Attachment #727061 - Flags: review?(joe) → review+
Created attachment 730108 [details] [diff] [review]
Support WBMP on FirefoxOS privileged/certified app only, v2

skip all WBMP reftest cases on B2G platform because decoding WBMP in browser app is not allowed. carry r+ since no logic change.
https://tbpl.mozilla.org/?tree=Try&rev=69ff66d60e42
Attachment #727061 - Attachment is obsolete: true
Attachment #730108 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/fdfe97b3caaf
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
https://hg.mozilla.org/releases/mozilla-b2g18/rev/53b5324732da
status-b2g18: --- → fixed
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → wontfix
status-firefox20: --- → wontfix
status-firefox21: --- → wontfix
status-firefox22: --- → fixed

Updated

5 years ago
Flags: in-moztrap-
You need to log in before you can comment on or make changes to this bug.