[B2G][OTA] Several issues opening smart collections migrated from 1.x

RESOLVED FIXED in Firefox OS v2.0

Status

Firefox OS
Gaia::Everything.me
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: crdlc, Assigned: crdlc)

Tracking

unspecified
2.1 S1 (1aug)
All
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

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

Details

(Whiteboard: [systemsfe])

Attachments

(5 attachments, 4 obsolete attachments)

(Assignee)

Description

4 years ago
Scenario:

* v.1.3

http://i.imgur.com/gKPB2s2.png

Predefined SC: Social, Games and Music
Installed SC: Astrology, Funny and I'm so Bored
Bookmarks: Flickr, Facebook and Twitter

* After OTA (2.0) the vertical home looks properly

http://i.imgur.com/EGF6ICc.png

but...

Predefined SC: 

The spinner does not disappear for Social and Music (pinned apps) and nor pinned apps neither web results are displayed
Games SC works fine (it does not have pinned apps)

Installed SC:

This message is always displayed "Connect to the internet to get apps for I'm So Bored" (I have connection and good signal)

Bookmarks: all of them work fine
(Assignee)

Comment 1

4 years ago
Created attachment 8458504 [details]
Trace opening social sc
(Assignee)

Comment 2

4 years ago
Created attachment 8458507 [details]
Descriptor migrated for social sc
(Assignee)

Comment 3

4 years ago
Created attachment 8458508 [details]
Trace opening funny sc
(Assignee)

Comment 4

4 years ago
Created attachment 8458509 [details]
Descriptor migrated for funny sc
(Assignee)

Updated

4 years ago
Blocks: 1015336
blocking-b2g: --- → 2.0?
(Assignee)

Updated

4 years ago
status-b2g-v2.0: --- → affected
status-b2g-v2.1: --- → affected
Whiteboard: [systemsfe]
(Assignee)

Comment 5

4 years ago
Amir, Ran

   Is there any issue in the descriptor generation during the migration [1] or are the bugs in collection app consuming data? or both?
 
[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/js/grid_components.js#L142
Flags: needinfo?(ran)
Flags: needinfo?(amirn)
FYI, it's not possible to test your upgrade on the Flame device right now, because of bug 1040146.
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking?]
Depends on: 1040146
(Assignee)

Comment 7

4 years ago
Yes, I know, but I modified my OTA here [1] returning true :)

https://github.com/mozilla-b2g/gaia/blob/master/shared/js/version_helper.js#L32

(In reply to Johan Lorenzo [:jlorenzo] from comment #6)
> FYI, it's not possible to test your upgrade on the Flame device right now,
> because of bug 1040146.
blocking-b2g: 2.0? → 2.0+
Assigning to Cristian since he is investigating. Please re-assign if there is a better owner.
Assignee: nobody → crdlc

Comment 9

4 years ago
Created attachment 8459302 [details] [review]
Pull Request

Looks like any collection with pinned apps is not rendering because of dedupe.js error:
> E/GeckoConsole(  711): [JavaScript Error: "TypeError: result.dedupeId is undefined" {file: "app://collection.gaiamobile.org/shared/js/dedupe.js" line: 57}]

This happens because the pinned items are missing an 'identifier' property.
We were not migrating pinned apps data correctly :(

Cristian, can you please test and review the patch?

It should work for:
1. bookmarks from homescreen added to collection
2. mozapps from homescreen added to collection
3. E.me web results added to collection (long tap > 'add to top of collection')

(did not test myself since I don't know how to simulate OTA updates)
Attachment #8459302 - Flags: review?(crdlc)
Flags: needinfo?(ran)
Flags: needinfo?(amirn)
(Assignee)

Updated

4 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 10

4 years ago
ok, thanks a lot, I have to test this pr. It takes a few hours :)
(Assignee)

Comment 11

4 years ago
Comment on attachment 8459302 [details] [review]
Pull Request

We are in the correct way although there are some issues that should be fixed

1) There is a ReferenceError in the patch (see github)

2) Bookmarks from homescreen added to collection

It does not work. I bookmarked Facebook from Social SC in the old home. After this, I drop the new bookmark to social SC but it does not appear in 2.0

http://i.imgur.com/W4O5uZY.png

3) mozapps from homescreen added to collection

It works fine for regular moz apps and with entry points like "Phone"

4) E.me web results added to collection in 1.3 (long tap > 'add to top of collection')

It works fine too. I added to the social collection "Line" and it appears in 2.0 in the correct place

5) Once migrated if I try to add a web result to top of collection but I have this log

[JavaScript Error: "TypeError: this.toGridObject(...) is undefined" {file: "app://collection.gaiamobile.org/js/objects.js" line: 347}]

it works fine if I try to do the same for Games or Music so it seems that happens only for modified pinned arrays in 1.3 by the user interaction

6) No pre-installed SC cannot be launched (see log attached after posting this review)

Relevant info migrated for Funny:

query:undefined
categoryId:"1e84a07a-4d41-4fdb-a42e-47b204c5900b"
cName:"funny"

Relevant info migrated for I'm so bored:

categoryId:"8fa4b908-366b-4c53-9054-0358a9bd0e40"
query:"I'm So Bored"
cName:"i'm so bored"


7) Social SC keeps the icon from 1.3 instead the new one. It didn't happened last week
Attachment #8459302 - Flags: review?(crdlc)
(Assignee)

Comment 12

