1268019 - Incorrect icon is displayed for files in Download Manager

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Download Manager, defect, P1)

Description

[Teodora Vermesan (:TeoVermesan)]

Attachment: Screenshot_20160427-160606.png

Tested using:
Device: Sony Xperia Z2 Tablet (Android 5.0.2) and Nexus 9 (Android 6.0)
Steps to reproduce:
1. Open each of the links and download the files:
http://1.usa.gov/deeXKM
http://goo.gl/wC8xy
http://goo.gl/bl0U
2. Open the Download Manager

Expected results:
- Each file has the correct icon of the app associated with the file

Actual results:
- Incorrect icon is displayed.
- Please see the attached screenshot

Comment 1

[Firefox Bug Husbandry Bot]

Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.

Comment 2

[Brad Arant]

Issue is created by the fact that there is no registered intents for the extension and the system provisions to provide. The existing algorithm will retrieve the list of intent activities and use the first one in the list and retrieve the icon parameters from the intent.

If there are no registered intent activities to support the extension and its related mime type then the list is empty and the algorithm to retrieve the Drawable icon is handed null. In my recreations of the issue I see no icon and have seen grey as well. In either case the Drawable object is invalid and therefore un-displayable.

How should we handle this condition?

1. We can provide a default generic icon that is displayed for items that do not have an intent activity and return it instead of null on resolution errors leading up to the icon.

2. We can provide some additional lookups for commonly used extensions and offer better and more specific icons. Its just a matter of definition. An internal table of drawables to return based upon file extensions to be used as default drawable when no intent activities are registered for the extension type.

Perhaps another suggestion?

Comment 3

[Brad Arant]

Attachment: Generic File Icon

Please let me know if this is acceptable as a default icon.

Comment 4

[Brad Arant]

Submitting patch for providing a default icon for files that do not have an intent associated with their extensions.

Comment 5

[Brad Arant]

Attachment: about:Download with default icon for files that have no supporting intent app.

Shows icon on two downloaded files that have no associated app to handle them.

Comment 6

[Brad Arant]

Attachment: Bug 1268019 - Display default icon for files in Download Manager with no default app.;r?VladBaicu

Comment 7

[Brad Arant]

Attachment: Bug 1268019 - Fix code violation of missing braces on if code block.;r?VladBaicu

Comment 8

[Brad Arant]

Attachment: Bug 1268019 - Return default icon for download files that do not have intent for handling.;r?VladBaicu

Depends on D34386

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/572c808b302b
Fix code violation of missing braces on if code block.;r=VladBaicu

Comment 10

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/572c808b302b

Comment 11

[Chris Peterson [:cpeterson]]

[Tracking Requested - why for this release]:

We'll want to uplift this UI fix to Fennec 68, but we can wait until after 68 ESR is branched and ship it in Fennec 68.1.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Should this bug have been closed yet? Looks only one of the patches has landed at this point.

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

Changing the priority to p1 as the bug is tracked by a release manager for the current beta.
See What Do You Triage for more information

Comment 14

[Chris Peterson [:cpeterson]]

Brad, it looks like your code violation fix (D34386 in comment 7) landed, but not the default icon fixes themselves (D34220 in comment 6 and D34387 in comment 8). It's probably too late to uplift fixes to Fennec 68 Beta. After your fixes landed in mozilla-central, we can uplift them to Fennec ESR channel for the ESR 68.1 release (2019-09-03).

Comment 15

[Brad Arant]

Please apply D34387. Only first patch applied. After merge I will request Fennec ESR approval.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/76b7becd27ac
Return default icon for download files that do not have intent for handling.;r=VladBaicu

Comment 17

[Oana Pop-Rus]

Backed out changeset 76b7becd27ac (bug 1268019) for checkstyle build bustages in GeckoAppShell.java

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=253674561&resultStatus=superseded%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&revision=76b7becd27ac4c52e6ce2a4530804cae3def5867

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=253674561&repo=autoland&lineNumber=3063

Backout: https://hg.mozilla.org/integration/autoland/rev/482000973b027f93c360f159fdce52cea0ca39bb

Comment 18

[Brad Arant]

Working n the patch to remove extraneous reference.

Comment 19

[Andrei Lazar]

This has been resolved 2 weeks ago, but we haven't get to put the flag. Thanks!

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/56410364cab0
Return default icon for download files that do not have intent for handling.;r=VladBaicu

Comment 21

[Chris Peterson [:cpeterson]]

We'll want to uplift this fix to Fennec 69 Beta and ESR 68.1. We don't need this fix in a ESR 68.0.x dot release.

Comment 22

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/56410364cab0

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta & ESR68 approval when you get a chance.

Comment 24

[Stefan Deiac]

Hello, I can confirm that the issue is fixed on the latest version of Nightly from treeherder (https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&selectedJob=257089020&searchStr=android&revision=c05f610525761ddb7f8bed9927f9f9c45bcb507e).
Devices:

• Motorola Moto G6 (Android 8);
• Samsung Galaxy S8 (Android 9);
• Google Pixel (Android Q);

Comment 25

[Brad Arant]

Comment on attachment 9071042 [details]
Bug 1268019 - Return default icon for download files that do not have intent for handling.;r?VladBaicu

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Requested
• User impact if declined: No icons appear when no intent is there to provide one. The bug.
• Fix Landed on Version:
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Provides a default icon where a NULL value was previously retrieved.
• String or UUID changes made by this patch: None

Comment 26

[Brad Arant]

Comment on attachment 9071042 [details]
Bug 1268019 - Return default icon for download files that do not have intent for handling.;r?VladBaicu

Beta/Release Uplift Approval Request

• User impact if declined: Icon will not appear for items not having an intent. The bug.
• Is this code covered by automated tests?: Unknown
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Provides a default drawable where NULL was formerly returned.
• String changes made/needed: none

Comment 27

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9071041 [details]
Bug 1268019 - Fix code violation of missing braces on if code block.;r?VladBaicu

Fixes missing icons in the Fennec download manager. Note that the first patch landed on m-c when it was still tracking 69 and doesn't need uplift to Beta. Approved for Fennec 68.1b3.

Comment 28

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9071041 [details]
Bug 1268019 - Fix code violation of missing braces on if code block.;r?VladBaicu

As I said before, this patch is already on Beta based on when it landed on m-c.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8d8802147a78

Comment 30

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-esr68/rev/4600a8eb6de2
https://hg.mozilla.org/releases/mozilla-esr68/rev/aa83ff02de69

Comment 31

[Eliza Balazs]

Hi!
Verified as fixed on ESR 68.1b3, Beta 69.0b7 with OnePlus 5T (Android 9), HTC 10 (Android 8.0.0).
I will mark this issue as verified on Firefox esr68 and Firefox 69. Thanks!