Closed
Bug 1040443
Opened 10 years ago
Closed 10 years ago
[Lint] Found bitmap drawable res/drawable/phone.png in densityless folder
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 34
People
(Reporter: rnewman, Assigned: aymen, Mentored)
References
Details
(Whiteboard: lint, [good first bug])
Attachments
(1 file, 1 obsolete file)
392 bytes,
patch
|
rnewman
:
review+
|
Details | Diff | Splinter Review |
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•10 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•10 years ago
|
||
Assignee | ||
Comment 3•10 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 4•10 years ago
|
||
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•10 years ago
|
||
Sorry, used HG move this time.
Attachment #8461552 -
Attachment is obsolete: true
Attachment #8461612 -
Flags: review?(blassey.bugs)
Reporter | ||
Comment 6•10 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•10 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)
Comment 8•10 years ago
|
||
(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•10 years ago
|
||
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•10 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 → rnewman
Status: NEW → ASSIGNED
Reporter | ||
Comment 11•10 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•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/9b0b33938579
Reporter | ||
Comment 13•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/16c16c68ed5f
Reporter | ||
Comment 14•10 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
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•