179985 - Cannot subscribe/import to .ics files created from old Sunbird versions due to multiple BEGIN:VCALENDAR END:VCALENDAR in a single file

Status: RESOLVED WORKSFORME

(Calendar :: Internal Components, defect)

Description

[Andy Koch]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1) Gecko/20020826
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1) Gecko/20020826

When exporting multiple events, the ics file generated has each event wrapped in
BEGIN/END VCALENDAR.

Reproducible: Always

Steps to Reproduce:
1.export two events from calendar
2.attempt to open the resulting ics file with korganizer - it will error
3.remove the:

END:VCALENDAR
BEGIN:VCALENDAR
VERSION
 :2.0
PRODID
 :-//Mozilla.org/NONSGML Mozilla Calendar V1.0//EN
METHOD
 :PUBLISH

save and attempt to import into korganizer - no error

Actual Results:  
errors in korganizer, other calendars just quietly ignore the events.

Expected Results:  
use only a single BEGIN:VCALENDAR END:VCALENDAR per ics file

Comment 1

[Mike Potter]

-> mostafa

Comment 2

[gary.frederick]

I believe Apple's iCal will only import the first event also.

Comment 3

[Mike Potter]

Marking major bug since it would be really good to be able to be used by other
applications, and its probably easier to fix our implementation than the other
applications.

Comment 4

[Hypo G]

Yes, I've run into the same problem... I was trying to get the ics file to sync
with my palm pilot using kpilot.. doesn't work.. tried importing to korganizer
to see what's going on.. the import fails as mentioned in this bug report.. a
resolution would be really helpful..

Thanks,
nirmal

Comment 5

[Ry4an Brase]

While it's totally not a solution, here's the tiny perl script I use to clean up
the saved .ics files for import into other systems that don't accept multiple
VCALENDARs in the same file:

#!/usr/bin/perl
$skipLines = 0;
while (<>) {
    if (/^END:VCALENDAR/) {
        $skipLines = 6;
    }
    if ($skipLines) {
        $skipLines--;
        $skipped = $_;
    } else {
        print;
    }
}
print $skipped;

Comment 6

[Ry4an Brase]

