352795 - Sunbird 0.3a2+ ignores exceptions when importing ics calendar from sunbird 0.2

Status: RESOLVED FIXED

(Calendar :: Import and Export, defect)

Description

[Jef Driesen]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.6) Gecko/20060728 Firefox/1.5.0.6
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

[Dan Mosedale (:dmosedale, :dmose)]

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

[Stefan Sitter [:ssitter]]

Attachment: 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

[Dan Mosedale (:dmosedale, :dmose)]

shaver suggests that a potential workaround to use would be to use the TZID from the enclosing parent event.

Comment 4

[Matthew (lilmatt) Willis]

(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

[Matthew (lilmatt) Willis]

(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.

Thoughts?

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Sounds like a reasonable strategy to me.

Comment 7

[Matthew (lilmatt) Willis]

(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

[Matthew (lilmatt) Willis]

Attachment: 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 
DTSTART;TZID=/mozilla.org/20050126_1/America/Los_Angeles:20060919T120000

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.
EXDATE:20060920T190000
EXDATE:20060922T190000

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

Here it is.

Comment 9

[Matthew (lilmatt) Willis]

Attachment: 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

[Matthew (lilmatt) Willis]

Attachment: 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

[cmtalbert]

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

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 240285 [details] [diff] [review]
rev3 - Fixes it on storage also

Nice work; looks great!

Comment 13

[Matthew (lilmatt) Willis]

Patch checked in on MOZILLA_1_8_BRANCH and trunk.

-> FIXED

Comment 14

[Jef Driesen]

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

[Matthew (lilmatt) Willis]

No, this bug was specifically for importing.
I suspect we ought to relnote it however.

Comment 16

[Simon Paquet [:sipaq]]

I don't think that this should be relnoted any longer. Please speak up if you disagree.