GNOME Bugzilla – Bug 427469
Tasks: I imported a vcard file ...
Last modified: 2007-08-06 14:53:26 UTC
I imported a vcard file that looks like this into Evolution: BEGIN:VCARD N:Lastname;Firstname;;; ADR;DOM;HOME:;;Street Address;City;State;ZIP; TEL;HOME:+1-XXX-XXX-XXXX EMAIL:XXX@XXX.XXX END:VCARD It's a completely valid vcard. Evolution got it wrong in two different ways. First of all, next to the email address in the preview display it says "((null))". If the type of the email address wasn't specified, it should default to Other, probably. Second, it didn't import the address at all. Distribution: Fedora release 6.92 (Rawhide) Gnome Release: 2.18.0 2007-03-23 (Red Hat, Inc) BugBuddy Version: 2.18.0
valid? why should that one be valid?! ADR;DOM;HOME:;; also, where is the vcard VERSION statement?
I do not understand what your problem is with the ADR line, and the vCard specification does not require a VERSION statement. Please be more specific about why you think this vCard is invalid. I realize that MS is not exactly known for standards compliance, but it is worth noting that both Outlook and Windows Mobile accept vCards in this format correctly and without complaint.
http://tools.ietf.org/html/rfc2426 , sections 3.6.9 ("The VERSION property MUST be present in the vCard object") and 3.2.1 (the TYPE property is missing for ADR and TEL). closing as NOTABUG.
Sorry, I was wrong about the VERSION property being optional, but the same specification you cited to prove that it is required also indicates that the TYPE property is optional and specifies what the default should be if it's missing, so if you're not defaulting TYPE properly for ADR and TEL, then you've still got a bug.
yeah, TYPE is an optional, sure, but that's not what we're talking about here: The TYPE parameter values can include "dom" to indicate a domestic delivery address; "intl" to indicate an international delivery address; "postal" to indicate a postal delivery address; "parcel" to indicate a parcel delivery address; "home" to indicate a delivery address for a residence; "work" to indicate delivery address for a place of work; and "pref" to indicate the preferred delivery address when more than one address is specified. These type parameter values can be specified as a parameter list (i.e., "TYPE=dom;TYPE=postal") or as a value list (i.e., "TYPE=dom,postal"). This type is based on semantics of the X.520 geographical and postal addressing attributes. The default is "TYPE=intl,postal,parcel,work". The default can be overridden to some other set of values by specifying one or more alternate values. For example, the default can be reset to "TYPE=dom,postal,work,home" to specify a domestic delivery address for postal delivery to a residence that is also used for work. so what should "ADR;DOM;HOME:;;" be?!
TYPE= is optional in VCARD 2.1. Please see the example in section 2.3.1 of the vCard 2.1 specification. See also section 2.1.2 of the same specification, which says, "The value string can be specified alone in those cases where the value is unambiguous." Is it your intent to reject VCARD 2.1 inputs?
TYPE= is also optional in 3.0, that's not the issue. again: what should "ADR;DOM;HOME:;;" be?! can you answer that, please? :-) nobody wants to reject any inputs if they are valid files, but evo doesn't have a crystalball implemented for any broken apps producing broken outputs...
oops, that "ADR;DOM;HOME:;;" part was already answered, sorry... can i get a link to the 2.1 spec? thanks in advance.
Please see http://www.imc.org/pdi/pdiproddev.html. There are links there to the spec in a number of different formats. "ADR;DOM;HOME:" is equivalent to "ADR;TYPE=DOM;TYPE=HOME:" or "ADR;TYPE=DOM,HOME:". And incidentally, as far as I can tell from the RFC, "TYPE=" is *not* optional in 3.0. It says in section 5, 'The `TYPE=' prefix to parameter values is required. In[VCARD] this was optional."
the question is if there's anything in that 2.1 definition that says that version is optional...
(In reply to comment #10) > the question is if there's anything in that 2.1 definition that says that > version is optional... > in spec 2.1 file from link by Jonathan is written "This property must appear within the vCard data stream." There has been made some progress (I think as side effect of other bug), because with that email there is not shown "((null))" now, but "()" instead. In 2.1 the type in EMAIL is optional, like in TEL and ADR. So it maybe changed to "OTHER" like in ADR, I think. When I change original vcard from comment #1 on this one: BEGIN:VCARD VERSION:2.1 N:Lastname;Firstname;;; ADR;TYPE=DOM;TYPE=HOME:;;Street Address;City;State;ZIP; TEL;HOME:+1-XXX-XXX-XXXX EMAIL:XXX@XXX.XXX END:VCARD then it ignore ADR as well, so I think the problem isn't about missing VERSION property. I'm not sure why, but when I remove that TYPE=DOM from ADR then it is also shown in Evolution, otherwise it's imported but one could not see it in UI.
(I'm sorry, I mean comment #0 in the above comment.) patches from bug #313221 solve part of this bug (ADR is shown with them, regardless of TYPE values ordering.)
Created attachment 87723 [details] [review] e-d-s patch for evolution-data-server; You need to apply patch from comment #12 too for right behavior. When you use original vCard from statement, then while parsing input, TYPE params are added twice for ADR, except of only one param TYPE with two values. I changed e-vcard.c: e_vcard_attribute_add_param in a way it tests for duplicities and merge values if needed. I think it's better do here than in read_attribute_params.
Created attachment 87724 [details] [review] evolution patch for evolution. with this patch the TYPE=OTHER is added for EMAILs without TYPE=HOME and TYPE=WORK param. With all that patches (two here and two at bug #313221) together the whole bug could be fixed.
Comment on attachment 87724 [details] [review] evolution patch Looks good to me.
Comment on attachment 87723 [details] [review] e-d-s patch This needs to be as fast as possible as it will be called *a lot*. Instead of using the accessors, directly access the fields, i.e. param->name rather than e_vcard_attribute_param_get_name(), attr->params rather than e_vcard_attribute_get_params(). Also "if (foo) {", you often miss the space between ) and { or add a space before foo. I'm not entirely convinced that EVCard should be handling this though, I'm leaning towards saying that applications should iterate over every TYPE parameter instead of only the first.
Created attachment 89313 [details] [review] proposed e-d-s patch ][ for evolution-data-server; I changed it as Ross suggested. With that handling, I hope it's better to handle it here than have same code on each place where you need to add parameter to attribute.
Can we get this for 2.11.4?
Ross: Can you review this for 2.11.5?
The EDS patch looks good to me.
Commit this too.
e-d-s part committed to trunk. Committed revision 7925. evo part committed to trunk. Committed revision 33959.