[Gaia:Contacts] Same favorite contacts displayed multiple times

VERIFIED FIXED in Firefox OS v2.2

Status

Firefox OS
Gaia::Contacts
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: kanru, Assigned: ferjm)

Tracking

({regression})

unspecified
2.2 S8 (20mar)
All
Gonk (Firefox OS)
regression

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v2.2 verified, b2g-master verified)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
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.
(Assignee)

Updated

3 years ago
Assignee: nobody → ferjmoreno
(Assignee)

Comment 1

3 years ago
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?
Flags: needinfo?(kchen)
(Reporter)

Comment 2

3 years ago
(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.
Flags: needinfo?(kchen)
(Assignee)

Comment 3

3 years ago
Ok, got it. The favorite needs to be part of the non-cached content to reproduce it. Patch on the way.
Created attachment 8578049 [details] [review]
[gaia] ferjm:bug1143583.contactsfavs > mozilla-b2g:master
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.
(Assignee)

Comment 6

3 years ago
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.
(Assignee)

Comment 7

3 years ago
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
Attachment #8578049 - Flags: review?(francisco)
Comment on attachment 8578049 [details] [review]
[gaia] ferjm:bug1143583.contactsfavs > mozilla-b2g:master

LGTM, nothing to add.
Attachment #8578049 - Flags: review?(francisco) → review+
Keywords: checkin-needed

Updated

3 years ago
Keywords: checkin-needed

Updated

3 years ago
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Comment 10

3 years ago
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.
Attachment #8578049 - Flags: approval-gaia-v2.2?

Updated

3 years ago
blocking-b2g: --- → 2.2+

Updated

3 years ago
Attachment #8578049 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
v2.2: https://github.com/mozilla-b2g/gaia/commit/70cbae6d9f6334cb7ed532fb67908fafd5900923
status-b2g-v2.2: --- → fixed
status-b2g-master: --- → fixed
Target Milestone: --- → 2.2 S8 (20mar)
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.
(Assignee)

Comment 13

3 years ago
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.
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
status-b2g-v2.2: fixed → verified
status-b2g-master: fixed → verified
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.