1138377 - [B2G][Contacts] Unable to view some fields from a downloaded vcf file

Status: VERIFIED FIXED

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

Description

[Noemí Freire (:noemi)]

Attachment: Test_Case_Preview.png

STRs:
1. E-mail with .vcf file is received in FxOS device from an IOS (8.0) device
2. Click on the received vcard attachment to download the corresponding .vcf file
3. After completing the Download, click on the notification

Actual Result: The vcard is opened via Contacts app, but e-mail address and address fields are not shown in the preview (see "Test_Case_Preview.png" image attached)


Expected Result: The vcard is opened via Contacts app and all the contact fields are properly shown


Environmental Variables:
Device: Flame Master
Build ID: 20150302073244    
Gaia: cf3b761
Gecko: 75fc721
Platform Version: 39.0a1 (Master)
Firmware Version: v18D

Comment 1

[Noemí Freire (:noemi)]

Attachment: Test Case.vcf

.vcf file

Comment 2

[GitHub Autolander Bot]

Attachment: [gaia] ADLR-es:vcard-groups > mozilla-b2g:master

Comment 3

[Adrián de la Rosa]

Comment on attachment 8571431 [details] [review]
[gaia] ADLR-es:vcard-groups > mozilla-b2g:master

Problem caused by grouping properties. I learned that Apple vCards use property grouping a lot, mainly to group a standard property with a non-standard property that categorizes the first one instead of using standard types. Since it's a proprietary implementation, it's really difficult to know how they are doing it to be able to cover all the cases.

The patch just makes the vCard reader component to forget about groups. The effect is that standard properties are correctly read, even when grouped, and non-standard ones are ignored. This solves the bug, showing email and address that were previously missing, but since the reader ignores custom properties, some information like mail type could be lost in the process when it's stated in the file in a non-standard way, like Apple does.

Comment 4

[Jose Manuel Cantera]

Comment on attachment 8571431 [details] [review]
[gaia] ADLR-es:vcard-groups > mozilla-b2g:master

handing over the review to Sergi, as he developed the vCard parsing code.

Comment 5

[Francisco Jordano [:arcturus] [:francisco]]

Hi Adrian,

can we add a tiny unit test for this?

Thanks!

Comment 6

[Adrián de la Rosa]

Good idea, I'm on it!

Comment 7

[Adrián de la Rosa]

Unit test for this kind of vCards added to the PR. We just need to wait for Sergi to review it.

Thanks!

Comment 8

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Comment on attachment 8571431 [details] [review]
[gaia] ADLR-es:vcard-groups > mozilla-b2g:master

Looks good, thanks Adrián!

Comment 9

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/f9d18900d52e2d460a949cd9b226edccd926b775

Comment 10

[Adrián de la Rosa]

Comment on attachment 8571431 [details] [review]
[gaia] ADLR-es:vcard-groups > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): vCard reader component
[User impact] if declined: Parsing vCards from many providers (iOS, Gmail...) will ignore some fields due to being grouped. Right now our vCard reader doesn't support grouped properties in vCard files.
[Testing completed]: Unit test included.
[Risk to taking this patch] (and alternatives if risky): Low, small change and tests provided. The alternative is to keep ignoring some fields from some vCards.
[String changes made]: None.

Comment 11

[Noemí Freire (:noemi)]

(In reply to Adrián de la Rosa from comment #10)
> Comment on attachment 8571431 [details] [review]
> [gaia] ADLR-es:vcard-groups > mozilla-b2g:master
> 
> [Approval Request Comment]
> [Bug caused by] (feature/regressing bug #): vCard reader component
> [User impact] if declined: Parsing vCards from many providers (iOS,
> Gmail...) will ignore some fields due to being grouped. Right now our vCard
> reader doesn't support grouped properties in vCard files.
> [Testing completed]: Unit test included.
> [Risk to taking this patch] (and alternatives if risky): Low, small change
> and tests provided. The alternative is to keep ignoring some fields from
> some vCards.
> [String changes made]: None.

just adding that the current flow already available in 2.2 for importing contacts from vcard via Messaging app is affected by this issue. Thanks!

Comment 12

[Maria Angeles Oteo (:oteo)]

Checked with latest master build and it's working as expected, I am able to see the e-mail and address fields of the vcard when it's received via e-mail or MMS

Environmental variables:
Flame 3.0
BuildId: 20150308082154
Gecko: d5d934b
Gaia: 373c9dd
Platform version: 39.0a1
Firmware Version: v18D

Comment 13

[Maria Angeles Oteo (:oteo)]

Nominating to 2.2.

Right now in 2.2 we are able to see the received vcard (via messaging or e-mail app) without e-mail address and address fields, upifting this we will able to see all the fields and have the functionality completed.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/92840c7f715cd993f70483fe9aa5a0167f1da013

Comment 15

[Pi Wei Cheng [:piwei] (inactive)]

This issue is verified fixed on Flame 2.2 and 3.0. As opposed to bug behavior shown at comment 0's screenshot, now when user taps to open the attached .vcf file, they're being taken to an Add Contact screen with name, home & work phone numbers, email address, home address, and birthday pre-filled on screen. User can tap on Done button to add this contact and all the information is saved.

Device: Flame 3.0
BuildID: 20150511010202
Gaia: 6089234ace8b294a8feef064387604bae16254e3
Gecko: d8420a541d1c
Gonk: 040bb1e9ac8a5b6dd756fdd696aa37a8868b5c67
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0

Device: Flame 2.2
BuildID: 20150511002500
Gaia: 528ef60e7cda09ad43478065f5d33bda398fbeb7
Gecko: 8d04cc085cf5
Gonk: ab265fb203390c70b8f2a054f38cf4b2f2dad70a
Version: 37.0 (2.2) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0