Closed Bug 1029847 Opened 6 years ago Closed 5 years ago

[B2G][2.0][Flame][Vertical Homescreen] Icons don't keep the same order after OTA update.

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
2.0 S5 (4july)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: ychung, Assigned: crdlc)

References

Details

(Whiteboard: [2.0-flame-test-run-2][systemsfe])

Attachments

(3 files, 1 obsolete file)

Attached file IconOrderNotPreservedAfterUdateOTA.zip (obsolete) —
Description: 
When the user update from v.1.4 to v.2.0 after change the order of the icons, icons do not keep the same order on the vertical homescreen.

Repro Steps:
1) Update a Flame to 20140618063004 (v.1.4).
2) Move an icon (e.g. Phone) from the dock to the homepage.
3) Move another icon (e.g. Settings) from the homepage to the dock.
4) Upgrade the device to 2.0.
5) When the update is complete, verify that that icons from the dock fill the first row of the new homescreen. Verify that icons are displayed in the same order than the previous homescreen. 

Actual:
Icons are NOT displayed in the same order the user changed previously. The first row of the new homescreen is Phone, Messages, and Contacts. 

Expected:
Icons are displayed in the same order as on the homescreen on v.1.4 after the user moved the icons. The first row of the new homescreen is Messages, Contacts, Browser, and Settings.

=== Before Update ===
Environmental Variables:
Device: Flame 1.4
Build ID: 20140618000203
Gaia: 3bdd037ec1a11abebe16a5d7f6ff0d863e80bc07
Gecko: 523491fa3339
Version: 30.0 (1.4) 
Firmware Version: v121-2
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0 

=== After Update ===
Environmental Variables:
Device: Flame 2.0
Build ID: 20140618000202
Gaia: 83844c7679b3b9f6e7f1116c1eeec2d1e7a64eec
Gecko: 55679dc2e72b
Version: 32.0a2 (2.0) 
Firmware Version: v121-2
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 

Repro frequency: 100%
Link to failed test case: https://moztrap.mozilla.org/manage/case/13229/
See attached: screenshots, logcat
This issue DOES reproduce on Flame v.122 Base image:

=== Before Update ===

Environmental Variables:
Device: Flame 1.4
Build ID: 20140623000201
Gaia: 3419a1f68aaf64a0688685bce42d4173b6125597
Gecko: 34ecc9af3560
Version: 30.0 (1.4)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0 

=== After Update ===
Environmental Variables:
Device: Flame 2.0
Build ID: 20140624000201
Gaia: 9d2f7bd16a8dc0c74c97c5a40d2f0731f3dfff4b
Gecko: f118b45daada
Version: 32.0a2 (2.0)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 

Icons are NOT displayed in the same order the user changed previously.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
Whiteboard: [2.0-flame-test-run-2]
Per Naoki's email, it was determined that it wasn't necessary to check other branches or devices to see if this issue occurs.

Johan, can you look into this issue? I am unsure if this is an issue with OTA or a vertical homescreen.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker) → needinfo?(jlorenzo)
Please do not include zip files.
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage-]
Flags: needinfo?(ychung)
Attached image IconsMoved1.4.png
Attachment #8445499 - Attachment is obsolete: true
Flags: needinfo?(ychung) → needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage-] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
This is a Homescreen issue to me. 

Nevertheless, this test case was created from March 31st UX specs, when the possibility to switch between old and new homescreen was in the feature's scope. I am wondering if this case is still covered by the current scope.

What do you think Jacqueline? Do we want to the users to keep their layout from 1.4 to 2.0 or this is just a nice to have?
QA Whiteboard: [QAnalyst-Triage+] → [VH-FL-blocking-][VH-FC-blocking?][QAnalyst-Triage+]
Flags: needinfo?(jlorenzo) → needinfo?(jsavory)
The translation between the old homescreen and new one is defined in bug 1006172:
* Former concepts of dock, smart collections & pages will be matched to sections as follows:
    icons from dock
    -- divider --
    smart collection icons
    -- divider --
    icons from page 1
    -- divider --
    icons from page 2
Ordering, I'm assuming, should match the previous homescreen in each of these sections.
There's a key piece of information missing in the bug here - we need to understand the exact setup of the homescreen that was used to reproduce this bug (dock, smart collections, page 1 icons, page 2 icons) & the exact setup that resulted post update.
Blocks: 1015336
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking?][QAnalyst-Triage+] → [VH-FL-blocking-][VH-FC-blocking?][lead-review-]
Flags: needinfo?(ychung)
Keywords: qawanted
My understanding was that we agreed to follow these rules for customization in comment 7, but we originally decided to not try to migrate icon position to the vertical homescreen. I belive this was because there was not a very clear mapping between the two, but it was good enough to use for customization.

