1143583 - [Gaia:Contacts] Same favorite contacts displayed multiple times

Status: VERIFIED FIXED

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

Description

[Kan-Ru Chen [:kanru] (UTC+9)]

I believe I started to see this after bug 1112551 hit nightly.

Same favorite contacts appear multiple times. If I edit one of them and add/remove favorite, it returns to only one entry. Kill the contacts app and launch again, multiple entry appear again.

Comment 1

[Fernando Jiménez Moreno [:ferjm]]

I cannot reproduce this issue. Could you write a more clear STR, please?

Did this happen after updating the app from a previous version with an already existing contacts list?

Comment 2

[Kan-Ru Chen [:kanru] (UTC+9)]

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #1)
> I cannot reproduce this issue. Could you write a more clear STR, please?

The only step is to open the Contacts app..

> Did this happen after updating the app from a previous version with an
> already existing contacts list?

I think so. I found this on my dogfood phone so there are already many contacts.

Comment 3

[Fernando Jiménez Moreno [:ferjm]]

Ok, got it. The favorite needs to be part of the non-cached content to reproduce it. Patch on the way.

Comment 4

[GitHub Autolander Bot]

Attachment: [gaia] ferjm:bug1143583.contactsfavs > mozilla-b2g:master

Comment 5

[Francisco Jordano [:arcturus] [:francisco]]

Fernando can we make favourites a different group to cache? Didn't look at the patch yet but instead of making that section non cacheable we can experiment with the caching groups, as favourites is a group.

That will allow us to display the favorites really fast too, something that several people are demanding.

Comment 6

[Fernando Jiménez Moreno [:ferjm]]

That's the current status :) Favorites and ICE contacts are part of the cache. Basically, anything that is above the fold is part of the cache.

Comment 7

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8578049 [details] [review]
[gaia] ferjm:bug1143583.contactsfavs > mozilla-b2g:master

Let me explain the situation a little bit to ease the review process.

We keep a list in memory with the contacts that are in the cache while we do the first rendering [1] so we can decide if a contact needs to be appended to the DOM or not (because it was already appended as a cached contact). We do the same with favorite contacts.

Every time we get a contact from the API during the initial list rendering process we check if the contact is on the cache and in that case we get it and remove it from these Maps [2]. That way, when we end rendering the list, we can check if any of the contacts applied in the cache needs to be removed from the DOM [3] (because they were removed from the API while we were not looking).

The issue is that we were removing the contact from both maps (normal contacts and favorite contacts) at once. And because the list code needs to append a favorite contact twice (one to its group and another to the favorites group) we were failing at [4] and we were considering the favorite contact as not part of the cache. And so the duplicated entry.

[1] https://mxr.mozilla.org/gaia/source/apps/communications/contacts/js/bootstrap.js#54
[2] https://mxr.mozilla.org/gaia/source/apps/communications/contacts/js/bootstrap.js#306
[3] https://mxr.mozilla.org/gaia/source/apps/communications/contacts/js/views/list.js#2375
[4] https://mxr.mozilla.org/gaia/source/apps/communications/contacts/js/views/list.js#800

Comment 8

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8578049 [details] [review]
[gaia] ferjm:bug1143583.contactsfavs > mozilla-b2g:master

LGTM, nothing to add.

Comment 9

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/3f75f6262bac8f0ca7a26769f78d0849ba6bf2f0

Comment 10

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8578049 [details] [review]
[gaia] ferjm:bug1143583.contactsfavs > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Bug 1112551. If that one goes to 2.2, we need to take this fix as well.
[User impact] if declined: Terrible UX. The user will see its favorite contacts list growing with each execution of the contacts app.
[Testing completed]: Manual tests. Unit tests added.
[Risk to taking this patch] (and alternatives if risky): Low risk.
[String changes made]: None.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/70cbae6d9f6334cb7ed532fb67908fafd5900923

Comment 12

[Pi Wei Cheng [:piwei] (inactive)]

Is there an STR for this bug? I'm trying to verify this issue and I flashed to a 3/16 master build in order to see this bug reproduce but can't repro it.

Comment 13

[Fernando Jiménez Moreno [:ferjm]]

According to comment 3 you need to add a favorite that is not cached. We only cache the first chunk which is the first 20 contacts. You should be able to reproduce this if you favorite one contact that is not one of the first 20 contacts of the list.

Comment 14

[Pi Wei Cheng [:piwei] (inactive)]

Thanks, I was able to reproduce the issue with the information on comment 13 on a 3/16 master build. I verified that this issue is fixed on latest Flame 3.0 and 2.2. No duplicate contacts appear.

Device: Flame 3.0
BuildID: 20150429010205
Gaia: 6e35b0948c42a4398b8a5916015de167121683a1
Gecko: 1ad65cbeb2f4
Gonk: b83fc73de7b64594cd74b33e498bf08332b5d87b
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Device: Flame 2.2
BuildID: 20150429002501
Gaia: 1b7aa7e60788668ed09abf76022dfa231dbe88d4
Gecko: d38ff4717f39
Gonk: ebad7da532429a6f5efadc00bf6ad8a41288a429
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0