Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support X-ABLabel and ITEM2.xxx properties #42

Closed
brjhaverkamp opened this issue Nov 28, 2016 · 46 comments
Closed

Support X-ABLabel and ITEM2.xxx properties #42

brjhaverkamp opened this issue Nov 28, 2016 · 46 comments
Labels
3. to review Waiting for reviews enhancement New feature or request
Milestone

Comments

@brjhaverkamp
Copy link

brjhaverkamp commented Nov 28, 2016

I copied this issue from the owncloud/contacts github issue list where it was issued by rfc2822.
I have a keen interest in this feature as well, as it would simplify creating address sticker for my mailings.
I even created the php code to create the address label already, but lack the skills to integrate them in the contacts app. but it would be very simple with the right skills to hook it up on import or address change! If one of the devs would like to do this or mentor me, that would be great.

Steps to reproduce

Create a contact with a home address. For instance, create a user "Alpha Beta" and use PO Box "1", street address "Street 2", postal code "333", city "Virtual", country "Schland" as address.
Download the VCard using CardDAV:

GET …/owncloud/remote.php/dav/addressbooks/users/test/default/bc2620f7-2ed3-4f5f-9404-b2bafd12ae16.vcf http/1.1 Accept: text/vcard;version=4.0, text/vcard;charset=utf-8;q=0.8, text/vcard;q=0.5

It will look like this:

BEGIN:VCARD
VERSION:4.0
PRODID:-//Sabre//Sabre VObject 3.5.0//EN
FN:Alpha Beta
UID:bc2620f7-2ed3-4f5f-9404-b2bafd12ae16
ADR;TYPE=HOME:1;;Street 2;Virtual;;333;Schland
END:VCARD

Note there's no LABEL in the ADR property, so that clients will generated the formatted address themselves.

When a client modifies the VCard (for instance, "street 2" is changed to "street 2a") and adds a LABEL for its representation, it will upload something like this:

PUT …
Content-Type: text/vcard;version=4.0

BEGIN:VCARD
VERSION:4.0
PRODID:+//IDN bitfire.at//DAVdroid/1.2.3-gplay vcard4android ez-vcard/0.9.1
1
UID:bc2620f7-2ed3-4f5f-9404-b2bafd12ae16
FN:Alpha Beta
N:Beta;Alpha;;;
ADR;LABEL=Street 2a^n1^nVirtual 333^nSchland;TYPE=home:1;;Street 2a;Virtual
;;333;Schland
REV:20160901T213222Z
END:VCARD

Here, the LABEL has been set (and RFC 6868 has been used for the line breaks).

Now, when you look in the OwnCloud Contacts again, everything is correct (street is "Street 2a").
However, if you then change "Street 2a" to "Street 3a", OwnCloud Contacts will update the formatted address, but not the LABEL:

BEGIN:VCARD
VERSION:4.0
PRODID:-//Sabre//Sabre VObject 3.5.0//EN
UID:bc2620f7-2ed3-4f5f-9404-b2bafd12ae16
FN:Alpha Beta
N:Beta;Alpha;;;
ADR;LABEL="Street 2a^n1^nVirtual 333^nSchland";TYPE=home:1;;Street 3a;Virtu
al;;333;Schland
REV:20160901T213222Z
END:VCARD

If you then synchronize this VCard to another client, it will be confused and show the LABEL address with the (incorrect) "Street 2a" as formatted address, but when editing the components, it will (correctly) show "Street 3a".

Expected behaviour

The LABEL parameter of the ADR property should be handled correctly. For instance, it could be dismissed when a change is done in the Web UI. However, an incorrect LABEL should never be sent.

Actual behaviour

LABEL is stored when received, and then sent without modification, even when the content of the address field has been modified in the OwnCloud Contacts app.

Required change.
the contacts app should be able to update the LABEL field when the address changes or fill it on importing an contact vcard with an empty LABEL field.

Since different countries have different address formats (some have postal code before the city, some city before the postal code, etc.) the contacts app has to be aware of the county that the address is refering to or if the country field empty default to a predefined default country. This default county can not be derived from the language; many people use a different language than the language of the country they live in. So it should be seperate setting, either in the contact app or overall in nextcloud.