It's probably something that is possible to do, but we did not plan on doing it for 2.0.
(In reply to Kevin Grandon :kgrandon from comment #9)
> My understanding was that we agreed to follow these rules for customization
> in comment 7, but we originally decided to not try to migrate icon position
> to the vertical homescreen. I belive this was because there was not a very
> clear mapping between the two, but it was good enough to use for
> customization.
> 
> It's probably something that is possible to do, but we did not plan on doing
> it for 2.0.

I wasn't involved in those conversations but speculate that they may have occurred when we were considering shipping both the new and classic versions. In that case I can understand not mapping them as  users may choose to have different organization patterns for each home screen.

In the 2.0 case, though, there is only one homescreen. If, upon upgrading, if the previous ordering is lost, I can see many users getting extremely frustrated. So I would view implementing the mapping as described in comment 7 as a must-have for 2.0
Kevin, you're right in that the original context of the discussion was having two homescreens and not mapping the ordering from one to the other.  (since the user would manually choose the new one)  This is likely a miss on my part upon the decision to only ship the vertical homescreen.

Just so we can evaluate the worst case, if we didn't do the translation described for existing users, what does the default layout look like?
Flags: needinfo?(kgrandon)
Jason, I'm not setting the flag, but I consider this as a blocker. Users must not be able to lose the organization they created: retaining user created (or organized) information is a requirement for bookmarks on the Homescreen, Smart Collections and more across the OS.
If we did try to preserve ordering, comment 7 makes sense to follow. For now, I think we just use the default ordering which should look something like the following.

dialer, sms, contacts
--- divider ---
collections: social, games, music
collections: showbiz
--- divider ---
camera, gallery, fm
settings, marketplace, calendar
clock, costcontrol, email
music, video
--- divider ---
All other installed apps and collections.
Flags: needinfo?(kgrandon)
What about the Browser icon? If we follow the ordering in comment 7, it should be in the first group of icons.
(In reply to Johan Lorenzo [:jlorenzo] from comment #14)
> What about the Browser icon? If we follow the ordering in comment 7, it
> should be in the first group of icons.

I'll check on the browser today, I was just going by what's in the configuration file, but there was some special handling for browser. I agree that it should be in the first group, if it's not let's do that work in another bug.
Carmen, Cristian - could you guys weigh in with the level of effort and risk to accomplish this?

The vertical homescreen app currently does not define any IAC channels. I'm assuming that we would need to create one, then call into it from the homescreen during the migrate step. Then at first load if we do not have a migrated ordering saved, we would load init.json instead.
Flags: needinfo?(crdlc)
Flags: needinfo?(cjc)
(In reply to Jason Smith [:jsmith] from comment #8)
> There's a key piece of information missing in the bug here - we need to
> understand the exact setup of the homescreen that was used to reproduce this
> bug (dock, smart collections, page 1 icons, page 2 icons) & the exact setup
> that resulted post update.

1.4
[Default Setup]
Dock: Phone, Messages, Contacts, Browser
page 1: Camera, Gallery, FM Radio, Settings
Marketplace, Calendar, Clock, Usage
E-mail, Music, Video
page 2: Social, Games, Music, Showbiz

[After moving icons]
Dock: Phone, Messages, Contacts, Settings
page 1: Camera, Gallery, FM Radio, Marketplace
Calendar, Clock, Usage, E-mail
Music, Video, Phone
page 2: Social, Games, Music, Showbiz

2.1
[After OTA]
Phone, Messages, Contacts
-------------------------
Camera, Gallery, FM Radio
Settings, Marketplace, Calendar
Clock, Usage, E-mail
Music, Video
------------------------
Browser

Please let me know if you need further information. Thanks.
Flags: needinfo?(ychung)
The after moving icons section doesn't include the browser in that list. Also, can we have this tested doing a 1.4 --> 2.0 migration, not 1.4 --> 2.1?
Flags: needinfo?(ychung)
(In reply to Jason Smith [:jsmith] from comment #18)
> The after moving icons section doesn't include the browser in that list.
> Also, can we have this tested doing a 1.4 --> 2.0 migration, not 1.4 --> 2.1?

Sorry about that. I meant 1.4 --> 2.0 on comment 17.


1.4
[Default Setup]
Dock: Phone, Messages, Contacts, Browser
page 1: Camera, Gallery, FM Radio, Settings
Marketplace, Calendar, Clock, Usage
E-mail, Music, Video
page 2: Social, Games, Music, Showbiz

[After moving icons]
Dock: Messages, Contacts, Browser, Settings
page 1: Camera, Gallery, FM Radio, Marketplace
Calendar, Clock, Usage, E-mail
Music, Video, Phone
page 2: Social, Games, Music, Showbiz

2.0
[After OTA]
Phone, Messages, Contacts
-------------------------
Camera, Gallery, FM Radio
Settings, Marketplace, Calendar
Clock, Usage, E-mail
Music, Video
------------------------
Browser
Flags: needinfo?(ychung)
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking?][lead-review-] → [VH-FL-blocking-][VH-FC-blocking?][QAnalyst-Triage+][lead-review+]
Keywords: qawanted
Is comment 19 implying that post migration, the collections weren't present? If so, we need a new bug for this filed, as that's a bad data loss bug here.

Can you try reproducing this also with a 1.4 --> 2.0 migration in which core apps are present on the 2nd page (e.g. Clock, Usage on homescreen page 2)?
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking?][QAnalyst-Triage+][lead-review+] → [VH-FL-blocking-][VH-FC-blocking?]
Flags: needinfo?(ychung)
Keywords: qawanted
I have to talk with Carmen because she knows how the vertical homescreen code works regarding to place icons via init.json. From old home point of view, in the migration process, it is not very complicated to iterate the old layout and send it to vertical. But I think that the most complicate part here is to consume these data in the vertical homescreen. When the vertical receives the old-layout where will save it, indexedDB? Because when it starts rendering this info is needed. Risky? A lot thought because we could receive the old-layout while rendering, before/after SV, etc... We have to control several cases... I think that the safer scenario is not to launch the vertical homescreen before migration. I was thinking aloud.. now we need the Carmen's feedback who knows how it works

(In reply to Kevin Grandon :kgrandon from comment #16)
> Carmen, Cristian - could you guys weigh in with the level of effort and risk
> to accomplish this?
> 
> The vertical homescreen app currently does not define any IAC channels. I'm
> assuming that we would need to create one, then call into it from the
> homescreen during the migrate step. Then at first load if we do not have a
> migrated ordering saved, we would load init.json instead.
Flags: needinfo?(crdlc)
What do you think about saving this info in the vertical datastore like we have "grid-cols" currently?

https://github.com/mozilla-b2g/gaia/blob/master/shared/js/homescreens/vertical_preferences.js

1) Migration process saves the old-layout in the datastore under the key "grid.layout"
2) At first load if we do not have a "grid.layout" in our datastore, we would load init.json instead

I think that we could avoid more IACs with this approach
Flags: needinfo?(kgrandon)
Hmm, it does seem a bit messy - but I do suppose it's better than IAC and I don't really have a better idea.. So it works for me :)
Flags: needinfo?(kgrandon)
We should guarantee:

