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
4.32 MB, application/pdf
4.92 MB, application/pdf
46 bytes, text/x-github-pull-request
|Details | Review|
2.96 MB, text/plain
2.11 MB, video/mp4
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
8 years ago
Wesley, this is a critical bugs.
blocking-b2g: backlog → 2.0+
Michal, could you help?
Component: NFC → Gaia::Contacts
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?
I'm on it now.
(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?
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.
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.
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.
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?
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 . Is this acceptable? I talked to Vivien about this and opened a bug for change in the mozNfcAPI .  https://github.com/michalbe/gaia/commit/c1413dba3590c06f24493fd2280573a54916909c  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.
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.
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.
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.
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.
It looks good. I'm working on it now. Thanks everyone.
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.
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
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!
You need to log in before you can comment on or make changes to this bug.