832376 - The default icon doesn't appear when manifest has an invalid icon URL - stays in "downloading" mode

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Homescreen, defect)

Description

[Julien Wajsberg [:julienw]]

STR:
* install app in http://everlong.org/mozilla/brokenhostedappcache/
* wait until it is installed
* go to the homescreen

Expected:
* default app icon is added for this application

Actual:
* spinning downloading icon is still present

Useful log:
E/GeckoConsole(  616): [JavaScript Error: "NS_ERROR_INVALID_POINTER: Component returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER) [nsIDOMMozURLProperty.createObjectURL]" {file: "app://homescreen.gaiamobile.org/js/page.js" line: 288}]

Nominating because this _will_ happen.

Comment 1

[Julien Wajsberg [:julienw]]

The same happens with app at http://everlong.org/mozilla/brokenpackaged/.

Comment 2

[Julien Wajsberg [:julienw]]

As said in the title of this bug, the problem here is that the icon URL in the manifest points to an URL that does not exist.

Comment 3

[Julien Wajsberg [:julienw]]

Also, after restarting the phone, there is no icon _at all_ for these apps.

See screenshot, the icon at the bottom right. So the user can't even uninstall the app (maybe by the Settings app but this is bad).

Comment 4

[Julien Wajsberg [:julienw]]

Attachment: screenshot

Comment 5

[Jason Smith [:jsmith]]

Well, this looks bad. I can reproduce.

Looks like the app is launchable in the spinning state. After a restart, there's no icon there at all! It becomes hard to launch too - you need to hit the label.

It's possible to uninstall the app still.

From the marketplace side, the validator would help check this periodically.

Comment 6

[Julien Wajsberg [:julienw]]

Jason> I couldn't uninstall the app, at least from the homescreen.

This is also a regression from Bug 818417... I already fixed this bug once, I can do it another time ;)

Comment 7

[Julien Wajsberg [:julienw]]

BTW, in Bug 818417 I spent some times and added 7 tests for this specific error. They don't run at all now. Really this is not right...

Comment 8

[Julien Wajsberg [:julienw]]

CCing Dale as it seems to be because of fix to Bug 828888.

Comment 9

[Julien Wajsberg [:julienw]]

Attachment: patch v1

- fix homescreen tests, had to change the TextOverflowDetective for that
- Don't use the stored old rendered icon if it's not a proper blob.
---
 apps/homescreen/js/page.js             |   32 +++++++++++------
 apps/homescreen/test/unit/page_test.js |   62 +++++++++++++++++++++++++++++++-
 2 files changed, 83 insertions(+), 11 deletions(-)

Comment 10

[Julien Wajsberg [:julienw]]

Note for drivers> this must absolutely land in v1.0.0.

Comment 11

[Julien Wajsberg [:julienw]]

Comment on attachment 704887 [details] [diff] [review]
patch v1

ask for review from crdlc instead of Dale as he's busy this week.

Comment 12

[Julien Wajsberg [:julienw]]

Comment on attachment 704887 [details] [diff] [review]
patch v1

Dale seems to be there after all :)

Comment 13

[Dale Harvey (:daleharvey)]

Comment on attachment 704887 [details] [diff] [review]
patch v1

Small patch, I reproduced manually and the patch worked, tests also passing

Comment 14

[Julien Wajsberg [:julienw]]

https://github.com/mozilla-b2g/gaia/commit/a9f8b47f70c1fba2bdf83d2f9c821e175ff1231c

Comment 15

[Julien Wajsberg [:julienw]]

it's not landed on v1.0.0 yet, as I'm not sure if we should land this ourself or if we should wait for someone to do that, per vivien's advice.

Comment 16

[Cristian Rodriguez (:crdlc)]

sorry Julien, I was on vacations and I couldn't review

Comment 17

[Julien Wajsberg [:julienw]]

crdlc> np :) you should change your bugzilla's nickname to reflect that, next time !

Comment 18

[John Shih]

wait for landed

Comment 19

[Jason Smith [:jsmith]]

Verified on 1/25 build.

Comment 20

[Lukas Blakk [:lsblakk] use ?needinfo]

Marking status-b2g18-v1.0.0 fixed based on Vivien's drive-by fixing on status-b2g18 fix, assuming this has been merged to v1.0.0 branch.