Closed Bug 997599 Opened 8 years ago Closed 8 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)

VERIFIED FIXED
2.0 S3 (6june)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: ashiue, Assigned: mbudzynski)

References

Details

Attachments

(5 files)

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
blocking-b2g: --- → backlog
Wesley, this is a critical bugs.
blocking-b2g: backlog → 2.0+
Blocks: 894678
Michal, could you help?
Component: NFC → Gaia::Contacts
Flags: needinfo?(mbudzynski)
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.
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?
blocking-b2g: 2.0+ → 2.0?
Keywords: qawanted
I'm on it now.
Flags: needinfo?(mbudzynski)
Assignee: nobody → mbudzynski
(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?
Flags: needinfo?(ashiue)
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.
Flags: needinfo?(ashiue)
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.
Flags: needinfo?(ashiue)
But if we have non-FB data only, than the bug doesn't occur because we are allowed to send everything, right?
Hi Michal, you are right. Only FB data forbidden to share.
Flags: needinfo?(ashiue)
Keywords: qawanted
Ok, I will fix it after I'll finish working on Bug 886754 (later this week).
Bug 886754 is ready for r, so I'm working on this now.
Victoria, do we have a visuals for this? How this 'red flash overlay' (from page 8 of the PDF attached) should look like?
Flags: needinfo?(vpg)
Depends on: 1003268
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
(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.
Flags: needinfo?(vpg)
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.
This bug blocks NFC 2.0 release.
Blocks: b2g-NFC-2.0
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.
Flags: needinfo?(jhuang)
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.
Flags: needinfo?(jhuang)
Oops I put the wrong bug number. It should be bug 1003268.
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.
blocking-b2g: 2.0? → 2.0+
Summary: [NFC] Try to share facebook-linked data still shows shrinking UI → [NFC] When sharing contact info: Facebook data is forbidden to share, only local data allowed
Hi Michal, do you have any concern for this new UX design.
Flags: needinfo?(mbudzynski)
It looks good. I'm working on it now. Thanks everyone.
Flags: needinfo?(mbudzynski)
Blocks: 894676
No longer blocks: 894678
No longer depends on: 1003268
WIP info: Working on a patch, I'll submit something working later today.
Target Milestone: --- → 2.0 S3 (6june)
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.
Working on this now.
Attached file Patch
Attachment #8436891 - Flags: review?(francisco)
Prepared the patch, waiting for :arcturus to r. Hope we will be able to land it before the branching.
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!
Attachment #8436891 - Flags: review?(francisco) → review+
You mean 'on github' :)?

Thank you very much Francisco, I'm waiting for Travis to finish and land!
LANDED!
https://github.com/mozilla-b2g/gaia/commit/a17ab53867d95e755ec3d883183b197d5710dd88

Thanks everyone, especially Francisco for superfast review in the middle of his lunchtime :)
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Verified on
Gaia      a3a5322692578e0a577fb7fa08e32144b2b05ba3
Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/0293597de41f
BuildID   20140612160201
Version   32.0a2
Status: RESOLVED → VERIFIED
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
Flags: needinfo?(hlu)
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
Add NI?whsu to follow up.
Thanks everyone!

Happy New Year! :)
Flags: needinfo?(hlu) → needinfo?(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!
Flags: needinfo?(whsu)
You need to log in before you can comment on or make changes to this bug.