944311 - When a Contact is deleted by external means the contacts app does not update the list properly

Status: RESOLVED WONTFIX

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

Description

[Jose Manuel Cantera]

Open the contacts app in the list view.

Go to the UI Test and delete all contacts. The Contacts App does not refresh the view.

This can have user impact for instance if the FB synchronization results in a friend no longer present. 

In any case right now the Contacts App is not consistent and this seems to be a regression.

Comment 1

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

Not so sure if this is a bug related to the contacts list or the UI tests.

Actually, if you use contacts and contacts within dialer (two different process), if you remove elements, or perform any modification, you will realise how the list are updated.

Seems to me more like, either a problem in the UI app, or the api not sending an event for contact change when removing ALL the contacts.

In which case I would suggest to have a message too.

wdyt?

Comment 2

[Jason Smith [:jsmith]]

I'm assuming qawanted is to see if we can reproduce this.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

I believe this is a side effect of bug 924565.  Now that DOMRequestHelpers are truly only held be weak references they can now be garbage collected.  In this case, though, the object is being collected even when the client is registered to receive async oncontactchanged events.

The best solution at the moment would be to land/uplift bug 928389.  We could also consider backing out bug 924565 on v1.2.

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Jose, to verify if comment 3 is really the issue you could modify the contacts code to hold a reference to mozContacts.  Something like this:

--- a/apps/communications/contacts/js/contacts.js
+++ b/apps/communications/contacts/js/contacts.js
@@ -691,6 +691,7 @@ var Contacts = (function() {
     }
   };
 
+  var mozContactsStrongRef = navigator.mozContacts;
   navigator.mozContacts.oncontactchange = function oncontactchange(event) {
     if (typeof pendingChanges[event.contactID] !== 'undefined') {
       pendingChanges[event.contactID].push({
@@ -865,6 +866,7 @@ var Contacts = (function() {
   }
 
   return {
+    'strongRef' : mozContactsStrongRef,
     'goBack' : handleBack,
     'cancel': handleCancel,
     'goToSelectTag': goToSelectTag,

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Now that bug 928389 has landed, you can ignore the patch in comment 4 and just retest with a build that has these commits:

https://hg.mozilla.org/mozilla-central/rev/2f40cd447dc0
https://hg.mozilla.org/mozilla-central/rev/f3174f0757c8

Comment 6

[Preeti Raghunath(:Preeti)]

Jose,

Please do take a look at verifying the patch.

Comment 7

[Jose Manuel Cantera]

with the latest Gecko I have 8c8945 I'm still reproducing the issue

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

Hmm, it must be a different issue, then.  Sorry about leading us down the wrong path there.

Comment 9

[Jason Smith [:jsmith]]

Comment for 12/5 triage:

This is bad for third party app integration that plan to use the Contacts API. If we confirm this is a regression that works on 1.1 & doesn't work on 1.2, then I think we need to block on this.

Comment 10

[Jose Manuel Cantera]

(In reply to Jason Smith [:jsmith] from comment #9)
> Comment for 12/5 triage:
> 
> This is bad for third party app integration that plan to use the Contacts
> API. If we confirm this is a regression that works on 1.1 & doesn't work on
> 1.2, then I think we need to block on this.

I agree

Comment 11

[Preeti Raghunath(:Preeti)]

Gregor,

Please review the same.

Comment 12

[Gregor Wagner [:gwagner]]

My guess is that the contacts app is expecting a contact ID for a changed contact. Whenever we modify or delete a contact we broadcast the event with the contact ID. In the case of a 'clear DB', we just send a 'remove' event without an ID. Can someone check the contacts app code?

Comment 13

[Preeti Raghunath(:Preeti)]

David

Please review from contacts perspective.

Comment 14

[Gregor Wagner [:gwagner]]

I am not sure the use-case in the description is valid here. Removing all contacts is very different from removing one single contact. Does removing one single contact work?
It also works if the contact app is restarted right?

For koi?: This shouldn't block if the bug is only present with a deleteAll call and the UI is refreshed after a restart.

Comment 15

[Jason Smith [:jsmith]]

(In reply to Gregor Wagner [:gwagner] from comment #14)
> I am not sure the use-case in the description is valid here. Removing all
> contacts is very different from removing one single contact. Does removing
> one single contact work?
> It also works if the contact app is restarted right?
> 
> For koi?: This shouldn't block if the bug is only present with a deleteAll
> call and the UI is refreshed after a restart.

I don't think I agree here. The problem here is that doing a delete all action on all of your contacts, then that will leave the contacts app in a bad state overall until you restart it. That basically breaks any chance of having a third-party use case with the contacts API, as the contacts app will fall apart as soon as a delete interaction takes place outside the context of the contacts app. A restart is problematic to use as a solution because that's going to have to happen on every single third party app deletion use case that happens. That will likely cause a lot of negative reviews on marketplace apps as a result, as the users of those apps will negatively blame the app developers for this.

I think I still stand on thinking this is a blocker if this is a confirmed regression. Otherwise, this is going to cause way too many problems to count with content management.

Comment 16

[sarsenyev]

The bug reproduces on 1.1, 1.2 and 1.3 Engineering builds.
When deleting all contacts from UI Test, the Contacts App does not refresh the view.
To see the changes a user needs to kill the Contacts app or restart the device


Device: Buri 1.2 ENG build
BuildID: 20131205004003
Gaia: 0659f16b9790b1cf9eba4d80743fcc774d2ffe3a
Gecko: af2c7ebb5967
Version: 26.0

Device: Buri 1.2 ENG build
BuildID: 20131205040201
Gaia: 1dd0e5c644b4c677a4e8fa02e50d52136db489d9
Gecko: 725c36b5de1a
Version: 28.0a1

Device: Buri 1.1 ENG build
BuildID: 20131009041203
Gaia: 53e2a70d85fb3748d0768218a5efffe5806073f0
Gecko: c630289d6388
Version: 18.0

Comment 17

[Jason Smith [:jsmith]]

Okay - that's what I was looking to know. If this is already present on 1.1, then my content management concerns don't apply, as developers are already aware of this bug. So this is a not a blocker then.

Comment 18

[David Scravaglieri [:scravag]]

clear n?, this bug has been minused

Comment 19

[BMO Automation]

Firefox OS is not being worked on