[B2G] [Contacts] Facebook contacts are present when choosing which contacts to export

VERIFIED FIXED in 2.2 S4 (23jan)

Status

defect
VERIFIED FIXED
6 years ago
4 years ago

People

(Reporter: ckreinbring, Assigned: hola)

Tracking

unspecified
2.2 S4 (23jan)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.2+, b2g-v1.4 affected, b2g-v2.0 affected, b2g-v2.1 affected, b2g-v2.2 verified, b2g-master verified)

Details

(Whiteboard: permafail)

Attachments

(2 attachments, 1 obsolete attachment)

Description:
Facebook contacts are present on the Export Contacts pages.

Repro Steps:
1) Update Buri to Build ID: 20131011004001
2) Launch the Contacts app.
3) If there aren't already contacts imported from Facebook, import some from the Settings page.
4) Select Settings then Export Contacts.
5) Choose any of the export options.
6) Look for the Facebook contacts on the Export page.

Actual:
Facebook contacts are present on the Export Contacts page.

Expected:
Facebook contacts are filtered out of the Export Contacts page.

Environmental Variables
Device: Buri 1.2 mozilla RIL
Build ID: 20131011004001
Gecko: http://hg.mozilla.org/releases/mozilla-aurora/rev/c8e97fd5b94d
Gaia: 79abf09f2b5b6440f43cb5ae44ef6c85c0437e8d
Platform Version: 26.0a2

Notes:
Repro frequency: 100%
Link to failed test case: https://moztrap.mozilla.org/manage/cases/?filter-id=9785
See attached screenshot
Why is this necessary?
Whiteboard: burirun2 → burirun2, burirun3
Whiteboard: burirun2, burirun3 → burirun2, burirun3, burirun1.3-3
yes, they are present but its info is not actually exported.

Closing worksforme
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
Duplicate of this bug: 1108148
Carrying over the flags from bug 1108148.

I don't think a Facebook contact should be displayed in the export list as it may confuse an end user. Or at least, we should display an error message to explain to a user that the contact data will be partially exported.

Currently, we export a FB contact's name via BT or memory card without any warning. A user will probably think that everything is correctly exported.

Jose, I think this is still an issue, what's your take on this?
Status: RESOLVED → REOPENED
Flags: needinfo?(jmcf)
Resolution: WORKSFORME → ---
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(dharris)
Whiteboard: burirun2, burirun3, burirun1.3-3 → permafail
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(dharris)
(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #4)
> Carrying over the flags from bug 1108148.
> 
> I don't think a Facebook contact should be displayed in the export list as
> it may confuse an end user. Or at least, we should display an error message
> to explain to a user that the contact data will be partially exported.
> 
> Currently, we export a FB contact's name via BT or memory card without any
> warning. A user will probably think that everything is correctly exported.
> 
> Jose, I think this is still an issue, what's your take on this?

We keep the FB Contacts in order to render the list faster. Filtering out FB contacts will make the list build longer affecting performance.
Flags: needinfo?(jmcf)
I understand the performance concern. I think we should at least show a warning when we export one or more FB contacts because a user would expect to have all the data exported.

Carrie, what behavior should we have here?
Flags: needinfo?(cawang)
Hi,

I think if we don't allow users export FB contacts, we should 
1. filter out the FB contacts 
or
2. keep the FB contacts unselectable (being greyed-out)
Flags: needinfo?(cawang)
[Blocking Requested - why for this release]: Even though this issue exists since the dawn of this project, the behavior is confusing for an end user who might think the functionality is broken. Moreover, if you export all your contacts, having FB contacts in the export list can lead to have a lot of absolutely useless entries in the generated vCard file. This will pollute the user's next importation and will create a lot a dupes at the next FB sync (as we don't automatically merge FB contacts).
blocking-b2g: --- → 2.2+
Oops, comment 8 was only a request.
blocking-b2g: 2.2+ → 2.2?
triage: keep the nomination for now and we'll go back to triage this after hearing feedbacks from developers regarding UX's suggestion.
Hi Maria,
Mind to let us know if you get any feedback?
Flags: needinfo?(oteo)
I think the best way to resolve this bug is as suggested by Carrie in comment 7, option 2. Actually it can be done using CSS, which should not have impact on performance.
Flags: needinfo?(oteo)
Jose  - can you have a look at fixing it but I would not block on it. If this is ready in time for 2.2 then we will pick it up based on risk.
blocking-b2g: 2.2? → -
Flags: needinfo?(jmcf)
yes, it should be ready for v2.2
Assignee: nobody → hola
Flags: needinfo?(jmcf)
Assignee: hola → jmcf
Assignee: jmcf → hola
Can we do this more generic.

