If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED in Firefox 34

Status

()

Firefox for Android
General
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: rnewman, Assigned: Aymen Qader, Mentored)

Tracking

Trunk
Firefox 34
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: lint, [good first bug])

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

3 years ago
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]
(Assignee)

Comment 1

3 years ago
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.
(Assignee)

Comment 2

3 years ago
Created attachment 8461552 [details] [diff] [review]
1040443_1.patch Moved phone.png to resolution-specific drawable-mdpi directory
(Assignee)

Comment 3

3 years ago
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.
Attachment #8461552 - Flags: review?(blassey.bugs)
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
Attachment #8461552 - Flags: review?(blassey.bugs)
Attachment #8461552 - Flags: review-
Attachment #8461552 - Flags: feedback+
(Assignee)

Comment 5

3 years ago
Created attachment 8461612 [details] [diff] [review]
1040443_2.patch

Sorry, used HG move this time.
Attachment #8461552 - Attachment is obsolete: true
Attachment #8461612 - Flags: review?(blassey.bugs)
(Reporter)

Comment 6

3 years ago
(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",
(Assignee)

Comment 7

3 years ago
(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)
(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 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
Attachment #8461612 - Flags: review?(blassey.bugs) → review?(rnewman)
(Reporter)

Comment 10

3 years ago
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.
Assignee: nobody → aymen
Mentor: blassey.bugs@lassey.us → rnewman@mozilla.com
Status: NEW → ASSIGNED
(Reporter)

Comment 11

3 years ago
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.
Attachment #8461612 - Flags: review?(rnewman) → review+
(Reporter)

Comment 12

3 years ago
https://hg.mozilla.org/integration/fx-team/rev/9b0b33938579
(Reporter)

Comment 13

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/16c16c68ed5f
(Reporter)

Comment 14

3 years ago
Ignore the fx-team landing; the tree was closed, but my hg extension posted to Bugzilla regardless.
https://hg.mozilla.org/mozilla-central/rev/16c16c68ed5f
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
You need to log in before you can comment on or make changes to this bug.