Last Comment Bug 352795 - Sunbird 0.3a2+ ignores exceptions when importing ics calendar from sunbird 0.2
: Sunbird 0.3a2+ ignores exceptions when importing ics calendar from sunbird 0.2
: dataloss
Product: Calendar
Classification: Client Software
Component: Import and Export (show other bugs)
: Trunk
: All All
-- normal (vote)
: Sunbird 0.3
Assigned To: Matthew (lilmatt) Willis
Depends on:
  Show dependency treegraph
Reported: 2006-09-15 02:27 PDT by Jef Driesen
Modified: 2007-09-08 05:43 PDT (History)
1 user (show)
cmtalbert: in‑litmus?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

testcase from sunbird 0.2 (670 bytes, text/plain)
2006-09-15 14:34 PDT, Stefan Sitter
no flags Details
rev0 - INCOMPLETE - This works great, at first. (4.05 KB, patch)
2006-09-26 00:15 PDT, Matthew (lilmatt) Willis
no flags Details | Diff | Splinter Review
rev2 - Works good like (4.13 KB, patch)
2006-09-26 18:19 PDT, Matthew (lilmatt) Willis
no flags Details | Diff | Splinter Review
rev3 - Fixes it on storage also (4.19 KB, patch)
2006-09-26 22:08 PDT, Matthew (lilmatt) Willis
cmtalbert: first‑review+
dmose: second‑review+
Details | Diff | Splinter Review

Description User image Jef Driesen 2006-09-15 02:27:33 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv: Gecko/20060728 Firefox/
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060914 Calendar/0.3a2+

When importing an ics calendar, created with sunbird 0.2, exceptions are 
ignored. This is probably caused by an EXDATE without TZID or UTC. This 
information is added correctly in later versions (bug 330573), but sunbird 0.2 
still creates events using this older format. That will cause trouble 
when users are upgrading from 0.2 to 0.3.

Reproducible: Always

Steps to Reproduce:
1. Start sunbird 0.2
1. Create an new ics calendar (local or remote).
2. Create a recurring event and one or more exceptions.
3. Start sunbird 0.3a2+
4. Import the newly created ics calendar (or subscribe to it).
Actual Results:  
Exceptions of the imported event are ignored. That means that in the views, an event is also shown on days that originally had an exception.

Expected Results:  
The exception information should not be ignored.
Comment 1 User image Dan Mosedale (:dmose) 2006-09-15 13:27:29 PDT
If we could get a copy of some test ICS from 0.2 here to use for reproducing this, that would be very helpful.  Assuming we can confirm this, we need to fix it before 0.3.
Comment 2 User image Stefan Sitter 2006-09-15 14:34:33 PDT
Created attachment 238688 [details]
testcase from sunbird 0.2

Confirmed for non all-day event (storage and ics provider).

Event on 19-Sep-2006, repeats daily for 5 occurrences. 20-Sep-2006 and 22-Sep-2006 are marked as exception. This event was created using Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a5) Gecko/20050203 Mozilla Sunbird/0.2.

