348072 - repeating, multi days spanning events are not displayed in any view if the starting day is out of view

Status: VERIFIED FIXED

(Calendar :: Internal Components, defect)

Description

[LittleOnestone]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; de; rv:1.8.0.4) Gecko/20060508 Firefox/1.5.0.4
Build Identifier: Thunderbird Version 1.5.0.5 (20060719) + Lightning 0.1+ (20060809)

If you create an repeating event that lasts more than one day, the event is not displayed on any view if the starting day is not in the range of the actual view!

This happens with all actual nightly Lightning builds - at least on Linux with Thunderbird 1.5x

Reproducible: Always

Steps to Reproduce:
1. Create a multi day spanning event (e.g. Fr-Tue)
2. make it repeating (e.g. weekly) - if not repeating, all is displayed correctly
3. switch to any view. The event is not displayed if the starting day (e.g. the according Fr) is not part of the actual visible view. (e.g. Month starting on Sat to Tue, weekview [assuming the wek starts Sun or Mon], or dayview on Sat to Tue)



Expected Results:  
Every Event that has not ended yet (on the displayed day) should be displayed in any view, even if the starting day of the event is out of sight!

Comment 1

[LittleOnestone]

nominated for blocking 0.3 release because existing events are not displayed!

Comment 2

[Joey Minta]

What kind of calendar are the events in?  (Local, Remote ICS, or Remote CalDAV?)

Comment 3

[LittleOnestone]

In the case I tested it was a local storage.

Comment 4

[Joey Minta]

-> qawanted.  Only confirmed bugs can get blocking status, so we're going to need someone from QA to poke at this.

Comment 5

[Stefan Sitter [:ssitter]]

Confirmed with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060809 Calendar/0.3a2+.

Comment 6

[Stefan Sitter [:ssitter]]

Attachment: testcase with event

Comment 7

[Joey Minta]

Is this storage only?

Comment 8

[LittleOnestone]

Attachment: Export of the Local Calendar I used to an ICS

I Added the ICS Export of the Local storage Calendar file I used obtaining the bug

Comment 9

[Stefan Sitter [:ssitter]]

(In reply to comment #7)
> Is this storage only?

Opening the testcase as a local ics file shows the same problem. No regression, I see that in 0.3a1 and 0.3a2 too.

Comment 10

[LittleOnestone]

I saw this in Lightning 0.1 allready! I can confirm it in 0.1! Same behaviour.

Comment 11

[Joey Minta]

Attachment: testcase.js

This looks like a problem in libical.  The attached xpcshell test shows that we won't ever get enough occurrences back in this scenario.

Comment 12

[Joey Minta]

Attachment: tweak start range

Looking closer at this bug, I'm realizing that libical's behavior, while perhaps not ideal, is at least well defined and consistent.  Therefore, I've decided to patch our wrapping around libical instead.  We're going to move the occurrence range back by the duration of the event.  This assures us that we will then get occurrences that overlap the actual desired range.  With this patch I pass the xpcshell testcase.

Comment 13

[Matthew (lilmatt) Willis]

Comment on attachment 233023 [details] [diff] [review]
tweak start range

r=lilmatt

Comment 14

[Joey Minta]

Comment on attachment 233023 [details] [diff] [review]
tweak start range

dmose says mickey should review this.

Comment 15

[Michael Büttner (no reviews TFN)]

Comment on attachment 233023 [details] [diff] [review]
tweak start range

In my eyes this is a workaround that hides the real problem. As far as I can tell, this doesn't seem to be a problem of libical, but of the code iterating through the occurrences. 
I would suspect that the root cause of the issue can be found here: http://lxr.mozilla.org/seamonkey/source/calendar/base/src/calRecurrenceRule.cpp#471
since occurrences with dates before the search range will be filtered out.

Comment 16

[Joey Minta]

(In reply to comment #15)
> (From update of attachment 233023 [details] [diff] [review] [edit])
> In my eyes this is a workaround that hides the real problem. As far as I can
> tell, this doesn't seem to be a problem of libical, but of the code iterating
> through the occurrences. 
> I would suspect that the root cause of the issue can be found here:
> http://lxr.mozilla.org/seamonkey/source/calendar/base/src/calRecurrenceRule.cpp#471
> since occurrences with dates before the search range will be filtered out.
> 
Looking at calRecurrenceRule.cpp, I think that I'd essentially write the same patch I have here (offseting the start by the item duration), only in c++ in that file.  This would mean we'd have to do the computation for each rule, rather than once for all rules.  Do you see another fix or a compelling reason to move this logic into that file?

Comment 17

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

The problem is that there is not enough documentation. calIRecurrenceRule.idl doesn't know about the event it belongs to. It can only work on what it knows: the starttime. But that fact is not documented. The description of getOccurences should mention that. If we add that documentation, i think adjusting the caller is ok.
The other way would be to also add a duration parameter to getOccurences, and make that code work better.
I think fixing the caller is a workaround, and fixing the c++ is slightly cleaner. But my opinion isn't that strong, given that we need to add more parameters etc.

Comment 18

[Michael Büttner (no reviews TFN)]

> Looking at calRecurrenceRule.cpp, I think that I'd essentially write the same
> patch I have here (offseting the start by the item duration), only in c++ in
> that file.  This would mean we'd have to do the computation for each rule,
> rather than once for all rules.  Do you see another fix or a compelling reason
> to move this logic into that file?
I'd find it much cleaner to have getOccurrences() handle this case for the caller.   Unfortunately I must admit that there's no easy way to do this. granted, your approach seems to be the least invasive. So probably we should just go ahead and take this patch as it is.

Comment 19

[Michael Büttner (no reviews TFN)]

Comment on attachment 233023 [details] [diff] [review]
tweak start range

r=mickey per previous comment.

Comment 20

[Joey Minta]

Patch checked in.

Comment 21

[Damian Szczepanik]

verified with
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060929 Sunbird/0.3

Comment 22

[Damian Szczepanik]

Litmus testcase 2639 created