In bug 738078 we noticed that attachments in storage calendars don't have X-Props/IANA-Props preserved. Moving towards bug 498968 I have made these properties store their icalString instead of the single properties in DB fields.

Requires the patch for bug 785659 applied.

Comment on attachment 655440 [details] [diff] [review] Fix - v1 Review of attachment 655440 [details] [diff] [review]: ----------------------------------------------------------------- ::: calendar/test/unit/xpcshell.ini @@ +21,4 @@ > [test_freebusy.js] > [test_hashedarray.js] > [test_ics.js] > +[test_ics_service.js] Looks like this file is missing from the patch

Comment on attachment 655440 [details] [diff] [review] Fix - v1 Review of attachment 655440 [details] [diff] [review]: ----------------------------------------------------------------- Aside from the missing test file it seems to work well. ::: calendar/base/src/calRecurrenceRule.cpp @@ +565,5 @@ > + > + rv = prop->GetPropertyName(name); > + NS_ENSURE_SUCCESS(rv, rv); > + > + if (!name.EqualsLiteral("RRULE") && !name.EqualsLiteral("EXRULE")) { If EXRULE is deprecated (and not fully supported anyway in http://mxr.mozilla.org/comm-central/source/calendar/base/src/calRecurrenceInfo.js#282 ) should we allow it here? ::: calendar/providers/storage/calStorageUpgrade.jsm @@ +1497,5 @@ > + > + let attendee = cal.createAttendee(); > + > + attendee.id = aAttendeeId; > + attendee.commonName = aCommonName; The conversion throws errors with the corrupt organizer issue (see Bug 752163, the id and commonName properties are null) and results in a null icalString in the cal_attendees table. This seems to be no worse than it was before, but maybe we can use this opportunity to remove or correct the corrupt entries?

(In reply to Matthew Mecca [:mmecca] from comment #4) > Aside from the missing test file it seems to work well. These damn test files! Added in v2. > If EXRULE is deprecated (and not fully supported anyway in > http://mxr.mozilla.org/comm-central/source/calendar/base/src/ > calRecurrenceInfo.js#282 ) should we allow it here? I've taken it out, here and in the other place where EXRULE is specified. I don't think it will hurt. > > The conversion throws errors with the corrupt organizer issue (see Bug > 752163, the id and commonName properties are null) and results in a null > icalString in the cal_attendees table. This seems to be no worse than it was > before, but maybe we can use this opportunity to remove or correct the > corrupt entries? Tough call. Theoretically, yes. The only way to do I've come up with is to set the string to null if it throws and then DELETE the entries that are null. This will make other attendees go away if the block throws, but on the other hand if we can't convert the attendee the user is going to have to restore from a storage backup anyway (and we do make backups). See new patch for details.

Comment on attachment 658200 [details] [diff] [review] Fix - v2 r=mmecca

Comment on attachment 658200 [details] [diff] [review] Fix - v2 You forgot to assign a new UUID to the changed interfaces. According to devmo "the uuid must be changed anytime any part of the interface or its ancestors are changed".

Indeed, good catch! I'll change the UUID before checkin.

Pushed to comm-central changeset e552b849d527