Instead of adding a new class to the select list for exporting, that is pretty similar to selecting, I would like something generic for 'disabling clicking in fb contacts' no matter if we are exporting or not.

Then we can use that for, selecting ice contacts, or any other kind of action over contacts in the list.
See Also: → 1119738
(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #16)
> Can we do this more generic.
> 
> Instead of adding a new class to the select list for exporting, that is
> pretty similar to selecting, I would like something generic for 'disabling
> clicking in fb contacts' no matter if we are exporting or not.

That's certainly a good idea. 

> 
> Then we can use that for, selecting ice contacts, or any other kind of
> action over contacts in the list.
I think it's a backlog item rather than a minus. 
We would want it to be fixed.
blocking-b2g: - → backlog
Posted file Pull request
Attachment #8547597 - Flags: review?(jmcf)
Blocks: 1119738
triage: for it's blocking bug 1119738
blocking-b2g: backlog → 2.2+
Posted file Pull request (obsolete) —
Attachment #8548243 - Flags: review?(jmcf)
Attachment #8548243 - Attachment is obsolete: true
Attachment #8548243 - Flags: review?(jmcf)
Comment on attachment 8547597 [details] [review]
Pull request

please see the comments on GH. We need another round ...
Attachment #8547597 - Flags: review?(jmcf)
Attachment #8547597 - Flags: review?(jmcf)
Comment on attachment 8547597 [details] [review]
Pull request

please address the three pending comments on GH

good work!
Attachment #8547597 - Flags: review?(jmcf) → review+
https://github.com/mozilla-b2g/gaia/commit/bcc58499fa9429ee1a480f18357ba319c70bf505
Status: REOPENED → RESOLVED
Closed: 6 years ago5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S4 (23jan)
Comment on attachment 8547597 [details] [review]
Pull request

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Facebook restrictions for sharing data outside our device
[User impact] if declined: Medium, the user would expect her Facebook data exported but in reality it would not be exported. 
[Testing completed]: Yes, unit tests are provided 
[Risk to taking this patch] (and alternatives if risky): Low-risk patch
[String changes made]: None
Attachment #8547597 - Flags: approval-gaia-v2.2?
Keywords: verifyme
Attachment #8547597 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Issue verified fixed on Flame 2.2 and 3.0

Imported Facebook contacts are greyed out and unselectable in list when trying to export them. Locally stored non-Facebook contacts are still selectable in export list. Tested with Facebook contacts only as well as with a mix of facebook and non-facebook contacts.

Device: Flame 2.2
BuildID: 20150121093932
Gaia: e4f9b5da3751798f9cc5d95f302c30722cc11fca
Gecko: 53421e00294d
Version: 37.0a2 (2.2)
Firmware: V18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Device: Flame 3.0 Master
BuildID: 20150121141532
Gaia: 917b6c36717fddc6e71ffc1ec249633c8044c93c
Gecko: 06b590bf59f4
Version: 38.0a1 (3.0 Master)
Firmware: V18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Keywords: verifyme
Duplicate of this bug: 1145075
You need to log in before you can comment on or make changes to this bug.