Bug 836170 - [Contact]: Scrolling and Dragging in contact application is very slow #8045

arcturus · 2013-02-11T17:02:38Z

Fixed all unit tests and problems detected on the unit tests as well.

Rebased and conflicts solved with current master

crdlc · 2013-02-12T08:24:33Z

apps/communications/contacts/js/fb_extensions.js

@@ -290,8 +290,6 @@ if (typeof Contacts.extFb === 'undefined') {
        break;

        case 'fb_updated':
-          contacts.List.load();


In the line https://github.com/arcturus/gaia/blob/82ed2995ff3585f8e03230a6a9b020e2cfb6c177/apps/communications/contacts/js/fb_extensions.js#L309

When a synchronization finishes, maybe contacts may have been changed so we call to contacts.List.load() when at least one of them have been changed. Now is it ok? I guess you will receive oncontactchange and this call is not necessary anymore, true?

Yes, that's the case, when the synch happens, we will receive oncontactchange and will update if necessary.

ok, I understand, so we should remove that as well, right?

crdlc · 2013-02-12T10:03:57Z

I've detected two bugs:

Change the photo on Facebook to an imported friend doesn't work, the contact list is no updated. Go to Facebook in my laptop, change the photo to a friend, after that, I go to import view (the synchronization process finishes) but the contact list is not updated, the image is the same than before.
I have a friend called Manolo from Facebook imported in my contact list. I create a new one called Man. Then I link this one to Manolo on Facebook. Now, I have Manolo friend and Man linked, both of them on the contact list. It fails, I should only have Man linked. The friend should disappear.

arcturus · 2013-02-12T11:35:12Z

Thanks for the feedback Cristian, I understand that the photo is not stored in the contacts photo it self, so a oncontactschange is not fired and we don't have any clue of the change.

Same with the friend linking.

Here we have two actions, either force a change in the contact, or trying to fire a custom event that acts as the oncontactschange.

Thanks for spotting this!

crdlc · 2013-02-12T11:42:42Z

and maybe the same with the company

jmcanterafonseca · 2013-02-12T11:43:57Z

@arcturus when a link operation happens the local contact is updated in the Contacts DB, so the oncontactschange event should be fired.

The same happens with the photo, even if the photo is updated on other DB the local contact is updated, thus I think something wrong is being done in some place.

arcturus · 2013-02-12T13:42:49Z

@jmcanterafonseca you are right, after doing some tests, even writing some code I realise that you change the contact to specify the new url for the image. So that triggers a onContactsChange, and so far is working.

I tried for both the photo and the company and works as expected.

Same with the linking

@crdlc, is you gecko recent? This oncontactchange was added some days ago, could you update your gecko please?

crdlc · 2013-02-12T14:42:54Z

ok, testing, I updated my gecko one week ago (02-07-13/)

arcturus · 2013-02-12T20:53:52Z

Hi folks, both @crdlc and @albertopq could you try?

I had to modify the addToList method to take into account that it should receive contacts that are facebook or linked to facebook, and pass that extra information to the method for rendering the extra info.

Cheers!

crdlc · 2013-02-13T08:19:36Z

During morning I am going to test it

jmcanterafonseca · 2013-02-13T10:45:09Z

@arcturus the patch is not working properly. It introduces weird behaviors when linking and unlinking FB Friends. If you link a Contact to a FB Friend temporarily you see the Contact List and afterwards a the Contact detail. Even worst is the case when you unlink a FB Contact on which you temporarily see the unlinked device Contact and then you see the FB Friend Contact.

Please check these behaviors in the current master or train branch just to see how it should be working.

arcturus · 2013-02-13T11:32:16Z

@jmcantera we solve the first and the worst problems.

Hope now are looking ok for you :)

albertopq · 2013-02-13T12:11:26Z

apps/communications/contacts/js/contacts_list.js

    }
    showGroup('favorites');
  };

