Closed
Bug 1119738
Opened 6 years ago
Closed 6 years ago
[Contacts] Pick vcard Activity should not return Facebook data
Categories
(Firefox OS Graveyard :: Gaia::Contacts, defect)
Tracking
(blocking-b2g:2.2+, b2g-v2.2 verified, b2g-master verified)
People
(Reporter: jmcf, Assigned: hola)
References
Details
Attachments
(3 files, 1 obsolete file)
STR: MMS App, Attach Contact, Facebook Contact. We should not allow to attach Facebook Contacts in an MMS Message.
Reporter | ||
Comment 1•6 years ago
|
||
Asking blocking as contractually we should not allow to export FB Contacts to any media.
blocking-b2g: --- → 2.2?
Reporter | ||
Comment 2•6 years ago
|
||
Attachment #8546536 -
Flags: review?(francisco)
Comment 3•6 years ago
|
||
Hi Jose, Does it actually share the data out? If so, we should block since the case is worse than bug 925988.
Flags: needinfo?(jmcf)
Reporter | ||
Comment 4•6 years ago
|
||
(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
Flags: needinfo?(jmcf)
Comment 5•6 years ago
|
||
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.
Attachment #8546536 -
Flags: review?(francisco)
Comment 6•6 years ago
|
||
(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!
Flags: needinfo?(cawang)
Comment 7•6 years ago
|
||
removing ni to Carrie since the final idea is FB contacts appearing "disabled". Sorry for the misunderstanding.
Flags: needinfo?(cawang)
Reporter | ||
Updated•6 years ago
|
Assignee: jmcf → hola
Comment 8•6 years ago
|
||
triage: blocking per comment 4. And we'll also make bug 925988 2.2+ for consistency.
Updated•6 years ago
|
blocking-b2g: 2.2? → 2.2+
Assignee | ||
Comment 9•6 years ago
|
||
Pull request with the proposed new approach. Facebook contacts are disabled when trying to pick one of them instead of showing an error.
Attachment #8546536 -
Attachment is obsolete: true
Attachment #8549640 -
Flags: review?(jmcf)
Reporter | ||
Comment 10•6 years ago
|
||
Comment on attachment 8549640 [details] [review] PR We need more work on the PR thanks
Attachment #8549640 -
Flags: review?(jmcf)
Updated•6 years ago
|
status-b2g-v2.2:
--- → affected
status-b2g-master:
--- → affected
Updated•6 years ago
|
Target Milestone: --- → 2.2 S4 (23jan)
Assignee | ||
Comment 11•6 years ago
|
||
Comment on attachment 8549640 [details] [review] PR Everything pointed out in comments has been solved. Works as expected on device.
Attachment #8549640 -
Flags: review?(jmcf)
Reporter | ||
Comment 12•6 years ago
|
||
Comment on attachment 8549640 [details] [review] PR We need another round. Please address all the comments on GH thanks!
Attachment #8549640 -
Flags: review?(jmcf)
Assignee | ||
Comment 13•6 years ago
|
||
Comment on attachment 8549640 [details] [review] PR Everything fixed. Thanks!
Attachment #8549640 -
Flags: review?(jmcf)
Reporter | ||
Comment 14•6 years ago
|
||
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-
Assignee | ||
Comment 15•6 years ago
|
||
Comment on attachment 8549640 [details] [review] PR Code updated to work with arrays or strings.
Attachment #8549640 -
Flags: review- → review?(jmcf)
Reporter | ||
Comment 16•6 years ago
|
||
Comment on attachment 8549640 [details] [review] PR please address the nits on GH good work! thanks
Attachment #8549640 -
Flags: review?(jmcf) → review+
Reporter | ||
Comment 17•6 years ago
|
||
https://github.com/mozilla-b2g/gaia/commit/1296f6713a5274534347ae5db7696c396fefc2d6
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 18•6 years ago
|
||
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?
Updated•6 years ago
|
Updated•6 years ago
|
Attachment #8549640 -
Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Comment 19•6 years ago
|
||
v2.2: https://github.com/mozilla-b2g/gaia/commit/310a45c0b0ead803f55c7cd1c593d1c11ba979b0
Comment 20•6 years ago
|
||
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
QA Whiteboard: MGSEI-Triage+
Comment 21•6 years ago
|
||
Comment 22•6 years ago
|
||
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.
Description
•