1. for predefined collections 2. on first init 3. on app installs 4. on app updates (icon, locale) 5. on app un-installs

I'd love to get some feedback here (still WIP). Specifically, when pinning installed bookmarks/apps to collections - what should we pin? I would prefer saving only the bookmarkURL/manifestURL and fetch any other data when we need to render it (when viewing a collection). Note that when we implemented pinning web results there was no choice but to save all the data because it isn't stored anywhere else.

(In reply to Amir Nissim (Everything.me) from comment #1) > I would prefer saving only the bookmarkURL/manifestURL and fetch any other > data when we need to render it (when viewing a collection). I think we can do this either by IAC, or by creating a new database where we would store only the apps displayed on the homescreen. I prefer the latter, we would have bookmarks_store, collections_store and apps_store. This will allow the collections app to utilize all the processing that is done by the homescreen (processing entry points, hidden roles, locales, icons etc.) Kevin, what do you think?

(In reply to Amir Nissim (Everything.me) from comment #2)> > apps_store. This will allow the collections app to utilize all the > processing that is done by the homescreen (processing entry points, hidden > roles, locales, icons etc.) It's interesting and something I thought about before. Entry points are super painful, and we are looking at removing them in the future. Locales are also painful. Datastore may be a way to resolve some of these painpoints, or potentially using a small lightweight library in each application. I will look at the patch shortly, but I think my preference for now would be to store the minimum, and fetch the data necessary. In the homescreen we only store the detail object for each grid item in indexedDB, and that's enough to get the data that we need. We have some small libraries in the homescreen which links the two up (verticalhome/js/sources/*). Perhaps we can move these into the component, or into shared.

Comment on attachment 8436938 [details] WIP Overall feedback plus. I will take a deeper look if you mark me for R?. Thanks!

steps to test: 0. reset device 1. add music collection 2. open music collection - triggers the population task (aka native_info) 3. no pinned apps shown yet cause the collection was rendered before the task completed 4. close the music collection and open it again (task does not run again) 5. 3 pinned apps appear in the music folder :) known bug: when pinning a webresult to the collection, the previously pinned apps are not shown until the collection is closed and opened again note: I had to pass a homeIcons object to every |collection.render| call since it now renders homescreen icons. I don't like it, but can't think of any other way of doing this without a homescreen_database. please take a look at the PR and let me know what you think. Thanks.

Comment on attachment 8438537 [details] PR Overall I think this is looking pretty good. I would like to see another version before R+. Also the first time I installed I noticed some apps appear in a smart collection (with some bugs), but cool! But then I couldn't get it to replicate again. Nice work!

Comment on attachment 8438537 [details] PR Clearing r till PR is updated

Comment on attachment 8438537 [details] PR updated PR. see comment #5 on how to test this. (In reply to Amir Nissim (Everything.me) from comment #5) > known bug: > when pinning a webresult to the collection, the previously pinned homeIcons are > not shown until the collection is closed and opened again Kevin, I would love some help with this. I have no idea what's going. I know the PR is not perfect but it is already quite big so I think we can land it and file followups for missing functionality and bugfixes. Thanks

Comment on attachment 8438537 [details] PR Forwarding review to James.

Amir, a bug I found: 1. Add Utilities to homescreen. (or any SM that includes mozApps) 2. Click on it. Expected: displays mozApps. Actual: Doesn't but will the next time it's opened. This may be the same as the comment #8 bug

(In reply to Ran Ben Aharon (Everything.me) from comment #10) > Amir, a bug I found: > 1. Add Utilities to homescreen. (or any SM that includes mozApps) > 2. Click on it. > > Expected: displays mozApps. > Actual: Doesn't but will the next time it's opened. > > This may be the same as the comment #8 bug That's not a bug, it's a feature :) see comment 5

I am fairly sure that this is because we simply call setup() without waiting for any pinning. I would recommend either re-rendering after we've done the setup or change the initialization to something like: NativeInfo.setup().then(function() { HandleView(activity) });

Comment on attachment 8438537 [details] PR Clearing the review on James until it's working fully.

Ok, I think I found the problem. I think this is due to the fact that even if we call clear, existing grid items hold onto their element references, even though they have been removed from the dom: https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_grid/js/items/grid_item.js A quick fix for this from the collection app would be to create a new GaiaGrid.Mozapp() object, but I will also investigate if there is an easy fix from the gaia grid.

Sorry, meant to link to this line: https://github.com/mozilla-b2g/gaia/blob/master/shared/elements/gaia_grid/js/items/grid_item.js#L306

This should fix 'mozapps only visible on 2nd open' by running the population task upon creation of new collections: https://github.com/EverythingMe/gaia/commit/d0aab2926eae2bd56114ff4958053bf7c1ae51f0 sadly it does not work due to [JavaScript Error: "DataCloneError: The object could not be cloned."] anyone has an idea what this is about?

(In reply to Amir Nissim (Everything.me) from comment #16) > This should fix 'mozapps only visible on 2nd open' by running the population > task upon creation of new collections: > https://github.com/EverythingMe/gaia/commit/ > d0aab2926eae2bd56114ff4958053bf7c1ae51f0 deferring to bug 1027003. I think this patch is already too big.

It does feel a bit off to land this patch without apps being displayed on the first view. I do see we have a patch in the other bug that could address this though, so if we land that other one quickly maybe it's not too bad.

(In reply to Kevin Grandon :kgrandon from comment #18) > It does feel a bit off to land this patch without apps being displayed on > the first view. I do see we have a patch in the other bug that could address > this though, so if we land that other one quickly maybe it's not too bad. squashed PR for both bugs and closed 1027003 as duplicate of this one. waiting for Travis/TBPL..

landed in master: https://github.com/mozilla-b2g/gaia/commit/75ecfdd5dc3882a95cff6ff8f2bf4ce9db823d64 tbpl: https://tbpl.mozilla.org/?tree=Gaia-Try&rev=0f76c48edacd3ece333be4e033d952bd030c7699

Comment on attachment 8438537 [details] PR This is needed for the vertical homescreen.

2.0: https://github.com/mozilla-b2g/gaia/commit/9a50c4b12f3c26891b34188befa3a05ca4777d48

Verified with dependent bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1027005#c20

Verified the bug is fixed on 2.2, 2.1 and 2.0 Collection is populated with install apps Device: Flame 2.2 Master KK BuildID: 20141028040202 Gaia: 6a7fb482a03c5083ef79b41e7b0dfab27527cd04 Gecko: a255a234946e Gonk: 6e51d9216901d39d192d9e6dd86a5e15b0641a89 Version: 36.0a1 (2.2 Master) Firmware: V188 User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0 Device: Flame 2.1 BuildID: 20141028001203 Gaia: a0174f7166745256aaca1cb3aa9f894033fbffa6 Gecko: 43bda3541f6b Gonk: 6e51d9216901d39d192d9e6dd86a5e15b0641a89 Version: 34.0 (2.1) Firmware: V188 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 Device: Flame 2.0 BuildID: 20141028000202 Gaia: 5e532a84e762b1bb6231756182cf1465671a061e Gecko: 124f0bed1700 Gonk: 6e51d9216901d39d192d9e6dd86a5e15b0641a89 Version: 32.0 (2.0) Firmware: V188 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0