Closed Bug 469840 Opened 11 years ago Closed 11 years ago

Recurring Sundays incorrect

Categories

(Calendar :: Internal Components, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: James.Peterson, Assigned: dbo)

Details

(Keywords: regression, Whiteboard: [libical-upstream+])

Attachments

(5 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.3) Gecko/2008092416 Firefox/3.0.3
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1b3pre) Gecko/20081216 Lightning/1.0pre Shredder/3.0b2pre

If start of week is anything other then Sunday the recurring days are incorrect.


Reproducible: Always

Steps to Reproduce:
1. Set start of week to Monday
2. Add recurring weekends(Fri-Sun) every 2 weeks
3. View displayed results
Actual Results:  
Friday & Saturday are correct but the previous Sunday is being selected and not the Sunday following.

Selected Dec 14th (Sun)
Selected Dec 19th-20th (Fri-Sat)

Expected Results:  
Selected Dec 19th-21st (Fri-Sun)


This was not an issue in lighting up to 0.9.
I have just noticed this in 1.0 version.
Confirming with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20081216 Lightning/1.0pre Shredder/3.0b2pre

It seems that the weekly recurrence pattern always assumes a week start at Sunday now, while before (in 0.9) it basically assumed a week start on another day.

From looking at the changes to recurrence code, bug 392465 looks like a possible culprit. Daniel, can you please take a look at this?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
OS: Linux → All
Hardware: PC → All
I don't see changes to expanding recurring series in bug 392465; why do you think it's related? I rather think that libical 0.40 might have a regression or our changes to it have regressed wkst: hg diff -r 714 libical/src/libical
James, could you export the event as ICS and attach that to this bug?

I think the behavior might be correct, for example if the startdate of the event is on sunday.
(In reply to comment #3)
> I don't see changes to expanding recurring series in bug 392465; why do 
> you think it's related? I rather think that libical 0.40 might have a 
> regression or our changes to it have regressed

That may very well be the case. I only thought bug 392465 might be the possible 
cause, because it made some changes to our recurrence code, but you know this 
code area far better than I, of course.

(In reply to comment #4)
> James, could you export the event as ICS and attach that to this bug?
> 
> I think the behavior might be correct, for example if the startdate of the
> event is on sunday.

mvl, see the two attached events. The generated .ics looks very similar. The 
only difference I see is in the lines

X-MOZ-GENERATION:4
SEQUENCE:2

compared to 

X-MOZ-GENERATION:2

in the 0.9 file.
Attachment #353392 - Attachment mime type: text/calendar → text/plain
Attachment #353394 - Attachment mime type: text/calendar → text/plain
Attached file Export of event
Note: Event is using gdata-provider
Attachment #353736 - Attachment mime type: application/octet-stream → text/plain
Flags: blocking-calendar1.0+
Debugging libical it seems to be a regression of it. I suspect the following change is the cause:
<http://freeassociation.svn.sourceforge.net/viewvc/freeassociation/trunk/libical/src/libical/icalrecur.c?view=diff&r1=743&r2=744>, line 1607 and following.

Reverting that (taking week start into account as previously) actually fixes the bug; I've attached unit tests for that (including a test case for different WKST).

I am not sure why that change has actually been done, can't relate a bug id to it.
Assignee: nobody → daniel.boelzle
Status: NEW → ASSIGNED
Attachment #353825 - Flags: review?(philipp)
Flags: in-testsuite?
(In reply to comment #9)
Maybe Wilfried knows why the change was applied to libical that is supposed to cause this regression. In that case it probably should be fixed upstream too.
Assignee: daniel.boelzle → nobody
Component: General → Internal Components
QA Contact: general → base
Assignee: nobody → daniel.boelzle
Comment on attachment 353825 [details] [diff] [review]
proposed fix including unit tests - v1

r=philipp

I'll post this to the libical list soon.
Attachment #353825 - Flags: review?(philipp) → review+
Whiteboard: [libical-upstream?]
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/de52d877915d>

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → 1.0
FYI: the patch for this bug has been applied to upstream libical.
Thanks for the note, sorry for not taking care earlier!
Whiteboard: [libical-upstream?] → [libical-upstream+]
Checked in lightning and sunbird build 20090106 -> VERIFIED.
Status: RESOLVED → VERIFIED
These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.