469840 - Recurring Sundays incorrect

Status: VERIFIED FIXED

(Calendar :: Internal Components, defect)

Description

[James Peterson]

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.

Comment 1

[James Peterson]

Attachment: Image of recurring setup

Comment 2

[Simon Paquet [:sipaq]]

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?

Comment 3

[Daniel Boelzle [:dbo]]

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

Comment 4

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

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.

Comment 5

[Simon Paquet [:sipaq]]

Attachment: Test event generated with 0.9

Comment 6

[Simon Paquet [:sipaq]]

Attachment: Test event generated with 1.0pre

Comment 7

[Simon Paquet [:sipaq]]

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

Comment 8

[James Peterson]

Attachment: Export of event

Note: Event is using gdata-provider

Comment 9

[Daniel Boelzle [:dbo]]

Attachment: proposed fix including unit tests - v1

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.

Comment 10

[Stefan Sitter [:ssitter]]

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

Comment 11

[Philipp Kewisch [:Fallen][☁️📆]]

Comment on attachment 353825 [details] [diff] [review]
proposed fix including unit tests - v1

r=philipp

I'll post this to the libical list soon.

Comment 12

[Daniel Boelzle [:dbo]]

Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/de52d877915d>

-> FIXED

Comment 13

[Art Cancro]

FYI: the patch for this bug has been applied to upstream libical.

Comment 14

[Philipp Kewisch [:Fallen][☁️📆]]

Thanks for the note, sorry for not taking care earlier!

Comment 15

[Andreas Treumann]

Checked in lightning and sunbird build 20090106 -> VERIFIED.

Comment 16

[Philipp Kewisch [:Fallen][☁️📆]]

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