Bug 1300144 adds a bunch of VectorDrawable's that will be used in the new ActivityStream context menu. Some of these are duplicates of existing png resources - we should try and replace those if possible. Note: the existing icons aren't 1:1 copies, moreover VectorDrawable's can't be used everywhere on Android 4 devices, hence we need to be careful about testing every replacement. E.g. updating the share icon in the main and context menus is requiring changes to icon loading code.

Comment on attachment 8803544 [details] Bug 1312114 - Pre: Move getDrawable() from (geckoview) BitmapUtils into (fennec) MenuBitmapUtils to allow support library access https://reviewboard.mozilla.org/r/87778/#review86812 stamp! FYSA, we don't want to force GV consumers to be appcompat-v7 consumers. So this is great! FYI, I'm on leave until January. mcomella and sebastian are good reviewers for build stuff, jchen and snorp for build and GV stuff.

Did you talk to Bryan and/or Antlam about this? Afaik they have been meeting in Toronto last week and also talked about design/icon consistency.

Comment on attachment 8803545 [details] Bug 1312114 - Support VectorDrawable's in ResourceDrawableUtils https://reviewboard.mozilla.org/r/87780/#review86962 ::: mobile/android/base/java/org/mozilla/gecko/util/ResourceDrawableUtils.java:118 (Diff revision 1) > } > > if (data.startsWith("drawable://")) { > final Uri imageUri = Uri.parse(data); > final int id = getResource(context, imageUri); > - final Drawable d = context.getResources().getDrawable(id); > + final Drawable d = AppCompatDrawableManager.get().getDrawable(context, id); It seems like this class is hidden in later releases of the support library. Hopefully there's a different approach we can use later? https://android.googlesource.com/platform/frameworks/support/+/master/v7/appcompat/src/android/support/v7/widget/AppCompatDrawableManager.java#64

Comment on attachment 8803546 [details] Bug 1312114 - Add VectorDrawable support to GeckoMenu https://reviewboard.mozilla.org/r/87782/#review86964

Comment on attachment 8803547 [details] Bug 1312114 - Post: make catch more specific to not hide exceptions https://reviewboard.mozilla.org/r/87784/#review86966 r+++++

(In reply to Sebastian Kaspari (:sebastian) from comment #9) > Comment on attachment 8803545 [details] > Bug 1312114 - Support VectorDrawable's in ResourceDrawableUtils [...] > > + final Drawable d = AppCompatDrawableManager.get().getDrawable(context, id); > > It seems like this class is hidden in later releases of the support library. > > Hopefully there's a different approach we can use later? > > https://android.googlesource.com/platform/frameworks/support/+/master/v7/ > appcompat/src/android/support/v7/widget/AppCompatDrawableManager.java#64 I've been spending some time digging into this, but am not really sure what we want to do here: - Support library 24.2.0 introduces AppCompatResources.getDrawable(): https://developer.android.com/topic/libraries/support-library/revisions.html#24-2-0-api-updates API docs: https://developer.android.com/reference/android/support/v7/content/res/AppCompatResources.html That means we at least have a solution once we're on >= 24.2. (Note: I haven't actually tested this, so maybe there are unknown issues lurking there...) Which leaves 23.4.0: I can't actually find older versions of AppCompatDrawableManager, and it's entirely possible that it's hidden even on 23.4.0. (I.e. on androidxref, I can only find AppCompatDrawableManager when searching in 7.0, but not in 6.0 or earlier - I'm going to see if I can just find the upstream support library repo.) However it does work without issues on the devices I've tested, and it's also what the various AppCompat widgets are using (e.g. AppCompatImageView). So if we stick to the same API, then we *might* still be ok with using it for 23.4.0. I don't feel particularly comfortable doing that though, so I'm going to try and do some more research and testing...

I wonder if loading the drawables using VectorDrawableCompat.create() would be a better idea for now. That would however require checking the type of drawable before inflation and/or trying normal inflation, and falling back if normal inflation throws an Exception because of a <vector>. (Which is essentially what AppCompatDrawableManager does: it iterates over various load-delegates, and finally tries the default Resoureces.getDrawable() if no other method succeeds.)

(In reply to Andrzej Hunt :ahunt from comment #12) > That means we at least have a solution once we're on >= 24.2. (Note: I > haven't actually tested this, so maybe there are unknown issues lurking > there...) Sounds good! As soon as we are building with SDK 24/25 we can switch to the latest support library. > Which leaves 23.4.0: I can't actually find older versions of > AppCompatDrawableManager, and it's entirely possible that it's hidden even > on 23.4.0. (I.e. on androidxref, I can only find AppCompatDrawableManager > when searching in 7.0, but not in 6.0 or earlier - I'm going to see if I can > just find the upstream support library repo.) > > However it does work without issues on the devices I've tested, and it's > also what the various AppCompat widgets are using (e.g. AppCompatImageView). > So if we stick to the same API, then we *might* still be ok with using it > for 23.4.0. I don't feel particularly comfortable doing that though, so I'm > going to try and do some more research and testing... 23.4 is what we are using. If it compiles then it shouldn't be hidden and we can just use it for now. I was just wondering if there's something we can switch to later (24.x / 25.x). :)

I've filed Bug 1314902 to do the actual drawable replacement, I'm going to focus on landing the code changes needed for VectorDrawable support in this bug.

Comment on attachment 8803549 [details] Bug 1312114 - Post: centralise AppCompatDrawableManager calls to allow easy replacement https://reviewboard.mozilla.org/r/87788/#review90066

Comment on attachment 8803548 [details] Bug 1312114 - Post: cleanup imports https://reviewboard.mozilla.org/r/87786/#review90146

Pushed by ahunt@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9cf52476e2aa Pre: Move getDrawable() from (geckoview) BitmapUtils into (fennec) MenuBitmapUtils to allow support library access r=nalexander https://hg.mozilla.org/integration/autoland/rev/23668d4083a4 Support VectorDrawable's in ResourceDrawableUtils r=sebastian https://hg.mozilla.org/integration/autoland/rev/4c5989da040c Add VectorDrawable support to GeckoMenu r=sebastian https://hg.mozilla.org/integration/autoland/rev/93f705ff5404 Post: make catch more specific to not hide exceptions r=sebastian https://hg.mozilla.org/integration/autoland/rev/07ebcbebe013 Post: centralise AppCompatDrawableManager calls to allow easy replacement r=sebastian https://hg.mozilla.org/integration/autoland/rev/4be064a14b66 Post: cleanup imports r=ahunt

https://hg.mozilla.org/mozilla-central/rev/9cf52476e2aa https://hg.mozilla.org/mozilla-central/rev/23668d4083a4 https://hg.mozilla.org/mozilla-central/rev/4c5989da040c https://hg.mozilla.org/mozilla-central/rev/93f705ff5404 https://hg.mozilla.org/mozilla-central/rev/07ebcbebe013 https://hg.mozilla.org/mozilla-central/rev/4be064a14b66