Note: If the event is marked as all-day the problem doesn't exist and the event is displayed fine after import/subscribe.
Comment 3 User image Dan Mosedale (:dmose) 2006-09-19 12:21:38 PDT
shaver suggests that a potential workaround to use would be to use the TZID from the enclosing parent event.
Comment 4 User image Matthew (lilmatt) Willis 2006-09-19 13:49:00 PDT
(In reply to comment #3)

In the testcase above, I don't see a timezone for the event at all, nor do I see the DTSTART or DTEND defined in UTC/Z.

What to do then?
Comment 5 User image Matthew (lilmatt) Willis 2006-09-20 16:32:39 PDT
(In reply to comment #2)
> Event on 19-Sep-2006, repeats daily for 5 occurrences. 20-Sep-2006 and
> 22-Sep-2006 are marked as exception.

Note that 0.2's EXDATEs are for T000000, not T120000 (which is when the meeting is.) If you change the EXDATEs to T120000, leaving them floating, they will get interpreted properly.

Since 0.2 only allows date-based exceptions (since it uses a datepicker) we should be able to workaround this by assuming that you meant the event on the day in question. This should only be used in the case of a 0.2 file.

Comment 6 User image Dan Mosedale (:dmose) 2006-09-20 16:41:14 PDT
Sounds like a reasonable strategy to me.
Comment 7 User image Matthew (lilmatt) Willis 2006-09-23 23:10:00 PDT
(In reply to comment #6)
> Sounds like a reasonable strategy to me.

Where do we want to do this? I'm only asking because the ICS import code is very simple, and not cluttered by "if (this.prodId.indexOf('Mozilla Calendar') == -1)" stuff. 

The Right Place would be in the migration code which we're not taking for 0.3.
Comment 8 User image Matthew (lilmatt) Willis 2006-09-26 00:15:44 PDT
Created attachment 240111 [details] [diff] [review]
rev0 - INCOMPLETE - This works great, at first.

This works great at first, but fails because the EXDATE remains floating while the DTSTART gets pinned to our default timezone.

For example, using the testcase ICS and having Sunbird set to America/Los_Angeles (UTC-7 at the moment), when first importing (if I have the appropriate week in view before importing), I see the event correctly on Tu Th Sa. If I refresh the view, or quit and reopen, the event is back to occurring on all 5 days: Tu -> Sa.

Originally, the DTSTART was 12:00 floating.
Looking at the ICS output from the event dialog (without editing anything), the DTSTART has become 

While the EXDATEs are still floating, but adjusted for my timezone. Since they're STILL floating, the fact that 19:00UTC == 12:00 Pacific doesn't matter. They no longer match the occurrences that I want to omit.

So it needs more work, which I may not have time to touch while at interop.

Here it is.
Comment 9 User image Matthew (lilmatt) Willis 2006-09-26 18:19:12 PDT
Created attachment 240255 [details] [diff] [review]
rev2 - Works good like

This patch checks for an old Sunbird 0.2 file with EXDATEs.
Then if the EXDATE's time doesn't match the occurrence's DTSTART time, it adjusts it to match the DTSTART time. (NOTE: NOT the date)

Note that this fix has exposed a bug in the storage provider. The initial import will work and draw appropriately, but get screwed up on the first refresh of the views.

If you import into a "remote" ICS file (like to "On the network"  file:///foo.ics), you can test this patch without being affected by this bug.
Comment 10 User image Matthew (lilmatt) Willis 2006-09-26 22:08:27 PDT
Created attachment 240285 [details] [diff] [review]
rev3 - Fixes it on storage also

I was a moron on the previous patch. Not only did I miss a bunch of semicolons, but I forgot to normalize() the newRitem's datetime after creating it.

Adding a .normalize(); to the time after creating it is crucial to the nativeTime being generated properly. While CalDAV and ICS providers store EXDATEs in YYYYMMDDHHmm format, storage stores them in milliseconds from the epoch, and not normalizing caused dateToText() in the storage provider to come up with the wrong time in the EXDATE.

This fixes it across the board, and was tested against storage, ICS, Cosmo, and Apple CalDAV.
Comment 11 User image cmtalbert 2006-09-27 08:44:18 PDT
Comment on attachment 240285 [details] [diff] [review]
rev3 - Fixes it on storage also

r+ with a :-) (as per request). Seriously though, looks good.
Comment 12 User image Dan Mosedale (:dmose) 2006-09-27 09:48:01 PDT
Comment on attachment 240285 [details] [diff] [review]
rev3 - Fixes it on storage also

Nice work; looks great!
Comment 13 User image Matthew (lilmatt) Willis 2006-09-27 09:53:58 PDT
Patch checked in on MOZILLA_1_8_BRANCH and trunk.

Comment 14 User image Jef Driesen 2006-10-02 01:21:23 PDT
This patch seems to work only when importing the sunbird 0.2 calendar, but not when subscribing to it. I tried both file:// and http:// but the result is the same. Should I reopen this bug?
Comment 15 User image Matthew (lilmatt) Willis 2006-10-02 07:18:31 PDT
No, this bug was specifically for importing.
I suspect we ought to relnote it however.
Comment 16 User image Simon Paquet [:sipaq] 2007-09-08 05:43:44 PDT
I don't think that this should be relnoted any longer. Please speak up if you disagree.

Note You need to log in before you can comment on or make changes to this bug.