852053 - Restrict the WBMP support to Firefox OS only

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

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

follow-up bug for bug 847310, WBMP should be supported only on FirefoxOS, see https://bugzilla.mozilla.org/show_bug.cgi?id=847310#c20

Comment 1

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

Attachment: 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

Comment 2

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

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.

Comment 3

[Joe Drew (not getting mail)]

I'd be fine with #2 or #3.

Comment 4

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

Attachment: 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?

Comment 5

[Joe Drew (not getting mail)]

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.

Comment 6

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

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

Comment 7

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

Attachment: 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

Comment 8

[Ryan VanderMeulen [:RyanVM]]

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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

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

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/53b5324732da