Another potential challenge is that the country name in the vcard can be spelled in many differeny ways.
Detecting the destination for hte address formatting can be tricky because of this. Having the county be filled from a drop down box based on international standard country-codes might be the right solution (with a free format option for the exceptions, of course)

The address format can then be a simple lookup table from the country-code to the address format in that county.

@irgendwie irgendwie added 1. to develop Accepted and waiting to be taken care of bug Something isn't working labels Nov 29, 2016
@brjhaverkamp
Copy link
Author

Could one of the developers please point me in the right direction here? I would like to develop the fix. Where in the code can I add a functioncall for my function that will update the LABEL field, upon import or change of the address?

@irgendwie
Copy link
Member

irgendwie commented Dec 7, 2016

Since the vcard lib was updated, does the issue still occur with current master branch?

@brjhaverkamp
Copy link
Author

brjhaverkamp commented Dec 7, 2016 via email

@tbleher
Copy link

tbleher commented Feb 5, 2017

Would it be possible (as a stop-gap measure) to just reset the LABEL when the entry is modified? I think it would be great if that change went into the next stable update.

To explain, let me tell how I was recently bitten by this bug:
I few months ago, a friend of mine, who lives in another city, moved inside the city. At that time, I updated his address in the Nextcloud web frontend. Last weekend, we visited him. Since my address book is synced from Nextcloud to my Android smartphone, I took the address from there. When we arrived at the listed address, we could not find his name at the front door. Sure enough, Android still listed the old address, which meant another 20 minute drive through the city (luckily not more).
Investigating further, I found that Android showed the new address in edit mode, but in the normal display mode it uses the LABEL field. There was no indication that the data was inconsistent. I verified that Nextcloud stored the new address, but with the old label. Nextcloud only showed the new address (since it doesn't use the LABEL field). I found no way to delete or modify the LABEL inside Nextcloud.
Editing the address in Android deleted the LABEL field, so now the address is displayed correctly.

Note: the above issue happened with NextCloud 10, I haven't yet verified if the issue is the same with NextCloud 11.

@brjhaverkamp
Copy link
Author

Hello tbleher,
I assume this will be the same with NC11, as the core of the problem is in the contacts app. It is good to see that there is an even more pressing usecase that is touched by this bug. Maybe this will give an added incentive to solve it. (Preferably with more than just a stopgap measure, but really locallised to the country style)

@skjnldsv
Copy link
Member

skjnldsv commented Oct 1, 2018

@brjhaverkamp Hey! This is an old issue! Can you check if this is still happening on the 3.0.0 alpha? :)
Thanks a lot!

@skjnldsv skjnldsv changed the title LABEL doesn't update when ADR is modified in Web UI or contact is imported Support X-ABLabel and ITEM2.xxx properties Oct 2, 2018
@skjnldsv skjnldsv added enhancement New feature or request and removed bug Something isn't working labels Oct 2, 2018
@skjnldsv skjnldsv added this to the 3.2.0 milestone Oct 2, 2018
@brjhaverkamp
Copy link
Author

Hi, thanks for taking time to look at this.I can look on the latest release. Or has anything relevant been included in 3.0.0-alpha?

@skjnldsv
Copy link
Member

skjnldsv commented Oct 2, 2018

Lots of things, but not relevant to this ;)
This will come one day, that's for sure, but not on 3.0.0 :)

@brjhaverkamp
Copy link
Author

I can at at least confirm that the LABEL is not kept in sync with the ADDRESS properties are not kept in sync in Contacts 2.1.5 so for me it is still an issue.

@ghost
Copy link

ghost commented Mar 3, 2019

Is there a solution to the issue?
The addresses are also not displayed in the WebGui and listed in the vCard with "ITEM1.ADR".
Contacts: 3.0.3
Nextcloud: 15.0.5
iOS: 12.1.4

Paul

@skjnldsv
Copy link
Member

skjnldsv commented Mar 3, 2019

@Pablo1978 not yet !

@skjnldsv
Copy link
Member

Hello everyone!
A pull request have been pushed! Please help review to make sure no bugs were introduced and that everything is working properly :)

