Closed Bug 1194205 Opened 6 years ago Closed 11 months ago

Remove remaining mdpi assets

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: mhaigh, Unassigned)

References

Details

Attachments

(1 file)

In bug 1171946 comment 19 we discovered that there are still a few mdpi assets left [1].  Let's remove these aside from the xml files added in bug 1179758


1, http://mxr.mozilla.org/mozilla-central/find?text=&string=mdpi
Of the remaining assets, only this one doesn't have a hdpi equivalent in the codebase:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/crashreporter/res/drawable-mdpi/crash_reporter.png

antlam: Do you have the means to create a new hdpi asset for this?
Flags: needinfo?(alam)
I don't think I've seen this before actually... Can you point me to where this actually shows itself in our UI? Maybe I can offer an alternative - it seems dated anyways.
Flags: needinfo?(alam) → needinfo?(mhaigh)
It was part of the crash reporter UI. Though on Android 5.0 and 4.2 I am not seeing it. I don't have a reliable Android 2.3 crasher at the moment to check if the icon is used there.
Assignee: mhaigh → michael.l.comella
Flags: needinfo?(mhaigh)
NI-ing Sebastian here to see if he has any thoughts on this now :)
Flags: needinfo?(s.kaspari)
afaik, what still needs to be done is find out if this icon is used and if so, see how reasonable it is to remove.

To do this, we need to trigger the crash reporter on 2.3. The best way to do this is to have a dev write an explicit Intent to open the crash reporter and take a look at it.
I can explicitly open the crash reporter through intents but because we can't build Crash Reporter on OS X, I can't do this locally. I'm going to push to try to get builds to test this (e.g. comment 6).
The build in comment 9 works on API 11+ – now I need to find a 2.3 device to test on...
Attached image 2.3 crash reporter
Icon looks unused to me!
Looks like the icon should be unused – it's only used as the application icon [1]! Oops!

afaik, we can remove that because we don't show it anywhere (e.g. launcher, recent app switcher).

[1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/AndroidManifest.xml.in#404
We don't ship all of the icons Martyn declared in comment 0 – I think it's:
  * crash_reporter
  * mobile/android/themes/core/images/*

The latter is declared in jars (e.g. [1]) but I'm not sure how it's read into memory so I'll have to do some research on that before I just remove the assets (e.g. maybe it looks for mdpi if the device is mdpi configuration and fails to provide an icon if there's no mdpi asset).

We probably shouldn't remove the other icons for fear of breaking those projects (e.g. background tests, b2gdroid, sutagent), some of which have ldpi too. :P

[1]: https://mxr.mozilla.org/mozilla-central/search?string=reader-style-icon&find=mobile%2Fandroid&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central
Clearing NI: It looks like you know what you are doing? :)
Flags: needinfo?(s.kaspari)
I'm not going to get to this anytime soon.
Assignee: michael.l.comella → nobody
We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → INCOMPLETE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.