1736289 - Trying to save vCard mobile number, saves Work phone instead

Status: NEW

(Thunderbird :: Address Book, defect)

Description

[bn_kamalesh]

Attachment: thunderbirdbug.png

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:93.0) Gecko/20100101 Firefox/93.0

Steps to reproduce:

Accounts settings, for a Gmail account. Tried adding a vCard, added phone number in "Mobile" field.

Actual results:

After saving, and opening the edit dialogue box again. It shows the phone number in "Work" field

Expected results:

The phone number should have showed up under "Mobile"

Comment 2

[Ralph Day]

I am seeing the exact same scenario as bug 1737779 (marked as a dup of this bug) with version 102.2 which causes a big problem as TbSync (which works correctly) hasn't been updated for 102.2. This forced me to fall back to 91 so I could continue to use TbSync. In any event, I believe the source of this problem is that TB is following the examples in VCARD 4.0 RFC 6350 rather than the ABNF when generating VCARD's for CardDAV. Specifically it is enclosing comma separated TYPE parameter lists in double quotes. See RFC 6350 errata 3488 for a discussion of why this should not be done and the ambiguity it introduces. https://www.rfc-editor.org/errata_search.php?rfc=6350&eid=3488 But worse, TB will do this claiming VERSION: 3.0 where the 3.0 RFC 2426 does not mention allowing double quoted lists, even in its examples, making this clearly an invalid VCARD. An example of the VCARD created by TB for CardDAV:

BEGIN:VCARD
VERSION:3.0
UID:1acb01ed-6979-46c5-ac6b-0f855a5a2acd
FN:Test Testy
TEL;TYPE="HOME,VOICE":3215541236
N:Testy;Test;;;
END:VCARD

Due to the ambiguity of enclosing comma separated lists in double quotes as described in errata 3488 most implementations of CardDAV do not seem to support this. Instead they generate and accept this (with VERSION set to either 3.0 or 4.0):

BEGIN:VCARD
VERSION:3.0
UID:1acb01ed-6979-46c5-ac6b-0f855a5a2acd
FN:Test Testy
TEL;TYPE=HOME,VOICE:3215541236
N:Testy;Test;;;
END:VCARD

Comment 3

[Grzesiek]

Hi, I am using sabradav as a server and vcard version is 3.0 but I've not notice " in TEL field like you did.
Also when you export (drag'n'drop) contact ther is no " , but then verion is 4.0
For me only issue is that the TYPE is missing when there is only one phone number for the contact
TEL;VALUE=TEXT:123456789
when I set two phone numbers it looks like below
TEL;TYPE=home;VALUE=TEXT:123456789
TEL;TYPE=cell;VALUE=TEXT:987654321

Comment 4

[Ralph Day]

Attachment: Invalid Vcard

Odd it doesn't fail for you. Here's a screen shot of it failing. I'm using Horde on a Cpanel server which uses sabreDAV. Export works correctly, just server sync failing. I have TBSync installed, perhaps it changes the behavior somehow? I'll test without TBSync on a clean server account to see if there is some interaction.

Comment 5

[Ralph Day]

I have tested in Troubleshoot mode so that the TBSync addon is disabled and I still see the same behavior. Here's what I am seeing.

Add new contact with a home phone number and the TEL in the PUT VCARD 3.0 is:
TEL;VALUE=TEXT:3215541234

This is wrong in 2 ways: VALUE=TEXT is not valid in VCARD 3.0 and it does not specify the TYPE value as HOME.

The phone number is rejected on my server for some reason - perhaps it doesn't support the default TYPE of VOICE. Not a TB problem but it ends up leaving the contact with no phone number as the REPORT call to the server doesn't return the phone number.

Then I add a home phone number to the contact. This time it correctly creates the VCARD 3.0 with a TYPE parameter. Why the inconsistency with initial creation I don't understand.
TEL;TYPE=home;VALUE=TEXT:3215541234

This time the server accepts the phone number and stores it. The REPORT returns for the TEL:
TEL;TYPE=HOME,VOICE:3215541234