-  function addToFavoriteList(c) {
+  function addToFavoriteList(favorite) {
    var group = 'contacts-list-favorites';
    var container = document.getElementById(group);


Can we use headers['favorites'] here?

crdlc · 2013-02-13T15:52:38Z

Sorry it fails, I have an empty list. I import my friend Javi Chiquitin, then I create a new contact called Javi, I link this one to Javi Chiquitin and finally I have two duplicated contacts Javi Chiquitin

arcturus · 2013-02-13T16:08:46Z

@crdlc yep, after suggestions from @albertopq we had the same problem.

Working on it.

arcturus · 2013-02-13T19:56:20Z

Hi guys, lats changes done, and the rebase made with latest PR from @albertopq done as well.

Hope this time everything is really fixed.

jmcanterafonseca · 2013-02-14T09:25:51Z

apps/communications/contacts/js/contacts.js

+    if (pendingChanges[id].length >= 1) {
+      performOnContactChange(pendingChanges[id][0]);
+    } else {
+      console.log('No more changes for ' + id);


do we really need that in the final version?

Nope definitely not thanks for spoting :) (too much debug yesterday)

jmcanterafonseca · 2013-02-14T10:03:13Z

@arcturus apart from the two nits found I have not found any other regression concerning FB. Please note I have not tested possible regressions with Web Activities.

arcturus · 2013-02-14T10:53:47Z

@jmcanterafonseca yes we tried WebActivities, but we wanted other people to take a look, anyway QA folks will say something :)

… is very slow

Bug 836170 - [Contact]: Scrolling and Dragging in contact application is very slow

Bug 836170 - [Contact]: Scrolling and Dragging in contact application is very slow(cherry picked from commit 3859bf4) Conflicts: apps/communications/contacts/js/contacts_list.js apps/communications/contacts/test/unit/contacts_list_test.js (cherry picked from commit c48e9eb)

crdlc reviewed Feb 12, 2013
View reviewed changes

albertopq reviewed Feb 13, 2013
View reviewed changes

jmcanterafonseca reviewed Feb 14, 2013
View reviewed changes

Bug 836170 - [Contact]: Scrolling and Dragging in contact application…

2994422

… is very slow

arcturus added a commit that referenced this pull request Feb 14, 2013

Merge pull request #8045 from arcturus/bug-838622

3859bf4

Bug 836170 - [Contact]: Scrolling and Dragging in contact application is very slow

arcturus merged commit 3859bf4 into mozilla-b2g:master Feb 14, 2013

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 836170 - [Contact]: Scrolling and Dragging in contact application is very slow #8045

Bug 836170 - [Contact]: Scrolling and Dragging in contact application is very slow #8045

arcturus commented Feb 11, 2013

crdlc Feb 12, 2013

arcturus Feb 12, 2013

crdlc Feb 12, 2013

crdlc commented Feb 12, 2013

arcturus commented Feb 12, 2013

crdlc commented Feb 12, 2013

jmcanterafonseca commented Feb 12, 2013

arcturus commented Feb 12, 2013

crdlc commented Feb 12, 2013

arcturus commented Feb 12, 2013

crdlc commented Feb 13, 2013

jmcanterafonseca commented Feb 13, 2013

arcturus commented Feb 13, 2013

albertopq Feb 13, 2013

crdlc commented Feb 13, 2013

arcturus commented Feb 13, 2013

arcturus commented Feb 13, 2013

jmcanterafonseca Feb 14, 2013

arcturus Feb 14, 2013

jmcanterafonseca commented Feb 14, 2013

arcturus commented Feb 14, 2013

Bug 836170 - [Contact]: Scrolling and Dragging in contact application is very slow #8045

Bug 836170 - [Contact]: Scrolling and Dragging in contact application is very slow #8045

Conversation

arcturus commented Feb 11, 2013

crdlc Feb 12, 2013

Choose a reason for hiding this comment

arcturus Feb 12, 2013

Choose a reason for hiding this comment

crdlc Feb 12, 2013

Choose a reason for hiding this comment

crdlc commented Feb 12, 2013

arcturus commented Feb 12, 2013

crdlc commented Feb 12, 2013

jmcanterafonseca commented Feb 12, 2013

arcturus commented Feb 12, 2013

crdlc commented Feb 12, 2013

arcturus commented Feb 12, 2013

crdlc commented Feb 13, 2013

jmcanterafonseca commented Feb 13, 2013

arcturus commented Feb 13, 2013

albertopq Feb 13, 2013

Choose a reason for hiding this comment

crdlc commented Feb 13, 2013

arcturus commented Feb 13, 2013

arcturus commented Feb 13, 2013

jmcanterafonseca Feb 14, 2013

Choose a reason for hiding this comment

arcturus Feb 14, 2013

Choose a reason for hiding this comment

jmcanterafonseca commented Feb 14, 2013

arcturus commented Feb 14, 2013