Closed
Bug 1041295
Opened 10 years ago
Closed 10 years ago
[Collection App] Icons are not updated after installing packaged apps from marketplace
Categories
(Firefox OS Graveyard :: Gaia::Everything.me, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: amirn, Assigned: amirn)
References
Details
Attachments
(1 file, 2 obsolete files)
STR:
0) reset your device
1) Go to Marketplace > Games
2) Install 'Cut the Rope'
3) Go to vertical home after installing this game
4) Observe the 'Games' icon
Expected:
'Games' icon should show 'cut the rope' as the middle app icon
Actual:
'Games' show an blank icon as the middle app icon
More information:
For packaged apps, the app icon is a local asset downloaded from the web.
The 'install' event fires too early:
https://github.com/mozilla-b2g/gaia/blob/master/apps/collection/js/synchronize.js#L33
at this point the app hasn't finished downloading and retrieving the icon fails.
Suggested fix:
The event should be fired only when the app after download completes successfully.
Assignee | ||
Updated•10 years ago
|
Blocks: vertical-home-next
Jacqueline, would UX block on that bug?
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking?]
Flags: needinfo?(jsavory)
Assignee | ||
Comment 2•10 years ago
|
||
this patch defers the 'install' event to after 'dowloadapplied'. 2 things are not working at the moment:
1. 'downloadapplied' is not fired for hosted apps, so the 'install' event isn't triggered as well. not sure what the right hook for hosted apps should be.
2. homescreen is not updated when the new icon is saved to the CollectionsDatabase and still shows the old icon
Flags: needinfo?(kgrandon)
Comment 3•10 years ago
|
||
(In reply to Amir Nissim (:amirn) from comment #2)
> Created attachment 8460232 [details]
> WIP
>
> this patch defers the 'install' event to after 'dowloadapplied'. 2 things
> are not working at the moment:
>
> 1. 'downloadapplied' is not fired for hosted apps, so the 'install' event
> isn't triggered as well. not sure what the right hook for hosted apps should
> be.
To be honest, I'm not sure what to do here. I can't really think of anything better right now than messaging the collection app on each event.
> 2. homescreen is not updated when the new icon is saved to the
> CollectionsDatabase and still shows the old icon
This should totally work because we should get the datastore sync event. Cristian recently did some work to make homescreen launch faster by caching icons - I wonder if we need to manually bust the icon cache =/
Flags: needinfo?(kgrandon)
Assignee | ||
Comment 4•10 years ago
|
||
(In reply to Kevin Grandon :kgrandon from comment #3)
> To be honest, I'm not sure what to do here. I can't really think of anything
> better right now than messaging the collection app on each event.
The problem with this approach is that 'install' fires for both packaged and hosted app and I don't know how to tell which type is it. For hosted we should handle it, but for packaged we should ignore it and wait for 'downloadapplied'. Who can give us some mozApps advice?
> This should totally work because we should get the datastore sync event.
> Cristian recently did some work to make homescreen launch faster by caching
> icons - I wonder if we need to manually bust the icon cache =/
This explains why I saw the updated icon after restarting the device. Crisitian, can you help?
Flags: needinfo?(crdlc)
Comment 5•10 years ago
|
||
(In reply to Amir Nissim (:amirn) from comment #4)
> (In reply to Kevin Grandon :kgrandon from comment #3)
> The problem with this approach is that 'install' fires for both packaged and
> hosted app and I don't know how to tell which type is it. For hosted we
> should handle it, but for packaged we should ignore it and wait for
> 'downloadapplied'. Who can give us some mozApps advice?
I'll look into to see what else we can use. What's wrong with calling this twice for a packaged app? Will bad things happen? I know it's not ideal, but it may be an option while we continue to look for a solution. Also maybe check and see what we did in v1 that was different here?
> This explains why I saw the updated icon after restarting the device.
> Crisitian, can you help?
Can we simply delete the decoratedIconBlob property of icon.detail and call render? We should be able to do this in the 'update' datastore sync method.
Comment 6•10 years ago
|
||
I'm going mark this blocking- because I think having the collection icon not fully correct is a minor issue post installation of an app, as it does not cause any direct user impact (only visual impact). If somebody else disagrees, then feel free to flag down & indicate why.
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking?] → [VH-FL-blocking-][VH-FC-blocking-]
Comment 7•10 years ago
|
||
To be honest I am lost here about what you want from me here. I've installed "Cut The Rope" from marketplace and it is displayed properly on the vertical homescreen. According to bug's title the collection icon is not updated. Is it because of cached icon? I can see how the flow does not go through cache code when the icon should be updated
https://github.com/mozilla-b2g/gaia/blob/master/apps/verticalhome/js/sources/collection.js#L121
https://github.com/mozilla-b2g/gaia/blob/master/apps/verticalhome/js/sources/collection.js#L156
https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_grid/js/items/collection.js#L71
https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_grid/js/items/grid_item.js#L578
https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_grid/js/items/grid_item.js#L385
Flags: needinfo?(crdlc)
Comment 8•10 years ago
|
||
This patch calls to collection app when the download is applied but when we receive the update of the collection the icon is the same than the already displayed. See logs in the patch
Assignee | ||
Comment 9•10 years ago
|
||
Thanks for your input Cristian.
It took a lot of digging around, but I finally found the problem:
on some cases, HomeIcons.init was not triggered after new app installs, and it's app index was out of sync.
I think this happened when the collection app was still in memory, and a new 'install' event didn't really trigger a fresh launch.
The attached patch fixes that by forcing index re-building on 'install' and 'uninstall'.
Attachment #8460232 -
Attachment is obsolete: true
Attachment #8460766 -
Attachment is obsolete: true
Attachment #8460874 -
Flags: review?(crdlc)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → amirn
Comment 10•10 years ago
|
||
Comment on attachment 8460874 [details]
Pull Request
LGTM, just a minor comment on github, thanks a lot Amir
Attachment #8460874 -
Flags: review?(crdlc) → review+
Assignee | ||
Comment 11•10 years ago
|
||
fixed and landed. thanks :)
master: https://github.com/mozilla-b2g/gaia/commit/44162e0
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 12•10 years ago
|
||
I agree that this one is not a blocker, removing flag.
Flags: needinfo?(jsavory)
You need to log in
before you can comment on or make changes to this bug.
Description
•