for NFC, contact application needs to be able to create NDEF message

The user can share content to another NFC enabled device (e.g. Mobile device , tablet, laptop) either directly via NFC or via a BT connection initiated by NFC.1) The user can select a contact and then share it via NFC to another device by tapping the mobile device with another NFC enabled device. Transfer of contact vcard takes place directly over NFC. a) The user selects a contact and then selects “Share contact” from a contextual menu. b) The user taps the device with another NFC enabled device. c) On the receiving mobile, the device will get a prompt saying “Another device wants to share a contact with you – Accept/Reject” or something similar. d) Any vcard received is opened up in the contact edit page, so that the user can save it any of the accounts that the user has for PIM service providers.

Joe, could you please put this into your backlog and evaluate if your team can help or not? Thank you!

will be decided in 1.3 triages

Joe, can we have a assignee for this bug?

what's the status of the gecko support? can it be linked to this bug? thanks

(In reply to Joe Cheng [:jcheng] from comment #5) > what's the status of the gecko support? can it be linked to this bug? thanks Sure

1.3 targeted remove 1.3+

(In reply to Joe Cheng [:jcheng] from comment #7) > 1.3 targeted > remove 1.3+ Hi, Joe, does it mean that Comms team won't focus on this in v1.3? Just want to know if this would be slipped to v1.4 for sure. Thanks!

We wont have time to address this in v1.3 - we still need to work on the v1.2 open bugs plus committed features.

(In reply to Wilfred Mathanaraj [:WDM] from comment #9) > We wont have time to address this in v1.3 - we still need to work on the > v1.2 open bugs plus committed features. Thanks for the feedback, Wilfred.

Samll update, in the not yet landed NFC Manager (Bug 860910), one of the recent changes was to remove the need to parse the multiple possible VCards formats inside NFC Manager. It was done this way: formatVCardRecord: function nm_formatVCardRecord(record) { var vcardBlob = new Blob([NfcUtil.toUTF8(record.payload)], {'type': 'text/vcard'}); var activityText = { name: 'import', data: { type: 'text/vcard', blob: vcardBlob } }; return activityText; }, There's a confirmation screen on the other end in the contacts app, so it seems reasonable enough until more UX design is done for NFC P2P communication. If contacts wants to know the source of it, the current way is to register to receive "nfc-ndef-discovered" messages, and check if it's from a P2P source.

> If contacts wants to know the source of it, the current way is to register > to receive "nfc-ndef-discovered" messages, and check if it's from a P2P > source. The best way, is to register for P2P callbacks (when the code lands...). Something like this: navigator.mozNfc.onpeerfound = function foo(nfcPeerObject) { // do something... nfcPeerObject.sendNDEF(Contacts_NDEF_Message_Vcard_to_Other_Device); };

Hi Garner, I integrated the https://github.com/mozilla-b2g/gaia/commit/5376482f9e7f1724195001146eb132f98dd53209 nfc_manager.js. I tested with Android device by transferring contact info. 1. It reached the nfc_manager.js in formatMimeMedia: function nm_formatMimeMedia(record) 2. It has checking condition to execute the formatVCardRecord: function nm_formatVCardRecord(record) . 3. This condition gets failed and did not stored into contact information locally. 4. I enabled log on the record.type, It gives "text/x-vCard" instead of "text/vcard". [116,101,120,116,47,120,45,118,67,97,114,100] I changed the condition like, if ( (NfcUtil.equalArrays(record.type, NfcUtil.fromUTF8('text/x-vCard'))) || (NfcUtil.equalArrays(record.type, NfcUtil.fromUTF8('text/vcard'))) ) { .... } The above condition is working and able to store the contact information from another NFC device. please confirm it.

http://tools.ietf.org/html/rfc6350#section-10.1, Relevant section: Interoperability considerations: The text/vcard media type is intended to identify vCard data of any version. There are older specifications of vCard [RFC2426][vCard21] still in common use. While these formats are similar, they are not strictly compatible. In general, it is necessary to inspect the value of the VERSION property (see Section 6.7.9) for identifying the standard to which a given vCard object conforms. In addition, the following media types are known to have been used to refer to vCard data. They should be considered deprecated in favor of text/vcard. * text/directory * text/directory; profile=vcard * text/x-vcard So, we'd need to ask the contacts team to see if they want to support that mime-type. Probably yes, but it should probably be a deliberate decision for compatibility reasons.

Hi Ken, any plans for official alternate vcard format support? Alternate mimetypes for Vcards are pretty easy to add on the nfc_manager end.

Hi Sandip, can you please answer this question?

Currently we want to stick to the one single vcard format. For v1.4 consideration its important we stay with the MVP implementation. But for future releases this should definitely be one of the items to be considered for enhancements.

Update whiteboard tag to follow format [ucid:{id}, {release}:p{1,2}, ft:{team-id}]

Sami, are you going to take this bug? If not, I want to check if anyone can take it.

Hi Ken, yes I will take the bug. Please assign it to my name.

pull request : https://github.com/mozilla-b2g/gaia/pull/14595 Can you please review & provide feedback. I tested on Nexus-4 device. Reeving device ( Android Nexus-4, Lumia 920, Samsung-S3)

Comment on attachment 8346379 [details] [diff] [review] 0001-Bug-903245-Support-sharing-of-Contact-via-NFC-r-revi.patch Review of attachment 8346379 [details] [diff] [review]: ----------------------------------------------------------------- In general I see the following problems: - We already have a component to transform the contact to vcard. - In the contact detail check if the device has NFC support or not. - Add a method to listen to NFC events, but also remove it when you leave the detail view. Cheers! F. ::: apps/communications/contacts/js/contacts.js @@ +478,4 @@ > showForm(); > }; > > + var getCurrentContact = function c_getCurrentContact(){ I think Ben already commented this, but we have a component in shared/js/contact2vcard.js that transform a mozContact object into a vcard (format 4.0) Also the name of the function is not clear to me, as |getCurrentContact| is quite generic, would use |getCurrentContactAsVcard| As well I don't think we need this function in the 'contacts.js' file itself. ::: apps/communications/contacts/js/views/details.js @@ +68,5 @@ > '#details-back': handleDetailsBack, > '#edit-contact-button': showEditContact > }); > + > + window.navigator.mozNfc.onpeerready = function(event) { I understand with this code that no action is needed by the user when sharing a contact via NFC, you just need to be in the contact detail as requirement. Could we wrap this within a method that we call in the initialization? And also remove the listener when we are outside of the detail view? As well, what will happen with devices that don't support NFC? Will they have the mozNfc object? in that case we will need to check if the device supports or not nfc.

Comment on attachment 8346379 [details] [diff] [review] 0001-Bug-903245-Support-sharing-of-Contact-via-NFC-r-revi.patch I'm going to defer to Francisco on the review here as I'm quite buried with other bugs at the moment. Thanks Francisco!

(In reply to saminath from comment #20) > Hi Ken, yes I will take the bug. Please assign it to my name. Thanks and assigned.

1. To make vCard using ContactToVcard() function in contact2vcard.js 2. Addition/Removal of NfcPeerHandler method in Contact.js and details.js respectively 3. Various Test condition includes (mozNfc support, Valid Payload data, NDEF transfer status) 4. VCARD 4.0 Version Not Supported and NFC KEY ID Should be Upper Case (N,FN,BDAY,ADR,EMAIL). I changed into 2.1 VERSION. 5. setImmediate is not supporting now,Removed. 6. Tested against Nexus-4 & Samsung S4 Android device

1. To make vCard using ContactToVcard() function in contact2vcard.js 2. Addition/Removal of NfcPeerHandler method in Contact.js and details.js respectively 3. Various Test condition includes (mozNfc support, Valid Payload data, NDEF transfer status) 4. VCARD 4.0 Version Not Supported and NFC KEY ID Should be Upper Case (N,FN,BDAY,ADR,EMAIL). I changed into 2.1 VERSION. 5. setImmediate is not supporting now,Removed. 6. Tested against Nexus-4 & Samsung S4 Android device

pull request : https://github.com/mozilla-b2g/gaia/pull/14595/ Tested against Nexus-4 & Samsung S4 Android device

Hi, please take new PR from below link pull request : https://github.com/mozilla-b2g/gaia/pull/14706 Tested against Nexus-4 & Samsung S4 Android device

Is there a way of trying with unagi phones? If I remember correctly, those devices came with NFC support, wondering if I will be able to try with them.

Hi Francisco, Sorry, I do not have unagi phone. Today I checked with Samsung NOTE-II(Android-4.1.2). With VCARD Version 4.0, It receives the data, but not saved in the Contact. Dimi, Could you please helps out?

Sami, where is the code that receives the contact and calls mozContact.save()?

(In reply to Francisco Jordano [:arcturus] from comment #29) > Is there a way of trying with unagi phones? There is not NFC chip in unagi device...:-(. We use nexus 4 as a reference development phone.

| Sami where is the code that receives the contact and calls mozContact.save()? | Hi Ben, In Firefox, contact save is working. Implemented in gaia/apps/system/nfc_manager.js. formatMimeMedia() fill the action with vcard record, pass to MozActivity(). Please check: https://bugzilla.mozilla.org/show_bug.cgi?id=903245#c13

Comment on attachment 8348011 [details] [diff] [review] 0001-Bug-903245-Support-sharing-of-Contact-via-NFC-r-revi.patch Review of attachment 8348011 [details] [diff] [review]: ----------------------------------------------------------------- Sami, this is getting better and will be a great feature to have. I still have some concerns, though, so I need to r- for right now. Please ask me to re-review after you've taken a look at the comments. Also, I agree with Jose that we should probably try to isolate/modularize this code a bit more. Perhaps this could be moved to an nfc.js file within contacts. The contacts.js code would then just be responsible for initializing nfc.js and telling it when details are shown/hidden. ::: apps/communications/contacts/js/contacts.js @@ +250,5 @@ > + var contains = function contains(vcard) { > + var str; > + str = vcard; > + return ( (vcard.toLowerCase().indexOf(str.toLowerCase())) !== -1 ); > + }; As far as I can tell this function compares the vcard to itself. It will always return true, won't it? Should vcard and str be passed in separately? @@ +253,5 @@ > + return ( (vcard.toLowerCase().indexOf(str.toLowerCase())) !== -1 ); > + }; > + > + var nfcPeerHandler = function nfcPeerHandler (event) { > + var payload ; Nit: Please use 2-space indents throughout. @@ +260,5 @@ > + var count = 0; > + var res; > + > + var nfcdom = window.navigator.mozNfc; > + var nfcPeer = nfcdom.getNFCPeer(event.detail); Error check if nfcPeer is invalid here before proceeding? @@ +261,5 @@ > + var res; > + > + var nfcdom = window.navigator.mozNfc; > + var nfcPeer = nfcdom.getNFCPeer(event.detail); > + var records = new Array(); Nit: I think we prefer the literal |[]| instead of |new Array()|. @@ +262,5 @@ > + > + var nfcdom = window.navigator.mozNfc; > + var nfcPeer = nfcdom.getNFCPeer(event.detail); > + var records = new Array(); > + var tnf = 0x02; What is this magic value? @@ +266,5 @@ > + var tnf = 0x02; > + var type = 'text/x-vCard'; > + var id = ''; > + > + contact= currentContact ; Please handle the condition when |currentContact| is |null|. It seems we should be able to short-circuit here. @@ +280,5 @@ > + payload = str; > + } > + else{ > + return; > + } This |else { return; }| is not needed since you're in a callback. @@ +284,5 @@ > + } > + }); > + } > + ); > + if (payload) { Doesn't this block of code need to be up in the |ContactToVcard()| success callback? There is no guarantee that |ContactToVcard()| will run synchronously, so you may not have payload set correctly here. @@ +293,5 @@ > + payload); > + records.push(record); > + res = nfcPeer.sendNDEF(records); > + res.onsuccess = (function() { > + debug('contact data transfer successfully'); Should the success or failure of the transfer be reported back to the user on the screen? @@ +328,5 @@ > + > + if(window.navigator.mozNfc) { > + window.navigator.mozNfc.onpeerready = nfcPeerHandler; > + } > + }; This will enable the NFC peer handler (and if I understand automatic transfer to a ready peer) even if we fail to enter the details view. This should probably be moved up into |getContactById()| callback to ensure its only run if we actually navigate to details. ::: apps/communications/contacts/js/views/details.js @@ +86,5 @@ > } > } > + if(window.navigator.mozNfc) { > + window.navigator.mozNfc.onpeerready = null; > + } I think the code should be refactored in some way so that the |onpeerready| is set and cleared from the same code module. This helps ensure that the state is kept consistent. So, can you move this to |contacts.js| in someway? Perhaps with a callback indicating that details have closed if something like that doesn't already exist? ::: shared/js/contact2vcard.js @@ +26,4 @@ > /** Field list to be skipped when converting to vCard */ > var VCARD_SKIP_FIELD = ['fb_profile_photo']; > > + var VCARD_VERSION = '2.1'; This is going to change all of our vcard exports to 2.1. Do we really want to do that? We should probably make this code support either 2.1 or 4.0 via a parameter.

And thank you for working on this!

Comment on attachment 8348015 [details] [diff] [review] 0001-Bug-903245-Support-sharing-of-Contact-via-NFC-r-revi.patch This time I'm deferring the review in Ben's one. We are getting closer, once you add comments that Ben did we will be almost ready. Thanks a lot for working in this feature!

(In reply to saminath from comment #33) Hi, Saminath, we are highly appreciated for your help to take this bug. Thank you very much! Sorry that I need to move the ownership to Michal due to its high priority. Would you like to own other NFC feature work or bugs? Or you would like to own other Firefox OS parts? Thank you.

Hi Ben, I addressed all your comments and would be sending PR.

Comment on attachment 8350031 [details] [diff] [review] 0001-Bug-903245-Support-sharing-of-Contact-via-NFC-r-revi.patch Review of attachment 8350031 [details] [diff] [review]: ----------------------------------------------------------------- It's a bit weird, this patch is not against the original master branch, seems it's against another previous patch so cannot evaluate it correctly. Thanks again!

Hi Ben/francisco, Here with I attached the Patch, addressed with previous comments. Also Find the PR https://github.com/mozilla-b2g/gaia/pull/14927. This Bugs assigned to Michal, Thanks for your support.

(In reply to Kevin Hu [:khu] from comment #37) Hi Kevin, I submitted , What ever I completed. Presume that it could be useful. Thanks for your support.

(In reply to saminath from comment #42) > (In reply to Kevin Hu [:khu] from comment #37) > > Hi Kevin, > > I submitted , What ever I completed. Presume that it could be useful. > > Thanks for your support. Wow, thank you very much, saminath.

Hi Michal, thanks for your great help. Are you working on this bug? When do you plan to finish it? We would like to have a NFC demo during NFC workweek.

Hi folks, any idea about the ownership of this bug and if we should review or wait till Michal gives the current patch a look? Cheers, F.

Comment on attachment 8351126 [details] [diff] [review] 0001-Bug-903245-Support-sharing-of-Contact-via-NFC-r-revi.patch I spoke with Francisco and we decided to drop the review flags until Michal has had a chance to look at the patch. Sami, we greatly appreciate your work here! I'm sorry for the confusion on this bug. The lack of hardware made it difficult for me and Francisco to test patches. Also, the fact that its on our release roadmap created some time pressure not present on other bugs. I hope our delays reviewing and the change of ownership does not discourage you from continuing to contribute.

(In reply to Francisco Jordano [:arcturus] from comment #45) > Hi folks, > > any idea about the ownership of this bug and if we should review or wait > till Michal gives the current patch a look? > > Cheers, > F. I saw this conclusion, Michal is the owner for this bug, in email.

Is it possible to have this feature done during NFC work week?

No more blocks 1.3 committed but blocks 1.4 committed feature

remove 1.4 flag as this is a feature work. let's use user story or metabug to keep tracking.

I've attached the work in progress patch, it's working but without test. I'll add them later during the weekend.

Comment on attachment 8376634 [details] Patch Tests added, ready to r.

Can we get this reviewed at the earliest please? (Making the builds for an upcoming conference)

Already sent an email to Francisco to double confirm his availability. I believe he will do it as his earliest convenience.

Comment on attachment 8376634 [details] Patch Great job Michal! I finished doing the first round, some comments to address in the PR. Will love to see a second round for r+plusing. Specially in the comments related to not enable or load features in the phones with no NFC. I have also another question, mainly for product, we have several ways to accessing a contact detail, for example web activities, (we could be seeing a contact from a sms), right now we could share no matter what as long as we are in that view, is that the expected behavior? Also, I don't have a NFC capable device, could someone from the Paris office give it a try once we do the code review? Cheers!

Thank you Francisco, I fixed or commented most of your concerns. Could you pls r again?

Sure, let's take a look :)

Comment on attachment 8376634 [details] Patch Looking good to me, I tried on a non nfc phone, working great and ask Anthony to check the solution in nfc devices and he gave a positive feedback. r+ here, great job, please squash all the commits and merge!

Michal, I wonder if it was possible to land this by today?

It is ready to land since Friday, I get r+ yesterday but couldn't land it because the tree is closed. I'll land as soon as I will be able to.

Hi Michal, Thanks for the feature. I was trying out the code for the MWC demo, and found the onpeerready callback appears (from behavior only, not from looking at the code) to only be set on an existing user's details screen/view. It may be more useful if that callback is set on the possibly empty contacts list view itself, instead of the details of an unrelated contact? Also, there's no explicit nfc-ndef-discovered activity handler for vcards. This is somewhat fine as there is an explicit case for text/vcard in nfc_manager (but not the rest like text/x-vCard, etc).

^^ and Saminath too. :-)

But according to the spec we can only share one contact at a time, so I don't understand why do we want to set the callback also on the list. Could you pls be a little bit more clear there? NFC manager sent 'import' activity when we send text/vcard file, so Contacts app use importing action to receive sent contact. I don't see a point in duplicating this part of code to achieve the same thing.

And according to the new landing rules this patch cannot land till we will branch 1.4

(In reply to Michal Budzynski (:michalbe) from comment #64) > But according to the spec we can only share one contact at a time, so I > don't understand why do we want to set the callback also on the list. Could > you pls be a little bit more clear there? Actually, yes, we shouldn't send the whole contacts book :) So, no code changes are needed here, my comment is not relevant. To clarify what I was thinking: what might be nice, UX wise, is that each user can get a visual feedback on whether the devices are paired, separately from whether a particular user's phone is even able to send anything. It's not exactly obvious, on the ever larger phones these days, where the NFC antenna on the back of the device is. If the 2 phones are touching, it's not clear which phone vibrated either, so a visual "Phones are paired, but receiving only" is nice. This doesn't happen on Android, but we can potentially clear up this use case on the FirefoxOS side.

Yes, it sounds good, but I think this should be handled by the system app, not by all the apps itself.

remove target milestone. NFC sharing is not key focus in 1.4, so we should defer it to 1.5.

I wonder if we could restart this bug now?

Wesley and Joe, I wonder if this feature is in Comms team's backlog.

NFC is one of 1.5 features.

Wilfred, can u pls add to comms team 1.5 list. thx

its comms team component and its "backlog" in blocking flag so yes it will show up in comms team backlog

Quick quesetion, was this merged to 1.3T? In that case how we check that the mighty merge that will happen between 1.3T and master will work for this?

(In reply to Francisco Jordano [:arcturus] from comment #74) > Quick quesetion, was this merged to 1.3T? > > In that case how we check that the mighty merge that will happen between > 1.3T and master will work for this? No, it isn't in 1.3T yet. and we don't have plan to merge it to 1.3T.

Hi Michal, As the patch was there weeks ago, is it possible to land it to master recently? Not sure if rebase takes huge efforts. I know 1.5 has not started yet, but early landing gives us more time to test.

The work was done in v1.4 but the feature was not landed in v1.4 due to the new guidelines for v1.4. We can land this for v1.5 and I dont see any issue landing it in v1.5. I am leaving NI michal to see if there is any other work pending than landing the patch.

Wilfred, Patch is r+ for sometime, but it still somehow makes travis red, I'll investigate it today and land asap.

Patch merged [1], thanks everyone! [1] https://github.com/mozilla-b2g/gaia/commit/e7c182fed0e17e2d8b4739add6eaea6b4ebd1327