Closed
Bug 944311
Opened 11 years ago
Closed 7 years ago
When a Contact is deleted by external means the contacts app does not update the list properly
Categories
(Firefox OS Graveyard :: Gaia::Contacts, defect)
Tracking
(blocking-b2g:-)
RESOLVED
WONTFIX
blocking-b2g | - |
People
(Reporter: jmcf, Unassigned)
Details
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.
Reporter | ||
Updated•11 years ago
|
OS: Mac OS X → Gonk (Firefox OS)
Hardware: x86 → ARM
Whiteboard: regression, qawanted
Comment 1•11 years ago
|
||
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•11 years ago
|
||
I'm assuming qawanted is to see if we can reproduce this.
Keywords: qawanted,
regression
Whiteboard: regression, qawanted
Comment 3•11 years ago
|
||
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.
Updated•11 years ago
|
Comment 4•11 years ago
|
||
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,
Flags: needinfo?(jmcf)
Comment 5•11 years ago
|
||
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•11 years ago
|
||
Jose,
Please do take a look at verifying the patch.
Reporter | ||
Comment 7•11 years ago
|
||
with the latest Gecko I have 8c8945 I'm still reproducing the issue
Flags: needinfo?(jmcf)
Comment 8•11 years ago
|
||
Hmm, it must be a different issue, then. Sorry about leading us down the wrong path there.
Comment 9•11 years ago
|
||
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.
Reporter | ||
Comment 10•11 years ago
|
||
(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 12•11 years ago
|
||
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?
Flags: needinfo?(anygregor)
Comment 13•11 years ago
|
||
David
Please review from contacts perspective.
Flags: needinfo?(dscravaglieri)
Comment 14•11 years ago
|
||
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•11 years ago
|
||
(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•11 years ago
|
||
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
Keywords: qawanted
Comment 17•11 years ago
|
||
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.
blocking-b2g: koi? → -
Keywords: regression
Updated•10 years ago
|
Flags: needinfo?(ferjmoreno)
Updated•9 years ago
|
Flags: needinfo?(ferjmoreno)
Comment 19•7 years ago
|
||
Firefox OS is not being worked on
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•