4 years ago
Created attachment 8459431 [details]
Funny log.txt
Attachment #8458504 - Attachment is obsolete: true
Attachment #8458507 - Attachment is obsolete: true
Attachment #8458508 - Attachment is obsolete: true
Attachment #8458509 - Attachment is obsolete: true
(Assignee)

Comment 13

4 years ago
Regarding to point 7) Social SC keeps the icon from 1.3 instead the new one. After investigating a bit I realized that if the pre-installed didn't updated its icon this will be

app://collection.... (it implies that the icon will be the new one)

but how the icon was updated the icon is

data:image/png;base64,iVBORw0KGgoA.......
(Assignee)

Comment 14

4 years ago
Regarding to point 6, the first time that non-preinstalled SCs are opened the data is like this:

categoryId:"1e84a07a-4d41-4fdb-a42e-47b204c5900b"
query:"Funny"

and

categoryId:"8fa4b908-366b-4c53-9054-0358a9bd0e40"
query:"I'm So Bored"
(In reply to Cristian Rodriguez (:crdlc) from comment #13)
> Regarding to point 7) Social SC keeps the icon from 1.3 instead the new one.
> After investigating a bit I realized that if the pre-installed didn't
> updated its icon this will be
> 
> app://collection.... (it implies that the icon will be the new one)
> 
> but how the icon was updated the icon is
> 
> data:image/png;base64,iVBORw0KGgoA.......

Yes, if the user changes the collection, the pre-defined icon will be out of sync.
We can:
1) get the collection from the homescreen. GridManager.getIcon(collectionId) , or
2) the migration will create the icons again

I think 2 is better. Can you tell me where is the relevant migration code?
(Assignee)

Comment 16

4 years ago
1) I gonna check you latest patch in order to know if non-preinstalled SCs works fine regarding to background

2) I try to figure out how to fix the issue related to updated pre-defined icon
(In reply to Cristian Rodriguez (:crdlc) from comment #14)
> Regarding to point 6, the first time that non-preinstalled SCs are opened
> the data is like this:
> 
> categoryId:"1e84a07a-4d41-4fdb-a42e-47b204c5900b"
> query:"Funny"
> 
> and
> 
> categoryId:"8fa4b908-366b-4c53-9054-0358a9bd0e40"
> query:"I'm So Bored"

I think I found the problem:
https://github.com/EverythingMe/gaia/blob/1040618-migrate/apps/homescreen/js/grid_components.js#L112
(In reply to Cristian Rodriguez (:crdlc) from comment #16)
> 1) I gonna check you latest patch in order to know if non-preinstalled SCs
> works fine regarding to background
> 
> 2) I try to figure out how to fix the issue related to updated pre-defined
> icon

Pushed one just now. I hope it will fix 1 and 6
(Assignee)

Comment 19

4 years ago
testing, thanks
(Assignee)

Comment 20

4 years ago
Created attachment 8459460 [details] [diff] [review]
1040618-non-preinstalled-sc.patch

Please Amir add this patch to your pr to fix the point 6
Flags: needinfo?(amirn)
(Assignee)

Comment 21

4 years ago
I gonna fix the point 7 (Social SC keeps the icon from 1.3 instead the new one)
(Assignee)

Comment 22

4 years ago
Created attachment 8459475 [details] [diff] [review]
1040618-icon-for-preinstalled-scs.patch

Please Amir add this patch to fix the icon for pre-installed SCs
(Assignee)

Comment 23

4 years ago
remaining work:

2) Bookmarks from homescreen added to collection

It does not work. I bookmarked Facebook from Social SC in the old home. After this, I drop the new bookmark to social SC but it does not appear in 2.0

5) Once migrated if I try to add a web result to top of collection but I have this log

[JavaScript Error: "TypeError: this.toGridObject(...) is undefined" {file: "app://collection.gaiamobile.org/js/objects.js" line: 347}]

it works fine if I try to do the same for Games or Music so it seems that happens only for modified pinned arrays in 1.3 by the user interaction

I gonna investigate about point 2
(Assignee)

Comment 24

4 years ago
Created attachment 8459484 [details] [diff] [review]
1040618-bookmarks-and-top-collection.patch

The last one, this patch fixes points 2 and 5
(Assignee)

Comment 25

4 years ago
so when you apply the three patches to your pull request I will test all together, thanks Amir
Comment on attachment 8459302 [details] [review]
Pull Request

Crisitian, update PR with your patches.
Thanks for you help!
Attachment #8459302 - Flags: review?(crdlc)
Flags: needinfo?(amirn)
(Assignee)

Comment 28

4 years ago
Comment on attachment 8459302 [details] [review]
Pull Request

This is r+ once addressed the last comment about appURL -> appUrl. It works like a charm. Thanks Amir
Attachment #8459302 - Flags: review?(crdlc) → review+
(In reply to Cristian Rodriguez (:crdlc) from comment #27)
> Comment on attachment 8459302 [details] [review]
> Pull Request
> 
> Please fix this comment
> https://github.com/mozilla-b2g/gaia/pull/21964/files#discussion_r15163818

oops. fixed. thanks :)
Target Milestone: --- → 2.1 S1 (1aug)
master: https://github.com/mozilla-b2g/gaia/commit/7543ff4
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Assignee)

Updated

4 years ago
status-b2g-v2.1: affected → fixed
You need to log in before you can comment on or make changes to this bug.