1) Save the expected structure in "grid.layout" (the same as init.json) in the migration step
2) Launch vertical home after migration step

So we have only to add an if statement in the vertical home like "if we do not have a "grid.layout" in our datastore, we would load init.json instead"
I assign to myself although Carmen and me are working in parallel in order to have the patch as soon as possible
Assignee: nobody → crdlc
Status: NEW → ASSIGNED
Cristian already answered his NI, I'm working on it too, remove NI
Flags: needinfo?(cjc)
Thanks for the effort here.  Do we have good insight into how risky this approach is?
Flags: needinfo?(crdlc)
Target Milestone: --- → 2.0 S5 (4july)
(In reply to Jason Smith [:jsmith] from comment #20)
> Is comment 19 implying that post migration, the collections weren't present?
> If so, we need a new bug for this filed, as that's a bad data loss bug here.
> 
> Can you try reproducing this also with a 1.4 --> 2.0 migration in which core
> apps are present on the 2nd page (e.g. Clock, Usage on homescreen page 2)?

The Flame 2.0 build ID 20140624000201 was missing the smart collections. After I ran the the test case today again from the Flame 1.4 Build ID 20140625000201, the smart collections were properly displayed. However, the change in the order of the icons was NOT reflected on the 2.0 upgrade. 

=== Before Update ===
1.4

[Default Setup]
Dock: Phone, Messages, Contacts, Browser
page 1: Camera, Gallery, FM Radio, Settings
Marketplace, Calendar, Clock, Usage
E-mail, Music, Video
page 2: Social, Games, Music, Showbiz

[After moving icons]
Dock: Phone, Messages, Contacts, Settings
page 1: Camera, Gallery, FM Radio, Marketplace
Calendar, E-mail, Music, Video
Browser
page 2: Social, Games, Music, Showbiz
Clock, Usage

Environmental Variables:
Device: Flame 1.4
Build ID: 20140625000201
Gaia: c9416de14acf9e94ab006619cd2418c768422fcb
Gecko: cddf88f78632
Version: 30.0 (1.4)
Firmware Version: v121-2
ser Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0 

=== After Update ===
2.0

[After OTA]
Phone, Messages, Contacts
-------------------------
Social, Games, Music
Entertainment
-------------------------
Camera, Gallery, FM Radio
Settings, Marketplace, Calendar
Clock, Usage, E-mail
Music, Video
------------------------
Browser

Environmental Variables:
Device: Flame 2.0
Build ID: 20140626000202
Gaia: 6a1373340b40fcfe901336bc9e80676e5f2ba979
Gecko: 82ef9bf64d87
Version: 32.0a2 (2.0)
Firmware Version: v121-2
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
Flags: needinfo?(ychung)
Based on what seeing above, the organization of the homescreen post update will be random - no clear predictability on how the apps will show up.
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking?] → [VH-FL-blocking-][VH-FC-blocking?][QAnalyst-Triage+][lead-review+]
Keywords: qawanted
Well actually we are working on it now but it does not seem very risky. The most amount of code will be in the old-homescreen

