1047276 - [Vertical Home-screen] Change in position of smart collections in default-Homescreen json doesnt reflect on screen

Status: RESOLVED FIXED

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

Description

[Mukesh kumar]

[Title] Change in position of smart collections default-Homescreen json doesnt reflect on screen

STR for the issue.
1. Change the position of collectionss App in default-Homescreen.json,say, make a separate section for the smart collection at the top.
2. Build gaia using make / PRODUCTION=1 make

Expected:
Smart collections should Appear as in default-Homescreen.json file.

Actual:
Smart collections APP always add to the last section in the screen.

Comment 1

[Johan Lorenzo [:jlorenzo]]

As per your step 2, I presume that after the flash, you don't see the FTU tutorial which doesn't run the Homescreen update. Nevertheless, I tried with to flash my phone with that command (MOZILLA_OFFICIAL=1 NOFTU=0 make production) and the Smart Collection are not placed where I defined in  gaia/apps/homescreen/build/default-homescreens.json.

>  "homescreens": [
>    [
>      ["apps/homescreen/collections", "social"],
>      ["apps/homescreen/collections", "games"],
>      ["apps/homescreen/collections", "music"],
>      ["apps/homescreen/collections", "showbiz"]
>    ],
>    [
>      ["apps", "communications", "dialer"],
>      ["apps", "sms"],
>      ["apps", "communications", "contacts"]
>    ],
>    [
>      ["apps", "camera"],
>      ["apps", "gallery"],
>      ["apps", "fm"],
>      ["apps", "settings"],
>      ["apps", "marketplace.firefox.com"],
>      ["apps", "calendar"],
>      ["apps", "clock"],
>      ["apps", "costcontrol"],
>      ["apps", "email"],
>      ["apps", "music"],
>      ["apps", "video"]
>    ]
>  ],

Kevin, is there something wrong in the flashing command or in the default-homescreen.json file?

Comment 2

[Kevin Grandon :kgrandon]

No idea. Adding Cristian/Carmen here as I think they did the original positioning of the smart collections. Potentially a bug with the flash.sh script or the way we populate in FTU/homescreen.

Comment 3

[Youngjun Kim]

This issue isn't reproduced when SIM Card is inserted and network is connected. 
Please check root cause why network staus effects location of smart collection.

Comment 4

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Attachment: 2014-08-12-17-58-02.png

Mukesh, I can't reproduce it both on b2g-desktop and buri, could you provide your STR?

what I do is:
1. move collections to top of default-homescreens.json (attach it later)
2. make / PRODUCTION=1 make / make production with device

and collections will be on first section of homescreen.

Comment 5

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Attachment: default-homescreens.json

this file should be put into GAIA_DIR/apps/verticalhome/build/default-homescreens.json

Comment 6

[Mukesh kumar]

Attachment: default-homescreens.json

I have used attached default-homescreen.json file to build as present in Master
Command:
make / PRODUCTION=1 make
For me its not coming in proper order.

Comment 7

[Mukesh kumar]

Attachment: default.png

This is the screenshot of improper layout when build.

Gaia version(Master):  e9dc30188b535fd5adb0a47fb9d72d188c9c1bc6

If you are using any SIM in your device, please remove and build again.
With some SIM and network connection, it is coming as proper.

Comment 8

[Kevin Grandon :kgrandon]

