832861 - [OPEN_][contacts]Add new Contacts, you can't use "custom" to edit numbers, mailbox, mailing address type.(617001971176)

Status: VERIFIED FIXED

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

Description

[Firefox_Mozilla]

Steps to reproduce:
1.create a new contact
2.input the number type, mailbox, mailing address type in input box of "custom".

Actual results:
After we input some custom item in the input box of "custom", we can save this custom item.

Expected results:
create a new contact, normal self-definition of the type of number, E-mail, Address Type.

Comment 1

[KM Lee [:rexboy] (inactivate after Sep. 22)]

mouse_event_shim.js failed to synthesize a click event for customTag.onclick.  The examination below at handleTouchStart(:73) always failed:
    // Sometimes an unknown gecko bug causes us to get a touchstart event
    // for an iframe target that we can't use because it is cross origin.
    // Don't start handling a touch in that case
    try {
      e.changedTouches[0].target.ownerDocument;
    }
    catch (e) {
      // Ignore the event if we can't see the properties of the target
      return;
    }

We have an apparent workaround: just use ontouchend rather than onclick.  I'll patch it and try to add an unit test.

Comment 2

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Attachment: Patch

Well.. the patch ended up with more test codes than bugfix itself.

Arcturus, may you help reviewing this patch?
Thanks a lot!

Comment 3

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Comment on attachment 710110 [details]
Patch

Found something breaks the contacts_list test.  Let me check for a while and I will set review flag again.  Sorry for disturbing.

Comment 4

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Comment on attachment 710110 [details]
Patch

OK I got rid of the problem now (I think).  Setting r?.

Comment 5

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

Comment on attachment 710110 [details]
Patch

Asking Alberto to do the r?

Right now I'm a bit overloaded and he can do the r? better than me.

Thanks folks!

Comment 6

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Seems this patch has been out-dated mainly due to the startup time tuning & lazy loading recently.

Alberto if you are overloaded, please tell me if someone can help review this patch.  Thank you!

Comment 7

[alberto.pastor]

Sorry, completely forgot this patch after a crazy week of improving startup times... my fault.
Could you please rebase it with current master? I'll review it ASAP.

Thanks!

Comment 8

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Comment on attachment 710110 [details]
Patch

Alberto:  It's OK, I've rebased the patch.
To perform unit test, I made some refactoring that may need to be take care of (on initializing).  Thank you for the help!

Comment 10

[alberto.pastor]

I did some lasts comments on github. Thanks :rexboy!

Comment 11

[KM Lee [:rexboy] (inactivate after Sep. 22)]

I've updated the code.  If things are OK we can land it.
Thank you Alberto!

Comment 12

[Dietrich Ayala (:dietrich)]

Blocking, totally broken as-is.

Comment 13

[alberto.pastor]

https://github.com/mozilla-b2g/gaia/commit/374e612fcb1b57f99c927f4353f7186fbae213cf

Comment 14

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted commit 374e612fcb1b57f99c927f4353f7186fbae213cf as:
v1-train: 23b017d99169848b194f11e870ac7c9e8d2bc8ab

Comment 15

[Michael Vines [:m1] [:evilmachines]]

(tef+ due to partner request for uplift)

Comment 16

[John Ford [:jhford] CET/CEST Berlin Time]

(In reply to Michael Vines [:m1] [:evilmachines] from comment #15)
> (tef+ due to partner request for uplift)

This doesn't apply cleanly on v1.0.1.  It looks like the new bit of code is completely equivalent to the old bit of code, but I'd like someone who's more familiar double check.

diff --cc apps/communications/contacts/js/contacts.js
index c307a4e,110f766..0000000
--- a/apps/communications/contacts/js/contacts.js
+++ b/apps/communications/contacts/js/contacts.js
@@@ -790,15 -817,15 +808,24 @@@ var Contacts = (function()
            currentContact.id == event.contactID) {
            contactsList.getContactById(event.contactID,
              function success(contact, enrichedContact) {
++<<<<<<< HEAD
 +            currentContact = contact;
 +            var mergedContact = enrichedContact || contact;
 +            contactsDetails.render(mergedContact, false,
 +                                   enrichedContact ? true : false);
 +            contactsList.refresh(mergedContact, checkPendingChanges,
 +                                 event.reason);
++=======
+             currentContact = enrichedContact || contact;
+             contactsDetails.render(currentContact, false,
+                                    enrichedContact ? true : false);
+             contactsList.refresh(currentContact, checkPendingChanges,
+               event.reason);
++>>>>>>> 23b017d... Merge pull request #7961 from rexboy7/fix-bug832861
            });
          } else {
-           contactsList.refresh(event.contactID, checkPendingChanges, event.reason);
+           contactsList.refresh(event.contactID, checkPendingChanges,
+             event.reason);
          }
          break;
        case 'create':

Comment 17

[KM Lee [:rexboy] (inactivate after Sep. 22)]

In this patch these are just changes for linter check. So applying newer changes should be safe. Thanks!

Comment 18

[John Ford [:jhford] CET/CEST Berlin Time]

v1.0.1: cf3b5ae42a851d266752b4d82ca2d08a8eb0c459

Comment 19

[Jeni]

Verified fixed on 
Unagi Build ID: 20130401070203
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/b28463f2e718
Gaia: ddb38ac8a34f9e30e09d0ff3b5c1bfb9b664b7c3

and

Unagi Build ID: 20130401070203
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/b28463f2e718
Gaia: ddb38ac8a34f9e30e09d0ff3b5c1bfb9b664b7c3