#991
@brjhaverkamp @tbleher @Pablo1978

@skjnldsv skjnldsv added 3. to review Waiting for reviews and removed 1. to develop Accepted and waiting to be taken care of labels Mar 12, 2019
@j-ed
Copy link

j-ed commented Mar 28, 2019

At what time is the function triggered? Does it only work on a vcard import or on a sync activity? I've exported the whole address book to a vcf file and searched for "ITEM" entries, which I then selected using the GUI. I would have expected to get the mentioned address entry displayed too.

@skjnldsv
Copy link
Member

@j-ed it happens for every card.
This is a full support of the itemXX.XXX properties, not just a repair step or a migration :)

@j-ed
Copy link

j-ed commented Mar 28, 2019

Hmm, I'm a little bit confused because I couldn't see the described behavior.
What am I doing wrong?
Are you sure that the provided app archive really contains the fixes?
Here are two additional records which contain ITEM entries and didn't work as expected:

BEGIN:VCARD
VERSION:3.0
X-CONTACTSYNC-STARRED:FALSE
FN:Herr Dr. Jo Good
EMAIL;TYPE=work,pref:jo.good@somewhere.com
CATEGORIES:Testing
ORG:Somewhere Corp.
ITEM1.ADR;TYPE=work,pref:;;Street 1\n;City;;12345;Deutschla
 nd
ITEM1.X-ABADR:de
PRODID:-//NTBAB//Android//ContactSync//9.0//166
REV:20180713T093422Z
TITLE:Head of Desaster
TEL;TYPE=work,pref:+49 1234 556677
TEL;TYPE=work,cell:+49 333 778899
TEL;TYPE=fax,work:+49 1234 667788
N:Good;Jo;;Herr Dr.;
END:VCARD
BEGIN:VCARD
VERSION:3.0
PRODID:-//Apple Inc.//iOS 10.2//EN
N:Restaurant XX;;;;
FN:Restaurant XX
ORG:Restaurant XX;
EMAIL;TYPE=WORK,WORK,pref:xyz@hotmail.de
TEL;TYPE=WORK,VOICE:+49 1234 556677
TEL;TYPE=WORK,FAX:+49 1234 667788
ITEM1.X-ABLABEL:WORK\,VOICE
ITEM2.X-ABLABEL:WORK\,FAX
ADR;TYPE=WORK:;;Street 81;City;;12345;Deutschland
NOTE:Öffnungszeiten:\ntäglich von 16.00 bis 24.00 Uhr\nsowie sonn- und fe
 iertags von \n12.00 bis 24.00 Uhr\nDie Küche ist bis 23.00 Uhr geöffnet.
ITEM4.URL;TYPE=pref;VALUE=URI:http://www.restaurantxx.de
REV:20181227T184748Z
CATEGORIES:Testing
END:VCARD

@skjnldsv
Copy link
Member

What am I doing wrong?
Are you sure that the provided app archive really contains the fixes?

I'm starting to have doubts! 😁
I build the app again and updated the first post on #991
Please try again 😉

@j-ed
Copy link

j-ed commented Mar 28, 2019

You did it! 😄 The mentioned records are now correctly displayed.

@skjnldsv
Copy link
Member

skjnldsv commented Mar 28, 2019

@j-ed 🙈 Thanks for your patience 😁

Other than properly being display, could you wander around this a bit? Go back to an existing LABEL, change the custom label... etc :)
To make sure we don't miss a regression 😉

@j-ed
Copy link

j-ed commented Mar 29, 2019

@skjnldsv Unfortunately I cannot run tests on an iPhone at the moment, only on an Android phone. But, as usual, I will play around with the app.
What does custom label support mean in detail? Do you now allow to enter custom labels in the Contacts app or will custom labels only be supported when they have been entered on a device like an iPhone?

I hope @brjhaverkamp will be able to run tests on an iPhone to close the testing gap.

@skjnldsv
Copy link
Member

What does custom label support mean in detail? Do you now allow to enter custom labels in the Contacts app or will custom labels only be supported when they have been entered on a device like an iPhone?

