997599 - [NFC] When sharing contact info: Facebook data is forbidden to share, only local data allowed

Status: VERIFIED FIXED

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

Description

[Alison Shiue]

most up-to-date pvt build 2014/4/16

Prerequisite: at least 1 facebook-linked data in "Contacts", 2 phones with NFC & Bluetooth

STR:
1. Go to "Settings", Switch on NFC on both phones
2. Device A go to contacts and select one facebook-linked data.
3. Tap two phones together

Expected result: 
1. Once NFC has paired, a red flash will pop out for a second. 
2. A toast pops out on screen: The contact cannot be shared due to Facebook constraint.

Actual result:
Shrinking UI shows up

Comment 1

[Ken Chang[:kenkai]]

Wesley, this is a critical bugs.

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Michal, could you help?

Comment 3

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

By contract we cannot share FB information, but I guess if we have a linked contact that already contained that information we could send that contact, not the merged with the FB information.

Comment 4

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

QAWANTED to confirm if what's being is the non-FB data (since its a linked contacts, there could be non-FB data)

and let's put this back to 2.0?

Comment 5

[Michał Budzyński (:michalbe)]

I'm on it now.

Comment 6

[Jason Smith [:jsmith]]

(In reply to Joe Cheng [:jcheng] from comment #4)
> QAWANTED to confirm if what's being is the non-FB data (since its a linked
> contacts, there could be non-FB data)
> 
> and let's put this back to 2.0?

Alison - Can you address the QA Wanted request here?

Comment 7

[Alison Shiue]

Attachment: [NFC] Pairing Vcard_20131202.pdf

The spec defined the NFC sharing behavior for three contact types:
1. Local data (Manually created or imported from memory card ,...): Can share
2. Facebook data (Sync friend's profile from FB): Forbidden to share
3. Mix data (User can manually add some information on FB profile in contact, such as phone number, address, etc.): The local added data can share, but FB data is forbidden to share. Receiver would get only part of this mix profile.

Comment 8

[Jason Smith [:jsmith]]

Alison - That doesn't really answer the question about the QA Wanted request though. Joe is asking to see if this bug reproduces with non-FB data.

Comment 9

[Michał Budzyński (:michalbe)]

But if we have non-FB data only, than the bug doesn't occur because we are allowed to send everything, right?

Comment 10

[Alison Shiue]

Hi Michal, you are right. Only FB data forbidden to share.

Comment 11

[Michał Budzyński (:michalbe)]

Ok, I will fix it after I'll finish working on Bug 886754 (later this week).

Comment 12

[Michał Budzyński (:michalbe)]

Bug 886754 is ready for r, so I'm working on this now.

Comment 13

[Michał Budzyński (:michalbe)]

Victoria, do we have a visuals for this? How this 'red flash overlay' (from page 8 of the PDF attached) should look like?

Comment 14

[Michał Budzyński (:michalbe)]

After further investigation: The problem in here is that all we can do from the inside of the application using current mozNfcAPI is to register for the .onpeerready event and send data. Rest is managed by the system app. We don't know when the phones are tapped together, only the system app knows that, so we cannot display the overlay or the toaster. All I can do for now is just disabling sharing FB contacts without any notification for the user [1]. Is this acceptable?

I talked to Vivien about this and opened a bug for change in the mozNfcAPI [2].

[1] https://github.com/michalbe/gaia/commit/c1413dba3590c06f24493fd2280573a54916909c
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1003268

Comment 15

[Victoria Gerchinhoren [:vicky]]

(In reply to Michał Budzyński (:michalbe) from comment #13)
> Victoria, do we have a visuals for this? How this 'red flash overlay' (from
> page 8 of the PDF attached) should look like?

First time I see this. Why do we have a red flash there? Is it a convention or something related to NFC? For what I understand we need a feedback indicator that could either be a vibration or any other resource. So I would propose to have that flash being the whole screen turning into the same color we use for hitstates in Comms, that is #9ef2e7, in line with all phone hitstate color.

Thanks.

Comment 16

[Michał Budzyński (:michalbe)]

He have a vibration there already, just after two devices are paired. 

Thanks for your feedback, we will need to wait for Bug 1003268 to be fixed before solving this one anyway.

Comment 17

[Ken Chang[:kenkai]]

This bug blocks NFC 2.0 release.

Comment 18

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

Are we confident that Bug 1003268 can be done before FL?
If not, we should unblock the implementation at this point.
The MVP is skipping "red flash" and "toast message" addressed in page#8 (step3,4). 
ni? Juwei to see if this works.

Comment 19

[Juwei Huang[:juwei]]

Attachment: [NFC] Pairing Vcard_20140513.pdf

As our agreement through con-call, we skip the red hint in 2.0 due to the implementation of bug 1003628. The flow for solely Facebook contact would be:
1. Tap devices together
2. Sender: UI shrinks, swipe the UI up to share the contact
3. Sender: UI sends out, screen backs to contact detail view with a toast pops out to indicate the info is forbidden to share.
(Receiver will not get any information)

I put the original flow to appendix so we can have more discussion afterward depends on whether bug 1003628 is going to implement or not.

Comment 20

[Juwei Huang[:juwei]]

Oops I put the wrong bug number. It should be bug 1003268.

Comment 21

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

Per Juwei's input, we can keep on finishing the feature at this point.
This is a UX before bug 1003268 which I think requires longer discussion/implementation.
Let's allow the shrinkUI even for FB content, but deal it with constraints while sharing.

Comment 22

[Ken Chang[:kenkai]]

Hi Michal, do you have any concern for this new UX design.

Comment 23

[Michał Budzyński (:michalbe)]

It looks good. I'm working on it now. Thanks everyone.

Comment 24

[Michał Budzyński (:michalbe)]

WIP info: Working on a patch, I'll submit something working later today.

Comment 25

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

Comment 19 will be the expected behavior in 2.0.
Spoke with Alison and the current build is like:
1. Tap devices together
2. Sender: UI shrinks, swipe the UI up to share the contact
3. Sender: UI sends out, screen backs to contact detail view w/o a toast pops out. Receiver got info excluding those FB linked data

Step3 is what we expect to fix.

Comment 26

[Michał Budzyński (:michalbe)]

Working on this now.

Comment 27

[Michał Budzyński (:michalbe)]

Attachment: Patch

Comment 28

[Michał Budzyński (:michalbe)]

Prepared the patch, waiting for :arcturus to r. Hope we will be able to land it before the branching.

Comment 29

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

Comment on attachment 8436891 [details] [review]
Patch

Working perfeclty.

Left some minor comments on bugzilla, but nothing important.

Please merge once travis is green.

Thanks!

Comment 30

[Michał Budzyński (:michalbe)]

You mean 'on github' :)?

Thank you very much Francisco, I'm waiting for Travis to finish and land!

Comment 31

[Michał Budzyński (:michalbe)]

LANDED!
https://github.com/mozilla-b2g/gaia/commit/a17ab53867d95e755ec3d883183b197d5710dd88

Thanks everyone, especially Francisco for superfast review in the middle of his lunchtime :)

