368607 - attendees get doublequoted

Status: VERIFIED FIXED

(Calendar :: Import and Export, defect)

Description

[Bas van den Bosch]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; nl; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1
Build Identifier: 

When importing an event which has attendees CN with quoted strings (for instance from outlook if names contain a comma such as "Bosch,Bas") Sunbird adds extra quotes. Importing an ics with the produced double-quotes to a local calendar, ftp or webdav or editing the event containing the double-quotes deletes the CN completely (which is when rewriting the event). With communigate, server reports an error 500 (confirmed by developers), there's significant dataloss.

Reproducible: Always

Steps to Reproduce:
1. Import event holding attendees with quoted CN's
2. Export event to ics and open in editor
3. note double double-quotes
4. edit event and reload calendar
5. Export event to ics and open in editor
6. note missing CN
Actual Results:  
see above

Expected Results:  
quoted string for CN stays quoted string.

This doesn't happen to events produced by sunbird as these don't contain names for now, just email-adresses. 
Behaviour experienced in:
* Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a2pre) Gecko/20070115 Calendar/0.6a1
* TB Versie 1.5.0.9 (20061207) with lightning 0.4a1 (2007011505)

In most cases this is not a big problem as the emailadress is still shown in sunbird when the cn is missing but in communigate there's significant dataloss. set to Major for the dataloss in communigate.

Comment 1

[Bas van den Bosch]

Attachment: event with quoted CN

A sample event with quoted string...

Comment 2

[cmtalbert]

Confirmed on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2pre) Gecko/20070222 Calendar/0.4a1.  

Because this reduces interoperability and is dataloss, I'm going to ask for it to block for 0.5.

Comment 3

[Matthew (lilmatt) Willis]

Since 0.5 will actually do stuff with the attendees, this needs to block.

Comment 4

[Michiel van Leeuwen (email: mvl+moz@)]

This is mostly the same problem as bug 307948, but for property parameters instead of values. So it likely needs a very similar fix.

Comment 5

[Bas van den Bosch]

It's similar insofar as it's about the same parts of the code. However, the behaviour seems different to me (but don't know the codebase well). If there's a comma in the name (not email) of the attendee (in the attatchment it's: "Bas, Bosch") the CN gets double quotes ""Bosch, Bas"". If there's no comma like "Bas Bosch", the CN becomes Bas Bosch (without quotes). If you try to escape the comma with \, in the ICS which you import, the CN gets doublequotes anyway (testing for similarity with bug 307948). 

Comment 6

[Michiel van Leeuwen (email: mvl+moz@)]

No, it's the same as it being the same problem in the code. we use the escaped-for-ics string, instead of the normal string value of the properties.

Comment 7

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: patch v1

I had this patch lying around. I don't really like it, but attaching anyway, to prevent it from getting lost. The idea is that is make the value string of a property not quoted. I guess that it will die if you try it on a non-string values property. I'm not even sure what it will do to current callers. But it does seem to fix the bug...
(not requesting review yet, because i want to study on better solutions)

Comment 8

[Michael Büttner (no reviews TFN)]

I'm going to further investigate this issue...

Comment 9

[Michael Büttner (no reviews TFN)]

The problem happens indeed in the 'getParameter'-function() of calIIcalProperty. This can be verified by breaking at [1], the icalProperty-setter for attendee objects. the icalatt parameter is a calIcalProperty of type attendee, icalatt.icalString reveals the icalstring before its content is put into the atendee object:

ATTENDEE;RSVP=true;CN=\"Bosch, Bas\";PARTSTAT=NEEDS-ACTION;\r\n ROLE=REQ-PARTICIPANT:MAILTO:[e-mail]\r\n

Everything is still fine, but

icalatt.getParameter("CN") returns:

"\"Bosch, Bas\""

This is just to verify what mvl has already found out, including some context what exactly the problem is.

[1] http://lxr.mozilla.org/seamonkey/source/calendar/base/src/calAttendee.js#128

Comment 10

[Michael Büttner (no reviews TFN)]

Obviously, things went wrong inside calIcalProperty::GetParameter(). After some time fiddling around with where precisely we're doing something wrong, I was asking myself where the first pair of quotes have been introduced. This actually happens in icalproperty_get_parameter_as_string(), which in turn asks icalparameter_as_ical_string for a string that looks like 'param-name "=" param-value'. In this case it is perfectly valid to add the quotes to 'param-value' if necessary. icalproperty_get_parameter_as_string() just takes the result, searches for the '=' and returns the rest of the resulting string. That's where the quotes are initially introduced. Excuse me, isn't this completely wrong? The quotes are fine in case we're asking for the 'name = value' string, but not in case we're just asking for the value string itself. Thus I think it's better to strip off the quotes in the latter case.

Comment 11

[Michael Büttner (no reviews TFN)]

Attachment: patch v2

patch per previous comment.

Comment 12

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 259882 [details] [diff] [review]
patch v2

r=mvl

Comment 13

[Matthew (lilmatt) Willis]

Patch checked in on MOZILLA_1_8_BRANCH and trunk.

-> FIXED

Comment 14

[Bas van den Bosch]

Verified in TB 1.5.0.10 with lightning 2007040206, name gets preserved in the attendee though it's not shown in the event (bug 376284 filed).

Thanks all...