Welp, I looked into fixing this but it goes deeper than I feel comfortable
delving.  The oeICalImpl.cpp
(http://lxr.mozilla.org/mozilla/source/calendar/libxpical/oeICalImpl.cpp) file
seems to do all the writing by calling icalfileset_commit
(http://lxr.mozilla.org/mozilla/ident?i=icalfileset_commit) in icalfileset.c
(http://lxr.mozilla.org/mozilla/source/other-licenses/libical/src/libicalss/icalfileset.c)
as found in libical.  The icalfileset_commit method just iterates through all
the icalcomponents in the impl and calls icalcomponent_as_ical_string
(http://lxr.mozilla.org/mozilla/ident?i=icalcomponent_as_ical_string) on each
one.  In turn the icalcomponent_as_ical_string just recursively decends the
provided icalcomponent's own component children hierarcy dumping properties and
calling icalcomponent_as_ical_string as it goes.

A fix could be as simple as either adding a boolean flag to the
icalcomponent_as_ical_string causing it to discard the top level component of
the provided icalcomponent (which would bypass the BEGIN:VCALENDAR level and
start with the BEGIN:VEVENT) or perhaps, instead, decend one level inside the
icalfileset_commit function and pass the VEVENT/VTODO level icalcomponents
(rather than the VCALENDAR level) off to icalcomponent_as_ical_string.

Or maybe I'm way off base and the icalfileset *cluster being passed into
icalfleset_commit should rather than having lots of components have a single
VCALENDAR component that in turn contains all the VEVENTS and VTODOs. 

Or maybe I totally don't know what I'm talking about.

Comment 7

[Ry4an Brase]

Someone should give this bug 'dataloss' as a keyword because accessing calendar
created .ics data files using any other application and then resaving results in
the loss of all but one event.

Comment 8

[Mostafa Hosseini]

*** Bug 227822 has been marked as a duplicate of this bug. ***

Comment 9

[Mostafa Hosseini]

*** Bug 231751 has been marked as a duplicate of this bug. ***

Comment 10

[Ryan Shea]

I can confirm that only the first event will be imported into Apple iCal. 
Removing the:
END:VCALENDAR
BEGIN:VCALENDAR
lines, leaving the repeated prodid and version, will allow you to import into
iCal perfectly.

Comment 11

[Chris Charabaruk]

*** Bug 235574 has been marked as a duplicate of this bug. ***

Comment 12

[Disabled (no longer in use)]

Attachment: A proposed patch to fix this problem.

I traced down the "problem" to libxpical and the way events/todo objects are
added to the calendar. I came up with a patch that seems to be working fine as
far as I can tell. Please let me know if you think this patch is
(un)/acceptable.

The main components of my patch are:

-added a new function to libical called
icalerrorenum icalfileset_add_to_first_component(icalfileset *cluster,
				       icalcomponent* child)

This function adds the given component child to the first component of the
given cluster. If the cluster is empty, then a new VCALENDAR object is created
and the child component is added to that first VCALENDAR object.

-changed the AddEvent, ModifyEvent, AddTodo, ModifyTodo methods in
oeICalImpl.cpp in libxpical to add the event to the first component of the
calendar read from disk/network. This way all components are kept in the same
VCALENDAR object.

Comment 13

[Mostafa Hosseini]

Comments on the patch:
The patch is a good step towards solving the problem however these are the
shortcomings:

- There is no need to define a new function at the libical level to add events
to already existing vcalendar objects. The basic API functions can be used to do
the job. It is preferred not to touch libical and keep our libical branch as
close as possible to the official code. Therefore the additional function should
better be defined in oeICalImpl.cpp itself probably after icalcomponent_fetch
and without a need to modify header files.

- A *plain* VCALENDAR object is created if there's none while the standard
indicates that a PRODID and VERSION should be present in each VCALENDAR object.

- Any existing VTIMEZONE components will be discarded in the process. If the
event is using timezones the corresponding VTIMEZONE is added to its VCALENDAR
wrapper but this will be lost when just the VEVENT object is extracted.

- ( This is not about the patch but just an observation ); Even if the PRODID
and the VERSION already exist in the VCALENDAR object, should we change the
values to our own defaults? For example if we are adding an event to a VCALENDAR
created by evolution or Apple iCal, should we change the PRODID to our own or
leave it as it is? If yes, we are claiming that the rest of the events are
created by us, if no, we are losing our signature over the event.
In this case one would prefer to create a new VCALENDAR object but again that
beats the purpose of this bug. How does iCal and Evolution deal with this issue?

Although I wish other applications would comply to the standard and simply
support our implementation, I hope the momentum and motive for fixing this bug
doesn't die so please feel free to revise the patch towards perfection.

Comment 14

[Axel Junge]

(In reply to comment #13)

On point #3:
Evolution and KOrganizer both override PRODID and VERSION without notification
and claim the ownership. KOrganizer marks it own entries by a preceding
"KOrganizer-" string in UID. Hence the original program used for creation cannot
be lost. Maybe that is a possibilty to deal with this issue.

Comment 15

[Cristian Tapus]

Attachment: This is a new patch following suggestions from comment #13

This patch doesn't touch anything else except the oeICalImpl.cpp from
calendar/libxpical. 
For now, the PRODID and VERSION are replaced with Mozilla PRODID and VERSION. 
iCal does the same thing, and it is expected behavior since the content of the
calendar file is read by Mozilla, interpreted and then wrote back to the file
as per Mozilla interpretation.

Comment 16

[Cristian Tapus]

Attachment: Added explicit casts to prevent compilation errors for mozilla 1.7

This patch  is basically the same as my previous commited patch, but I added
explicit casts in a couple places to make it work with mozilla 1.7b release.

Comment 17

[Jussi Kukkonen]

*** Bug 239464 has been marked as a duplicate of this bug. ***

Comment 18

[Aleksey Nogin]

Comment on attachment 145229 [details] [diff] [review]
Added explicit casts to prevent compilation errors for mozilla 1.7

I've been running with this patch for a while, and it seems to be working
correctly. Can somebody review it and check it in? Thanks!

Comment 19

[Mostafa Hosseini]

Comment on attachment 145229 [details] [diff] [review]
Added explicit casts to prevent compilation errors for mozilla 1.7

Great patch, thanks.
One remaining issue though: If a specific vtimezone already exists in the main
VCALENDAR object, adding it again is redundant and probably against the
standard. There should be a mechanism in the new function to check for the
existence of the same vtimezone.
It would be great to have a patch for that too.
I won't let this issue prevent checking this in though.
Checked in.

Comment 20

[Jakob Fredriksson]

*** Bug 242060 has been marked as a duplicate of this bug. ***

Comment 21

[Amos Hayes]

Anyone able to provide the final patch Mostafa was hoping for? Would be nice to get this off the list.

Comment 22

[Simon Paquet [:sipaq]]

mvl, this is the issue that we talked about Friday evening on IRC.

Comment 23

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

0.2 doesn't create multiple vcalendar components. Only older version do. Also,
we don't track problems in other products here.

Comment 24

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

Actaully, on better reading of comment 0, this bug is worksforme on trunk. It is
about producing, not subscribing. trunk produces only one vcalendar when exporting.

Comment 25

[Reed Loden [:reed]]

The bugspam monkeys have been set free and are feeding on Calendar :: Internal Components. Be afraid for your sanity!