1040443 - [Lint] Found bitmap drawable res/drawable/phone.png in densityless folder

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Richard Newman [:rnewman]]

Found bitmap drawable res/drawable/phone.png in densityless folder

Issue: Ensures that images are not defined in the density-independent drawable folder
Id: IconLocation

The res/drawable folder is intended for density-independent graphics such as shapes defined in XML. For bitmaps, move it to drawable-mdpi and consider providing higher and lower resolution versions in drawable-ldpi, drawable-hdpi and drawable-xhdpi. If the icon really is density independent (for example a solid color) you can place it in drawable-nodpi.

[http://developer.android.com/guide/practices/screens_support.html]

Comment 1

[Aymen Qader]

I'll work on this - looks very simple, especially since I've had lots of android experience in the past.

I just need to finish setting up my development environment.

Comment 2

[Aymen Qader]

Attachment: 1040443_1.patch Moved phone.png to resolution-specific drawable-mdpi directory

Comment 3

[Aymen Qader]

Comment on attachment 8461552 [details] [diff] [review]
1040443_1.patch Moved phone.png to resolution-specific drawable-mdpi directory

This drawable isn't actually used.

Comment 4

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

Comment on attachment 8461552 [details] [diff] [review]
1040443_1.patch Moved phone.png to resolution-specific drawable-mdpi directory

if you're moving a file, use "hg move" rather than just moving it on the file system

Comment 5

[Aymen Qader]

Attachment: 1040443_2.patch

Sorry, used HG move this time.

Comment 6

[Richard Newman [:rnewman]]

(In reply to Aymen Qader from comment #3)

> This drawable isn't actually used.

It is:

mobile/android/chrome/content/SelectionHandler.js
641:      icon: "drawable://phone",

Comment 7

[Aymen Qader]

(In reply to Richard Newman [:rnewman] from comment #6)
> (In reply to Aymen Qader from comment #3)
> 
> > This drawable isn't actually used.
> 
> It is:
> 
> mobile/android/chrome/content/SelectionHandler.js
> 641:      icon: "drawable://phone",

Apologies, I was told that it wasn't used (and I couldn't find it using MXR)

Comment 8

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

(In reply to Aymen Qader from comment #7)
> (In reply to Richard Newman [:rnewman] from comment #6)
> > (In reply to Aymen Qader from comment #3)
> > 
> > > This drawable isn't actually used.
> > 
> > It is:
> > 
> > mobile/android/chrome/content/SelectionHandler.js
> > 641:      icon: "drawable://phone",
> 
> Apologies, I was told that it wasn't used (and I couldn't find it using MXR)

That was misinformation from me. Sorry. Using Java resources from JS makes them hard to find, especially with names like "phone".

Comment 9

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

Comment on attachment 8461612 [details] [diff] [review]
1040443_2.patch

moving the review to rnewman. I don't know how we handle dpi-dependant icons from js

Comment 10

[Richard Newman [:rnewman]]

It *should* be seamless, because we do this already, delegating to the usual Android search mechanism. E.g.,

mobile/android/chrome/content/browser.js
569:      icon: "drawable://ic_menu_share",

mobile/android/base/resources/drawable-hdpi/ic_menu_share.png
mobile/android/base/resources/drawable-mdpi/ic_menu_share.png
mobile/android/base/resources/drawable-xhdpi/ic_menu_share.png

A quick scan suggests that this is the code that does the processing:

        if (data.startsWith("drawable://")) {
            final Uri imageUri = Uri.parse(data);
            final int id = getResource(imageUri, R.drawable.ic_status_logo);
            final Drawable d = context.getResources().getDrawable(id);

            runOnBitmapFoundOnUiThread(loader, d);
            return;
        }

so it should do the right thing. Needs testing to verify.

Comment 11

[Richard Newman [:rnewman]]

Comment on attachment 8461612 [details] [diff] [review]
1040443_2.patch

Tested locally on an xhdpi device. I'll land this with a correct commit message and User tag.

Comment 12

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/fx-team/rev/9b0b33938579

Comment 13

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/16c16c68ed5f

Comment 14

[Richard Newman [:rnewman]]

Ignore the fx-team landing; the tree was closed, but my hg extension posted to Bugzilla regardless.

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/16c16c68ed5f