So this is how the the multiple value TEL TYPE gets introduced. The contact now correctly shows the phone number as a home phone. Odd that the server rejected a phone number with no TEL TYPE which should default to VOICE yet returns a TEL TYPE of VOICE but again not a TB problem. But it is a valid VCARD and TEL TYPE and TB correctly accepts it.

Now when updating the home phone number in the contact TB will generate the TEL TYPE enclosed in quotes. This appears to be because
VCardProperties.toVCard calls ICAL.stringify which eventually calls ICAL.stringify.propertyValue which will wrap any value containing a comma, colon or semicolon in double quotes.

So three bugs need fixing:

1. VALUE=TEXT should not be generated for VCARD 3.0
2. TEL TYPE shouldn't be omitted when creating a new contact and the correct type (home, cell, etc) should always be specified
3. TEL TYPE with multiple values should not be wrapped in double quotes

Comment 6

[Grzesiek]

In the new TB 102.2.1 (64 bity) cards are created correctly. VCARD version is 4.0

Data from sabredav DB
Contact created in TB with one phone number in mobile field:

BEGIN:VCARD
VERSION:4.0
N:;TEST2;;;
FN:TEST2
TEL;TYPE=cell;VALUE=TEXT:123456789
UID:d4a917da-3a9f-4014-aaa5-381dbf1bad2d
END:VCARD

When you select none as a type card looks like:
BEGIN:VCARD
VERSION:4.0
N:;TEST3;;;
FN:TEST3
TEL;VALUE=TEXT:234567890
UID:2dc442c8-8454-49f0-a2d1-40f495a35153
END:VCARD

Comment 7

[Ralph Day]

Confirmed that creating a new contact in 102.2.1 64-bit Windows works properly. It creates VCARD 4.0, VALUE=TEXT is correct and it supplies the correct TEL TYPE. The quoting of the TEL TYPE when the server returns a comma separated list of types still needs to be fixed.

Comment 8

[Ralph Day]

Looking at the code I believe the problem with wrapping the TEL TYPE parameter in double quotes is in calendar/base/modules/Ical.jsm. stringify.property at line 1581:

      if (params.hasOwnProperty(paramName)) {
        var multiValue = (paramName in designSet.param) && designSet.param[paramName].multiValue;
        if (multiValue && Array.isArray(value)) {
          if (designSet.param[paramName].multiValueSeparateDQuote) {
            multiValue = '"' + multiValue + '"';
          }
          value = value.map(stringify._rfc6868Unescape);
          value = stringify.multiValue(value, multiValue, "unknown", null, designSet);
        } else {
          value = stringify._rfc6868Unescape(value);
        }


        line += ';' + paramName.toUpperCase();
        line += '=' + stringify.propertyValue(value);
      }

The problem is stringify.propertyValue has its sole function to enclose the value in double quotes if the value contains a comma, colon or semicolon. Since the stringify.multivalue just created the value as a comma separated list of values it therefore will get wrapped in double quotes.

I believe the proper change to the code would be changing the above stringify.property code to:

      if (params.hasOwnProperty(paramName)) {
        var multiValue = (paramName in designSet.param) && designSet.param[paramName].multiValue;
        if (multiValue && Array.isArray(value)) {
          if (designSet.param[paramName].multiValueSeparateDQuote) {
            multiValue = '"' + multiValue + '"';
          }
          value = value.map(stringify._rfc6868Unescape);
*         value = value.map(stringify.propertyValue);
          value = stringify.multiValue(value, multiValue, "unknown", null, designSet);
        } else {
          value = stringify._rfc6868Unescape(value);
*         value = stringify.propertyValue(value);
        }


        line += ';' + paramName.toUpperCase();
*       line += '=' + value;
      }

This will enclose individual values in a list or a single value in double quotes if containing comma, colon or semicolon. Looking at the design sets in the code it appears that the TYPE parameter for a VCARD is the only multivalue parameter so this change should only affect the VCARD TYPE parameter. There are several iCalendar property values (not parameters) that are multivalues but that is handled by code later in stringify.property.