Closed Bug 348072 Opened 14 years ago Closed 14 years ago

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

Categories

(Calendar :: Internal Components, defect, major)

x86
All
defect
Not set
major

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: schaefer, Assigned: jminta)

Details

(Keywords: dataloss)

Attachments

(4 files)

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!
Flags: blocking0.3?
nominated for blocking 0.3 release because existing events are not displayed!
What kind of calendar are the events in?  (Local, Remote ICS, or Remote CalDAV?)
In the case I tested it was a local storage.
-> qawanted.  Only confirmed bugs can get blocking status, so we're going to need someone from QA to poke at this.
Keywords: qawanted
Confirmed with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20060809 Calendar/0.3a2+.
OS: Linux → All
Status: UNCONFIRMED → NEW
Ever confirmed: true
Is this storage only?
Flags: blocking0.3? → blocking0.3+
I Added the ICS Export of the Local storage Calendar file I used obtaining the bug
Keywords: qawantedtestcase
(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.
Keywords: testcaseqawanted
I saw this in Lightning 0.1 allready! I can confirm it in 0.1! Same behaviour.
Attached file 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.
Component: Calendar Views → Internal Components
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.
Assignee: nobody → jminta
Status: NEW → ASSIGNED
Attachment #233023 - Flags: second-review?(dmose)
Attachment #233023 - Flags: first-review?(mattwillis)
Comment on attachment 233023 [details] [diff] [review]
tweak start range

r=lilmatt
Attachment #233023 - Flags: first-review?(mattwillis) → first-review+
Comment on attachment 233023 [details] [diff] [review]
tweak start range

dmose says mickey should review this.
Attachment #233023 - Flags: second-review?(dmose) → second-review?(michael.buettner)
Keywords: qawanted
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.
Attachment #233023 - Flags: second-review?(michael.buettner) → second-review-
(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?
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.
> 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 on attachment 233023 [details] [diff] [review]
tweak start range

r=mickey per previous comment.
Attachment #233023 - Flags: second-review- → second-review+
Patch checked in.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
QA Contact: views → base
verified with
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060929 Sunbird/0.3
Status: RESOLVED → VERIFIED
Whiteboard: [litmus testcase wanted]
Litmus testcase 2639 created
Whiteboard: [litmus testcase wanted]
You need to log in before you can comment on or make changes to this bug.