Comment 32

[Alison Shiue]

Verified on
Gaia      a3a5322692578e0a577fb7fa08e32144b2b05ba3
Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/0293597de41f
BuildID   20140612160201
Version   32.0a2

Comment 33

[Shally Li（leave from Mozilla）]

This issue has been failed verified on Flame 2.1&2.0.
See attachment: verify_v2.1.MP4 and logcat_1054&1056.txt
Reproducing rate: 10/10

STR:
1. Go to "Settings", Switch on NFC on both phones.
2. Device A go to contacts and select one facebook-linked data.
3. Tap two phones together.
4. Swipe to share the contact. 
**Screen backs to contact detail view. The local added data can share but FB data is forbidden to 
share. Receiver would get only part of this mix profile.

Expected result: 
4.A toast pops out on screen "The contact cannot be shared due to Facebook constraint".please see Comment 19.

Actual result:
4.There is no toast popping out to indicate that the info is forbidden to share.Comment 19

Flame2.1 build:
Gaia-Rev        5372b675e018b6aac97d95ff5db8d4bd16addb9b
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/f34377ae402b
Build-ID        20141127001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141127.035005
FW-Date         Thu Nov 27 03:50:16 EST 2014
Bootloader      L1TC00011880

Comment 34

[Shally Li（leave from Mozilla）]

Flame2.0 build:
Gaia-Rev        8d1e868864c8a8f1e037685f0656d1da70d08c06
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c756bd8bf3c3
Build-ID        20141127000203
Version         32.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141127.034818
FW-Date         Thu Nov 27 03:48:29 EST 2014
Bootloader      L1TC00011880

Comment 35

[Shally Li（leave from Mozilla）]

Attachment: logcat_1054&1056.txt

Comment 36

[Shally Li（leave from Mozilla）]

Attachment: verify_v2.1.MP4

Comment 37

[William Hsu [:whsu]]

Add NI?whsu to follow up.
Thanks everyone!

Happy New Year! :)

Comment 38

[William Hsu [:whsu]]

(In reply to Shally from comment #33)
> Expected result: 
> 4.A toast pops out on screen "The contact cannot be shared due to Facebook
> constraint".please see Comment 19.
> 
> Actual result:
> 4.There is no toast popping out to indicate that the info is forbidden to
> share.Comment 19

Hi, Shally,

Thanks for your help.
As Wesley mentioned (Comment 25), I think it is expected behavior.
Have a nice day!