Closed Bug 914740 Opened 6 years ago Closed 6 years ago

Allow using android resources in drawable:// uri's

Categories

(Firefox for Android :: General, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 27

People

(Reporter: wesj, Assigned: wesj)

Details

Attachments

(2 files, 2 obsolete files)

An increasing number of our Addon apis require that you use a resource. Since bundling new ones is very hard for authors (impossible?) it would be nice to expose the Android system ones.
Attached patch Patch v1 (obsolete) — Splinter Review
This searches android.R.drawable namespace stock icons if we can't find an icon in the org.mozilla.gecko.R.drawable one.
Attachment #802499 - Flags: review?(mark.finkle)
Can't add-on devs use jar: URIs to get resources from XPI files?
Some API's (specifically the notification ones) require that you pass a resource ID, since the view that's showing lives outside our Application.
Attached patch Patch v2Splinter Review
This is a little bit more powerful. It allows you to use a resourceID in the drawable:// url if you want. Thought this might be useful for showing lists of apps installed.
Attachment #802499 - Attachment is obsolete: true
Attachment #802499 - Flags: review?(mark.finkle)
Attachment #807313 - Flags: review?(mark.finkle)
Comment on attachment 807313 [details] [diff] [review]
Patch v2

>diff --git a/mobile/android/base/gfx/BitmapUtils.java b/mobile/android/base/gfx/BitmapUtils.java

>     public static int getResource(Uri resourceUrl, int defaultIcon) {

>-            } catch (final Exception e) {} // just means the resource doesn't exist
>+            } catch (final NoSuchFieldException e1) {
>+
>+                // just means the resource doesn't exist for fennec. Check in Android resources

* Remove blank line
* // Just means the resource doesn't exist in this package. Check in Android resources.

>+                    Log.i(LOGTAG, "Exception getting drawable", e3);
>+                }
>+

Remove blank line

In general, I hope this doesn't hurt performance. If it might, try re-arranging the checks so the most typical callees get first try.
Attachment #807313 - Flags: review?(mark.finkle) → review+
Attached patch Alternate (obsolete) — Splinter Review
I needed this for bug 920170 and realized that the other patch didn't really do what I needed anyway. This is a bit different, but adds a new protocol "app-icon" that takes a package name and gives you the package icon.
Attachment #813302 - Flags: review?(mark.finkle)
Attached patch Alt patchSplinter Review
This uses -moz-icon instead.
Attachment #813302 - Attachment is obsolete: true
Attachment #813302 - Flags: review?(mark.finkle)
Attachment #816819 - Flags: review?(mark.finkle)
Comment on attachment 816819 [details] [diff] [review]
Alt patch

>diff --git a/mobile/android/base/gfx/BitmapUtils.java b/mobile/android/base/gfx/BitmapUtils.java

>+        if(data.startsWith("-moz-icon://")) {

if (

>         if(data.startsWith("drawable://")) {

if (



r+ (what could go wrong?)
Attachment #816819 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/mozilla-central/rev/b66f5da7a5e1
Assignee: nobody → wjohnston
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
You need to log in before you can comment on or make changes to this bug.