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

Contact groups/categories are stored wrongly #433

Closed
nickvergessen opened this issue Jun 9, 2016 · 10 comments

Comments

Projects
None yet
7 participants
@nickvergessen
Copy link
Contributor

commented Jun 9, 2016

Steps to reproduce

  1. Create a contact
  2. Assign multiple groups
  3. Check the VCard that is sent to the server

Expected behaviour

Expected: CATEGORIES:1,2

Actual behaviour

Actual: CATEGORIES:1\,2

So the , is escaped causing the value to be interpreted as one group called one-slash-comma-two instead of two groups one and two

@nickvergessen nickvergessen added the bug label Jun 9, 2016

@nickvergessen

This comment has been minimized.

Copy link
Contributor Author

commented Jun 9, 2016

@DeepDiver1975

This comment has been minimized.

Copy link
Member

commented Jun 9, 2016

@carlomorgenstern

This comment has been minimized.

Copy link

commented Jun 9, 2016

same thing happens with the "TYPE" property, which makes basically every vcard exported from owncloud invalid for me.

Steps to reproduce

  1. Create a contact
  2. Insert a parameter with the "TYPE" property (e.g. a work phone number)
  3. Check the VCard

Expected: TEL;TYPE=WORK,VOICE:xxxxxx
Actual: TEL;TYPE=WORK\,VOICE:xxxxxx

@j-ed

This comment has been minimized.

Copy link

commented Sep 5, 2016

ownCloud version: 9.0.4
Contacts app version: 1.4.0.0

> Expected: TEL;TYPE=WORK,VOICE:xxxxxx
> Actual: TEL;TYPE=WORK\,VOICE:xxxxxx

I can confirm that this issue causes problems when synchronize phone number types from ownCloud to Android using e.g. the "ContactSync - CardDAV und mehr" app. I had a discussion with the app developer and she confirmed that the escaped comma is causing the problem and that this need to be fixed in ownCloud.

@irgendwie

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2016

jfr. This issue is caused by our vcard parser:
https://github.com/Heymdall/vcard

@j-ed

This comment has been minimized.

Copy link

commented Sep 6, 2016

@irgendwie If the root cause has been identified half way of a fix has already been done ;-)
Let me know if anything should be tested, I'm usually synchronizing data to an Android phone and iPhone.
(BTW: wouldn't it be possible to provide pre-compiled test releases. This would make testing of individual fixes much easier)

@xenji

This comment has been minimized.

Copy link

commented Nov 5, 2016

@derqurps

This comment has been minimized.

Copy link

commented Nov 20, 2016

@xenji is the pull request only intended for the TYPE field or also for the groups/categories fields? If not, should i do another one for these fields?

@xenji

This comment has been minimized.

Copy link

commented Nov 21, 2016

@derqurps Due to https://github.com/Heymdall/vcard/pull/6/files?diff=unified#diff-d44c6e34337aec12784cda75e6a8a40aR245 it only targets the TYPE field. Other field should be easy to add in this if / else construct. Although I think there are better ways to solve this from the architectural perspective (delegation / strategy), than the way I did.

@DeepDiver1975 DeepDiver1975 modified the milestones: 1.5, 1.6 Nov 29, 2016

@xenji

This comment has been minimized.

Copy link

commented Dec 1, 2016

FYI: My patch in the vcard lib has been merged upstream.

@DeepDiver1975 DeepDiver1975 modified the milestones: 1.5, 1.6 Dec 1, 2016

DeepDiver1975 added a commit that referenced this issue Dec 1, 2016

DeepDiver1975 added a commit that referenced this issue Dec 2, 2016

DeepDiver1975 added a commit that referenced this issue Dec 5, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.