(In reply to Peter Dolanjski [:pdol] from comment #28)
> Thanks for the effort here.  Do we have good insight into how risky this
> approach is?
Flags: needinfo?(crdlc)
blocking-b2g: --- → 2.0?
Whiteboard: [2.0-flame-test-run-2] → [2.0-flame-test-run-2][systemsfe]
Jaime - Can you a product call if we need this for 2.0 or not?
Flags: needinfo?(jachen)
Jason, this would also be a product call from Peter. Added him.
Flags: needinfo?(pdolanjski)
Carmen is finishing the unit tests for her part so we will have the patch tomorrow ready for Grandon's review
(In reply to Stephany Wilkes from comment #33)
> Jason, this would also be a product call from Peter. Added him.

This is vital for users who will be getting an OTA update.
Obviously this doesn't matter for new users, so if needed, could we decouple this fix from the 2.0 schedule and have updating partners cherry pick the patch?  (since the OTA update schedule is likely not as tight as 2.0 on new devices)

If that is a bad idea, then I would block on it to make sure it's in 2.0.
Flags: needinfo?(pdolanjski) → needinfo?(jsmith)
(In reply to Peter Dolanjski [:pdol] from comment #35)
> (In reply to Stephany Wilkes from comment #33)
> > Jason, this would also be a product call from Peter. Added him.
> 
> This is vital for users who will be getting an OTA update.
> Obviously this doesn't matter for new users, so if needed, could we decouple
> this fix from the 2.0 schedule and have updating partners cherry pick the
> patch?  (since the OTA update schedule is likely not as tight as 2.0 on new
> devices)
> 
> If that is a bad idea, then I would block on it to make sure it's in 2.0.

I'd rather keep a single maintained 2.0 branch here - we'll run into a lot of partner communication overhead later on trying to make sure they have the right patch cherry picked. Keeping it in our own branch allows us to test it for regressions, rather than wait for a partner to fallout if there's any regressions.
blocking-b2g: 2.0? → 2.0+
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking?][QAnalyst-Triage+][lead-review+] → [VH-FL-blocking-][VH-FC-blocking+][QAnalyst-Triage+][lead-review+]
Flags: needinfo?(jsmith)
Flags: needinfo?(jachen)
(In reply to Jason Smith [:jsmith] from comment #36)
> I'd rather keep a single maintained 2.0 branch here - we'll run into a lot
> of partner communication overhead later on trying to make sure they have the
> right patch cherry picked. Keeping it in our own branch allows us to test it
> for regressions, rather than wait for a partner to fallout if there's any
> regressions.

Fair enough.  I think blocking is the right decision then.
The patch has been finished. We have to wait for Travis and also for the last test from Carmen. In three or four hours this will be available for Kevin's review
Tested and it works correctly
Comment on attachment 8446419 [details]
Go to github pull request

Our patch is ready to review. Thanks
Attachment #8446419 - Attachment description: WIP → Go to github pull request
Attachment #8446419 - Flags: review?(kgrandon)
Comment on attachment 8446419 [details]
Go to github pull request

This is a bit hairy, so the extensive unit tests are definitely appreciated. I can't really see anything to complain about, and I would like to get this in asap to start testing for real. 

Thanks so much for the quick job on this.
Attachment #8446419 - Flags: review?(kgrandon) → review+
Master: https://github.com/mozilla-b2g/gaia/commit/bb2d4191ae720e3c9c1e669e1205b1826d68f61b
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: needinfo?(jsavory)
Resolution: --- → FIXED
Depends on: 1035272
You need to log in before you can comment on or make changes to this bug.