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)

x86
macOS
defect
Not set
normal

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.
Depends on: 1038578
Jacqueline, would UX block on that bug?
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking?]
Flags: needinfo?(jsavory)
Attached file WIP (obsolete) (deleted) —
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)
(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)
(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)
(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.
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-]
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)
Attached patch 1041295.patch (obsolete) (deleted) — Splinter Review
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
Attached file Pull Request (deleted) —
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: nobody → amirn
Comment on attachment 8460874 [details] Pull Request LGTM, just a minor comment on github, thanks a lot Amir
Attachment #8460874 - Flags: review?(crdlc) → review+
fixed and landed. thanks :) master: https://github.com/mozilla-b2g/gaia/commit/44162e0
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
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.

Attachment

General

Created:
Updated:
Size: