993698 - Page action icon from chrome:// URL is scaled down

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P5)

Description

[:Margaret Leibovic]

I made an add-on that puts icons in a /skin directory, and when I try using those icons with the page actions API, they are scaled down.

All the places we use the pageactions API ourselves in fennec, we use drawable:// URLs, so we haven't run into this issue in the product, but we should figure out why these are inconsistent.

Comment 1

[:Margaret Leibovic]

I traced this issue back to GeckoJarReader, but I couldn't find anything obviously wrong in there. Is it possible this could be an issue with NativeZip?

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Wes - Feel free to dupe or "block" on other bugs you have open related to this

Comment 3

[:Margaret Leibovic]

I decided to use data: URIs as a workaround for my home feeds add-on, so it isn't super critical to get this done for 30 (although this is pretty bad for add-on authors).

Comment 4

[Wesley Johnston (:wesj)]

I tried to reproduce this using things in our own theme directory and couldn't see it. I used things like:

chrome://browser/skin/images/errorpage-larry-black.png
chrome://browser/skin/images/logo-hdpi.png
chrome://browser/skin/images/reader-plus-icon-mdpi.png

The reader icon was small, but its a small icon. The rest fit or were scaled down to fit. I wonder if digging into these nested add-on jar's is causing GeckoJarReader some confusion...

Comment 5

[:Margaret Leibovic]

I just realized this is probably dupe of bug 959896. But that was when I was trying to load an image in our own browser code, not through an add-on.

Sounds like we have some jar issues...

Comment 6

[Brad Lassey [:blassey] (use needinfo?)]

filter on [mass-p5]

Comment 7

[Wesley Johnston (:wesj)]

I have no idea whats up here. Margaret was talking about making these multi-resolution problems better in general. Is this still relevant?

Comment 8

[:Margaret Leibovic]

I'm not sure if this is still an issue, but I think add-ons mostly use data: URIs to work around problems like this, so we just might now be noticing it.

Redirecting NI to Sebastian, who's recently looked into how we load images from gecko.

Comment 9

[Sebastian Kaspari (:sebastian; :pocmo)]

Even though I recently dug into GeckoJarReader in an other bug I'm not exactly sure what's going on here. My first assumption is that Android might try to resize the bitmap to match the density of the device (because we are not loading from a drawable-*dpi folder).

Comment 10

[Vivek Balakrishnan[:vivek]]

Attachment: MozReview Request: Bug 993698 Update target density while reading BitmapDrawable from stream r?sebastian

Bug 993698 Update target density while reading BitmapDrawable from stream r?sebastian

Comment 11

[:Margaret Leibovic]

https://reviewboard.mozilla.org/r/21863/#review19825

::: mobile/android/base/util/GeckoJarReader.java:56
(Diff revision 1)
> +                // In fact it discards the resources https://android.googlesource.com/platform/frameworks/base/+/refs/heads/master/graphics/java/android/graphics/drawable/BitmapDrawable.java#191

Well that's not intuitive at all! I know we've had bugs in the past where we didn't properly pass in resources to the BitmapDrawable construtor, but this is another level of confusion. Nice find.

Without testing this out myself, this seems like a sane fix to me.

Comment 12

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8673185 [details]
MozReview Request: Bug 993698 Update target density while reading BitmapDrawable from stream r?sebastian

https://reviewboard.mozilla.org/r/21863/#review19991

LGTM. I tested this patch on my devices.

Comment 13

[Vivek Balakrishnan[:vivek]]

https://hg.mozilla.org/integration/fx-team/rev/c8ec00c5d04fab4dfc576dde62536272ff29e4fd
Bug 993698 Update target density while reading BitmapDrawable from stream r=sebastian

Comment 14

[Carsten Book [:Tomcat]]

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