Closed Bug 1119738 Opened 6 years ago Closed 6 years ago
[Contacts] Pick vcard Activity should not return Facebook data
STR: MMS App, Attach Contact, Facebook Contact. We should not allow to attach Facebook Contacts in an MMS Message.
Asking blocking as contractually we should not allow to export FB Contacts to any media.
blocking-b2g: --- → 2.2?
Hi Jose, Does it actually share the data out? If so, we should block since the case is worse than bug 925988.
(In reply to Wesley Huang [:wesley_huang] (EPM) (NI me) from comment #3) > Hi Jose, > Does it actually share the data out? yes, > If so, we should block since the case is worse than bug 925988. I think so
Comment on attachment 8546536 [details] [review] GH PR 27274 Hi, I tried out the patch and works perfectly, but was wondering if it's necessary that we add another facebook dependencies to the activities file. Can we move this dependency to a class that already contains the FB dependency, and make this 'selection' a bit more agnostic. I mean, something like let the list, or the list model to decide if you can select an element. And if not giving a 'reason'. So later on we could reuse this in different cases to allow or disallow (better said) the selection of some elements. I think that we have another patch to gray out this kind of contacts too, made the same comment, try to do the selection generic. In that case is for the select mode, but kinds of apply here too. Also the wording: 'The contact cannot be shared due to Facebook constraint.' sounds weird to me.
(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #5) > Comment on attachment 8546536 [details] [review] > GH PR 27274 > > Also the wording: 'The contact cannot be shared due to Facebook constraint.' > sounds weird to me. asking Carrie for her input here. Thanks!
removing ni to Carrie since the final idea is FB contacts appearing "disabled". Sorry for the misunderstanding.
Pull request with the proposed new approach. Facebook contacts are disabled when trying to pick one of them instead of showing an error.
Comment on attachment 8549640 [details] [review] PR We need more work on the PR thanks
Comment on attachment 8549640 [details] [review] PR Everything pointed out in comments has been solved. Works as expected on device.
Comment on attachment 8549640 [details] [review] PR We need another round. Please address all the comments on GH thanks!
Comment on attachment 8549640 [details] [review] PR Everything fixed. Thanks!
Comment on attachment 8549640 [details] [review] PR the code is not what was suggested at https://github.com/mozilla-b2g/gaia/pull/27419#discussion_r23208018 particularly it does not make any sense that activityType is not defined neither it is an array
Attachment #8549640 - Flags: review?(jmcf) → review-
Comment on attachment 8549640 [details] [review] PR Code updated to work with arrays or strings.
Attachment #8549640 - Flags: review- → review?(jmcf)
Comment on attachment 8549640 [details] [review] PR please address the nits on GH good work! thanks
Attachment #8549640 - Flags: review?(jmcf) → review+
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Comment on attachment 8549640 [details] [review] PR [Approval Request Comment] [Bug caused by] (feature/regressing bug #): contact vcard attachments [User impact] if declined: User impact is not high but the very high impact is on the current agreements with Facebook in order no to share FB data with the outside world [Testing completed]: unit tests are present in the patch [Risk to taking this patch] (and alternatives if risky): No alternative is possible [String changes made]: None
Attachment #8549640 - Flags: approval-gaia-v2.2?
Attachment #8549640 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
This issue has verify successfully on Flame 2.2. See attachment: verified_Flame2.2.mp4. Reproduce rate: 0/5. STR： Prerequisites： There are some Facebook contacts in Contact app. 1. Launch SMS. 2. Create a new message. 3. Tap "Attachments" button, select "Communications". 4. Check the status of Facebook contacts. **They all are gary, can't be selected. Flame2.2 build version: Gaia-Rev 0518f4581a0925c0b703d730ef289ab15cbd1216 Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c6aa604a7967 Build-ID 20150125002503 Version 37.0a2 Device-Name flame FW-Release 4.4.2 FW-Incremental eng.cltbld.20150125.035924 FW-Date Sun Jan 25 03:59:36 EST 2015 Bootloader L1TC000118D0
This issue has been verified successfully on Flame v3.0 See attachment:verify_v3.0.MP4 Rate:0/5 Flame 3.0 build: Gaia-Rev b02ec9713e6de8d96c6954d2c0dfd0442b0656ac Gecko-Rev https://hg.mozilla.org/mozilla-central/rev/38e4719e71af Build-ID 20150127010228 Version 38.0a1 Device-Name flame FW-Release 4.4.2 FW-Incremental eng.cltbld.20150127.043726 FW-Date Tue Jan 27 04:37:39 EST 2015 Bootloader L1TC000118D0
You need to log in before you can comment on or make changes to this bug.