(In reply to Mukesh kumar from comment #6)
> Created attachment 8471536 [details]
> default-homescreens.json
> 
> I have used attached default-homescreen.json file to build as present in
> Master
> Command:
> make / PRODUCTION=1 make
> For me its not coming in proper order.

What command are you using to flash the profile? Are you using make reset-gaia?

Comment 9

[Mukesh kumar]

> What command are you using to flash the profile? Are you using make
> reset-gaia?
 I am using the following commands
make or PRODUCTION=1 make or DEVICE_DEBUG=1 PRODUCTION=1 make

The collection Apps always add to the last section.

Comment 10

[Kevin Grandon :kgrandon]

(In reply to Mukesh kumar from comment #9)
>  I am using the following commands
> make or PRODUCTION=1 make or DEVICE_DEBUG=1 PRODUCTION=1 make
> 
> The collection Apps always add to the last section.

Sure, but how do you send the profile to the device?

Comment 11

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Mukesh,

could you provide your profile to me? that can help to find out what the problem is.

Comment 12

[Mukesh kumar]

Hi Yuren,

I am unable to share the profile.
Can you share the profile with me from which you are getting the proper layout.

So that I can check whether there is problem with pushing gaia into device or its gaia issue.

Comment 13

[Cristian Rodriguez (:crdlc)]

It seems a race condition.

* This is the correct scenario:

1) FTU runs and the collection app is initialized populating the datastore with defined collections [1]
2) Vertical home consumes the predefined layout and when it tries to paint collections in their positions, they are stored in the datastore and it works fine

Race condition scenario (maybe faster devices or with more memory)

1) FTU runs and the collection app is initialized populating the datastore but while this task is being performed...
2) The vertical home is launched and it tries to paint the predefined collection in the grid. When it tries to recover the info form datastore these data are not available and collections are not painted. After populating the datastore, collections are installed in the grid in the latest positions as new installations.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/ftu/js/app.js#L42

How could we fix it?

Fernando, do you know if we can move this call earlier?
Kevin, is there any chance to launch the vertical home once the population is performed to avoid the race condition?

Comment 14

[Fernando Campo (:fcampo)]

(In reply to Cristian Rodriguez (:crdlc) from comment #13)
> Race condition scenario (maybe faster devices or with more memory)
> 
> 1) FTU runs and the collection app is initialized populating the datastore
> but while this task is being performed...
> 2) The vertical home is launched and it tries to paint the predefined
> collection in the grid. When it tries to recover the info form datastore
> these data are not available and collections are not painted. After
> populating the datastore, collections are installed in the grid in the
> latest positions as new installations.
> 
> [1] https://github.com/mozilla-b2g/gaia/blob/master/apps/ftu/js/app.js#L42
> 
> How could we fix it?
> 
> Fernando, do you know if we can move this call earlier?
> Kevin, is there any chance to launch the vertical home once the population
> is performed to avoid the race condition?

From the FTU point of view, it is not possible to make the call earlier, it's already done in the first steps from the app.init. We might change it though for a Promise, and keep going with the FTU only when the notifyCollection is finished, but I doubt it will help, as it seems more likely to be a matter of Homescreen starting too early, but I'd rather have some confirmation from Kevin on this. It seems that the function notifyCollection from FTU sends a message to the system when it's done, could that be where it's failing?

Comment 15

[Kevin Grandon :kgrandon]

(In reply to Cristian Rodriguez (:crdlc) from comment #13)
> Kevin, is there any chance to launch the vertical home once the population
> is performed to avoid the race condition?

Seems tricky. We'd have to make some system changes here which might be painful. 

One option we might consider here is to change the behavior of the "added" collections database handler. Maybe if a collection is "added" we can check if it's a default collection, and if so try to insert it into the right position. Maybe we'd have to keep track of if it's the first time the homescreen has loaded or not though.

Comment 16

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

looks we found the cause, needinfo? me if having any build system issue.

Comment 17

[Cristian Rodriguez (:crdlc)]

Don't worry the build system is OK!

(In reply to Yuren [:yurenju] from comment #16)
> looks we found the cause, needinfo? me if having any build system issue.

Comment 18

[Cristian Rodriguez (:crdlc)]

Attachment: Github pull request

Comment 19

[Kevin Grandon :kgrandon]

Comment on attachment 8475095 [details]
Github pull request

Seems reasonable to me. Thanks!

Comment 20

[Cristian Rodriguez (:crdlc)]

Merged in master:

https://github.com/crdlc/gaia/commit/6411a9f47fa63854c3fe88d5d0fe296ec71b809c