996527 - [NFC] Could not share contacts

Status: VERIFIED FIXED

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

Description

[Alison Shiue]

Using most up-to-date pvt build (2014/4/14) to test
Two phones(Device A, Device B) with NFC
Device A has some contacts information

STR:
1. Enable NFC in settings
2. Device A open specific contact info
3. Touch two phones together
4. Swipe screen on Device A to share specific contact

Expected result:
Device B receive contact info shared by Device A

Actual result:
Device B receive nothing

Comment 1

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Yoshi, 
Is the unexpected situation happening in GECKO or contact app?

Comment 2

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Gecko has delievered the NDEF to Gaia successfully.
And last time I tested, I think the Contacts app on Device B is launched, and but Comment 0 says "Device B receive nothing".

I think there should be something wrong in Gaia.

Comment 3

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Alright. Calling out to Michal for comments.

Comment 4

[Joe Cheng [:jcheng] (please needinfo)]

triage: NFC contact sharing is committed for 2.0, so this bug stays 2.0+

Comment 5

[Michał Budzyński (:michalbe)]

I'm on it now, starting to investigate the problem with todays build

Comment 6

[Michał Budzyński (:michalbe)]

Indeed, somehow nothing happens. I'm investigating now.

Comment 7

[Michał Budzyński (:michalbe)]

According to ADB it's sending device bug, not the receiver:
 [JavaScript Error: "ReferenceError: setImmediate is not defined" {file: "app://communications.gaiamobile.org/shared/js/contact2vcard.js" line: 327}]

Comment 8

[Michał Budzyński (:michalbe)]

It's probably because setImmediate.js file is lazyload only when export view is loaded, working on it now.

Comment 9

[Michał Budzyński (:michalbe)]

I've added missing file [1] so everything is sent now, but Contacts app that receives the contact cannot parse it properly, checking it now.

[1] https://github.com/michalbe/gaia/tree/b6527-nfc

Comment 10

[Michał Budzyński (:michalbe)]

It's a regression made probably by separating FTU app from Comms, investigating it now with :arcturus

Comment 11

[Michał Budzyński (:michalbe)]

The problem was made by commented HTML section that was not lazyloaded correctly on import, with [1] it should work as expected.

[1] https://github.com/michalbe/gaia/commit/cb69f239b33f546da5504cc00106e3027d864569

Comment 12

[Michał Budzyński (:michalbe)]

Attachment: Screen

Now when we receive contacts the app try to display ContactsDetail view, but because of the missing 'id' parameter instead of full contact we have blank details view like on the screen.

Comment 13

[Michał Budzyński (:michalbe)]

according to the research made by fcampo, the Vcard parser have changed couple of months ago. We now send total number of contacts in callbacks, not the contacts. Working on fix now

Comment 14

[Michał Budzyński (:michalbe)]

The commit that changed the behavior: https://github.com/mozilla-b2g/gaia/commit/b1330c05d8c433f6749dccae6e3616686b232766

Comment 15

[Michał Budzyński (:michalbe)]

As :fcampo suggested, opening contacts list instead of details (like when we import multiple contacts) solves the problem for now [1].


[1] https://github.com/michalbe/gaia/commit/13d86acd24345b37fbe69672bb8dd31b8a6ed916

Comment 16

[Michał Budzyński (:michalbe)]

Attachment: Patch

Comment 17

[Michał Budzyński (:michalbe)]

During the IRC discussion with :arcturus & :fcampo we decided that instead of submitting temporary workaround we want this to be fixed as it should be fixed. I provided a WIP patch that opens detail-view when importing one contact & list-view on multiple contacts. This change makes couple of tests fail, so I'm preparing a fix now. We will also test it manually in all the places where VCard parser is used - NFC, importing from SDCard & bluetooth sharing.

[1] https://github.com/michalbe/gaia/commit/ace22bb5296930f481e0edd0e9832bd5ac17acbc

Comment 18

[Michał Budzyński (:michalbe)]

Contacts sharing via NFC works now as expected and described in previous comment, failing unit tests are fixed and WIP patch prepared [1]. I'm testing for regressions now (in bluetooth & contact import from SDCard)

[1] https://github.com/michalbe/gaia/commit/e2143886f800db7a03c7467fe816ccc1c0289cdb

Comment 19

[Michał Budzyński (:michalbe)]

After merging with master nothing seems to work because some files from contacts are moved to shared, fixing it now.

Comment 20

[Michał Budzyński (:michalbe)]

Paths in Lazyloades changed now[1], everything works as expected, I'm not polishing the patch and will send it for R immediately.

[1] https://github.com/michalbe/gaia/commit/76d2f3faac1300dce2edd4bceed699d98bed7d14

Comment 21

[Michał Budzyński (:michalbe)]

Attachment: Final Patch

Comment 22

[Michał Budzyński (:michalbe)]

nit changes in the patch as asked by the reviewer [1]. Additional check [2] is not needed, the function already check if activity is correct couple of line above [3].

[1] https://github.com/mozilla-b2g/gaia/pull/18689#discussion_r12030307
[2] https://github.com/mozilla-b2g/gaia/pull/18689#discussion_r12030303
[3] https://github.com/mozilla-b2g/gaia/pull/18689/files#diff-c421fae418bc6eabdd907455c45e768eR79

Comment 23

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

Comment on attachment 8412655 [details] [review]
Final Patch

Excellent job!

Tried with the bluetooth export and working great.

Thanks Michal!

Comment 24

[Michał Budzyński (:michalbe)]

Thanks Francisco.
I'm waiting now for :fcampo review.

Comment 25

[Michał Budzyński (:michalbe)]

Fernando, you work on a similar change afaik, could you pls check if our ideas are compatible?

Comment 26

[Fernando Campo (:fcampo)]

Comment on attachment 8412655 [details] [review]
Final Patch

Tested and working fine.

Just a big kudos to Michał for spotting the importing bug, fixing and improving the tests :)

Can't believe we've been months with this unspotted.

Next beer is on me mate.

Comment 27

[Michał Budzyński (:michalbe)]

Thanks Fernando!

I'm waiting for Travis to finish and merge.

Comment 28

[Michał Budzyński (:michalbe)]

Travis is green, merging [1]. Thanks everyone for great support and cooperation, especially Francisco & Fernando!

[1] https://github.com/mozilla-b2g/gaia/commit/0713998f60edd0185bc82dde68c08d5df10198fa

Comment 29

[Alison Shiue]

Gaia      db3bcec51a361daddb7d3d4ba4d8a2a664b7b6aa
Gecko     https://hg.mozilla.org/mozilla-central/rev/90267d87c375
BuildID   20140429160215
Version   32.0a1