It means you can now enter custom stuff with ABLABEL (like android+davdroid supports or iOS deals wit those. This is a full support, so if you change it on nc or in android, the data will be displayed and compatible with both.
See #27 for more details about that.
Example, email with label iCloud:

BEGIN:VCARD
VERSION:3.0
N:Doe;John;;;
FN:John Doe
item1.EMAIL;type=INTERNET:test3@cozy.io
item1.X-ABLabel:iCloud
END:VCARD

@j-ed
Copy link

j-ed commented Mar 29, 2019

@skjnldsv I've created a new record on my Android phone and the result looks pretty good, except for instant messaging addresses.

A custom phone number label: 👍
grafik
A custom email label: 👍
grafik
A custom address label: 👍
grafik

A custom Instant Messaging label: is not correctly displayed. Instead the record itself is prefixed with the label and in front of the field you find the request to select a type:** 👎
grafik

@skjnldsv
Copy link
Member

@j-ed thanks!! 😁
I'll try to check this! Care to attach the vcard?

@j-ed
Copy link

j-ed commented Mar 29, 2019

@skjnldsv Oh sorry, my fault. Here it is:

IMPP:MeinChat:a.t@chat.home

I looks like Android is handling labels for instant messaging entries differently. I found this issue which already covers it: #800

BTW, I think I've found some more issues which are related to this issue ticket, e.g.

  • wrong phone label
  • wrong url label
  • empty email label
  • dates not being displayed

Should I open separate issue tickets for it so that it can better be tracked or do you want me to add it to this conversation?

@skjnldsv
Copy link
Member

  • wrong phone label
  • wrong url label
  • empty email label
  • dates not being displayed

Please do open an issue yes! :)
If you think those are related to the same problem.

@j-ed
Copy link

j-ed commented Mar 29, 2019

@skjnldsv [x] done. I hope you agree that the issues are related 😉

@skjnldsv
Copy link
Member

@j-ed thanks!! I'll give them a review & analysis each!

@brjhaverkamp
Copy link
Author

Hi Guys, I'm onboard. I downloaded the contacts.tar.gz and unpacked it in the nextcloud/apps (after moving the original contacts dir to contacts-orig. Is that sufficient? Sofar I don't see anything to labels..

@skjnldsv
Copy link
Member

@brjhaverkamp it should be for the nextcloud part.
But you need to make sure you r browser cache is cleared too :)

@brjhaverkamp
Copy link
Author

Hi John, I wasn't sure if the contacts.tar.gz was the one including the label patches, so I downloaded the latest from github and compiled it. I got the following error at the end of the make, but since it is in testing I asumed it is not critical.
phpunit -c phpunit.xml
PHP Warning: require_once(/home/bert/Downloads/contacts-master/tests/../../../tests/bootstrap.php): failed to open stream: No such file or directory in /home/bert/Downloads/contacts-master/tests/bootstrap.php on line 12
PHP Fatal error: require_once(): Failed opening required '/home/bert/Downloads/contacts-master/tests/../../../tests/bootstrap.php' (include_path='.:/usr/share/php') in /home/bert/Downloads/contacts-master/tests/bootstrap.php on line 12

I uploaded the result to my nextcloud server, and changed a few letters in appinfo.xml. So I am now 100% sure I am using the latest contacts. But I think I am looking in the wrong place. Where can I add a custom label? or edit the label?

@brjhaverkamp
Copy link
Author

Oh, and I'm afraid I can't help with the Iphone testing. I am on the android side as well.

@j-ed
Copy link

j-ed commented Mar 31, 2019

@brjhaverkamp As far as I understood, the contacts app now supports the described fields which are created on other devices, but didn't allow to create it in the GUI yet. I created e.g. a new contact on my Android device, selected the field type "custom" and entered a custom description. After syncing it to Nextcloud, the custom field types are displayed in the GUI.

@skjnldsv
Copy link
Member

skjnldsv commented Mar 31, 2019

@brjhaverkamp you can also create custom labels. For a phone number, for example, you can click the select right next to the left of the phone input, and enter your own label (e.g. 'UK Office' or whatever).

For the phpunit error, you can ignore :)

@j-ed
Copy link

j-ed commented Mar 31, 2019

@skjnldsv Wow, It didn't catch that. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants