Note: There are a few cases of duplicates in user autocompletion which are being worked on.

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

RESOLVED FIXED in Sunbird 0.3

Status

Calendar
Import and Export
RESOLVED FIXED
11 years ago
10 years ago

People

(Reporter: Jef Driesen, Assigned: Matthew (lilmatt) Willis)

Tracking

({dataloss})

Trunk
Sunbird 0.3
dataloss
Bug Flags:
in-litmus ?

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

11 years ago
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.
(Reporter)

Updated

11 years ago
Flags: blocking0.3?

Comment 1

11 years ago
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.
Flags: blocking0.3? → blocking0.3+
Keywords: dataloss, qawanted
Whiteboard: [needs testcase]

Comment 2

11 years ago
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.

Updated

11 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → Windows 2000
Version: unspecified → Trunk
(Assignee)

Updated

11 years ago
Whiteboard: [needs testcase]

Comment 3

11 years ago
shaver suggests that a potential workaround to use would be to use the TZID from the enclosing parent event.
(Assignee)

Comment 4

11 years ago
(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?
(Assignee)

Updated

11 years ago
Component: Provider: ICS/Webdav → Import and Export

Updated

11 years ago
Keywords: qawanted
Whiteboard: [needs input]
(Assignee)

Comment 5

11 years ago
(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

11 years ago
Sounds like a reasonable strategy to me.
Hardware: PC → All
(Assignee)

Comment 7

11 years ago
(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.
Whiteboard: [needs input] → [needs input from ctalbert jminta mvl dmose]
(Assignee)

Updated

11 years ago
Whiteboard: [needs input from ctalbert jminta mvl dmose] → [needs patch]
(Assignee)

Comment 8

11 years ago
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 
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.
(Assignee)

Updated

11 years ago
Whiteboard: [needs patch] → [patch most of the way there - needs some more love]
(Assignee)

Comment 9

11 years ago
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.
Assignee: nobody → lilmatt
Attachment #240111 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #240255 - Flags: second-review?(dmose)
(Assignee)

Updated

11 years ago
Attachment #240255 - Flags: first-review?(cmtalbert)
(Assignee)

Comment 10

11 years ago
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.
Attachment #240255 - Attachment is obsolete: true
Attachment #240285 - Flags: second-review?(dmose)
Attachment #240285 - Flags: first-review?(cmtalbert)
Attachment #240255 - Flags: second-review?(dmose)
Attachment #240255 - Flags: first-review?(cmtalbert)
(Assignee)

Updated

11 years ago
Whiteboard: [patch most of the way there - needs some more love] → [patch in hand][needs review ctalbert dmose]
(Assignee)

Updated

11 years ago
OS: Windows 2000 → All
Target Milestone: --- → Sunbird 0.3

Comment 11

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

r+ with a :-) (as per request). Seriously though, looks good.
Attachment #240285 - Flags: first-review?(cmtalbert) → first-review+

Updated

11 years ago
Whiteboard: [patch in hand][needs review ctalbert dmose] → [patch in hand][needs review dmose]
Comment on attachment 240285 [details] [diff] [review]
rev3 - Fixes it on storage also

Nice work; looks great!
Attachment #240285 - Flags: second-review?(dmose) → second-review+
(Assignee)

Comment 13

11 years ago
Patch checked in on MOZILLA_1_8_BRANCH and trunk.

-> FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Whiteboard: [patch in hand][needs review dmose] → [patch in hand][needs review dmose][litmus testcase wanted]

Updated

11 years ago
Whiteboard: [patch in hand][needs review dmose][litmus testcase wanted] → [litmus testcase wanted]
(Reporter)

Comment 14

11 years ago
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?
(Assignee)

Comment 15

11 years ago
No, this bug was specifically for importing.
I suspect we ought to relnote it however.
Keywords: relnote
Whiteboard: [litmus testcase wanted] → [litmus testcase wanted][cal-relnote]

Updated

10 years ago
Whiteboard: [litmus testcase wanted][cal-relnote] → [litmus testcase wanted]

Updated

10 years ago
Flags: in-litmus?
Whiteboard: [litmus testcase wanted]
I don't think that this should be relnoted any longer. Please speak up if you disagree.
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.