485912 - Recurring events with monthly rule: "every" + "day of the month", show a wrong recurrence summary and cause an error.

Status: VERIFIED FIXED

(Calendar :: General, defect)

Description

[Decathlon]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; it-IT; rv:1.9.0.7) Gecko/2009021910 Firefox/3.0.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; it; rv:1.9.1b4pre) Gecko/20090329 Lightning/1.0pre (2009-03-29) Shredder/3.0b3pre


The recurrence summary for events with monthly rule "every day of the month" don't match with the rule, and a click on it to open Recurrence Edit dialog causes an error on the console.


Reproducible: Always

Steps to Reproduce:

1. create a new event;
2. select repeat: "custom...";
3. select recurrence pattern "monthly" and "every" + "day of the month" from  drop down menu;
4. conferm the settings with OK button;
5. click on recurrence summary link on New Event dialog.


Actual Results:  

After step 4:
  New Event dialog shows a recurrence summary that says:
  "Occurs day 0 of every month"
After step 5:
  - an error appears in the console:
       "Error: days[index] is undefined
        Source File: chrome://calendar/content/calendar-daypicker.xml
        Line: 273"
  - Edit Recurrence dialog shows "monthly" on repeat drop down menu but
    controls on the dialog are those one of "daily" option. 


Expected Results:  

A different summary recurrence;
no errors in the console


The error on the console is due to 'index' that inside calendar-daypicker.xml (line 273) is negative (-1).
(the same error as https://bugzilla.mozilla.org/show_bug.cgi?id=462048#c1 but with value 0 from the month rule).

Comment 1

[Stefan Sitter [:ssitter]]

Confirmed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090404 Calendar/1.0pre.

The "Every" rule doesn't exists in 0.9 and was added with Bug 463273.

Comment 2

[Decathlon]

I would try to fix this, but I need some informations about the best way to do it.
I've read iCalendar specification (http://tools.ietf.org/html/rfc2445) about monthly rule and, as far as I can see, there isn't a specific rule to match exactly the case "every day of the month" but only the possibility to add required days to other rules.
It seems to me there are two ways to make a rule "Every day of the month" (unless I missed something and this is probable):

BYMONTHDAY=1,2,3,...,31
 or
BYDAY=MO,TU,WE,TH,FR,SA,SU

The first one is handled by Lightnig\Sunbird UI because in Edit Recurrence window it can show the month grid with all days marked, moreover there is a proper string in recurrence summary without nothing else to add: "Occurs day 1,2,3,...,30 and 31 of every N months" (although the rule "every day of the month" set with dropdown menu becomes a rule "day 1,2,3,...,30 and 31 of every N months" showed with month grid).

The BYDAY rule with all weekdays isn't handled by L\S UI without modifies because it needs to be detected as a particular case to be showed with dropdown menu "every"+"day of the month" and needs a new string in recurrence summary: "Occurs every day of every N month". There would be a bit more work but perhaps it would look better. 

Is there another way to handle the rule "every day of the month"?
If the rule can be handled only with those two ways, what's better, BYMONTHDAY or BYDAY?

Comment 3

[Hubert Gajewski]

CC: Fallen, he seems to be the best person to answer for your question.

Comment 4

[Decathlon]

Attachment: Proposal patch with BYDAY rule

The patch generates a monthly rule with BYDAY=SU,MO,TU,WE,TH,FR,SA when "every" and "day of the month" options are selected from dropdown menu in Edit Recurrence dialog.
It checks if BYDAY rule has seven days with values from 1 to 7 when the code initializes controls and when it generates a string for the recurrence summary. I've added a new string "every day of the month every N month" as well.

If the issue must be resolved in a different way, please let me know how.

Comment 5

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 377559 [details] [diff] [review]
Proposal patch with BYDAY rule

Maybe ssitter can take a look at this one, he also reviewed my patch w.r.t the recurrence dialog.

Comment 6

[Stefan Sitter [:ssitter]]

Comment on attachment 377559 [details] [diff] [review]
Proposal patch with BYDAY rule

During testing I noticed the following issue: I created a repeating event with the 'every day of the month rule' that looks like:

	BEGIN:VCALENDAR
	PRODID:-//Mozilla.org/NONSGML Mozilla Calendar V1.1//EN
	VERSION:2.0
	BEGIN:VEVENT
	CREATED:20090529T202927Z
	LAST-MODIFIED:20090529T203001Z
	DTSTAMP:20090529T203001Z
	UID:fbacc989-a182-4def-8c76-15219d89689c
	SUMMARY:TEST
	RRULE:FREQ=MONTHLY;BYDAY=SU,MO,TU,WE,TH,FR,SA
	DTSTART;VALUE=DATE:20090601
	DTEND;VALUE=DATE:20090602
	TRANSP:TRANSPARENT
	END:VEVENT
	END:VCALENDAR

But it doesn't display at every day of the month. It is only shown on 20090601 and 20090607-20090630. Days 20090602-20090606 are missing.

Comment 7

[Decathlon]

Hello Stefan,
I imported the calendar you posted and it looks fine with the patch: events are displayed correctly every day, controls are set to "every" "day of the month" and recurrence summary says the right sentence.
I get the issue you mentioned if I create a repeating event (every day of the month) with start and due date: 
20090601 0:00
20090602 0:00
i.e.
DTSTART;VALUE=DATE:20090601T000000
DTEND;VALUE=DATE:20090602T000000
with Lightning I'm not able to build an event like your (without the time part of the date) unless manually editing the event.
I guess this event is the same you posted (though I haven't found on iCal specifications about the meaning of the absence of "T" part) and I get the same behavior: after the event is filed, I get the issue you mentioned, but exporting as ics and reimporting on a new calendar, the event display fine.


I get the same behavior if I create with Lightning a weekly event like this:

start 20090601 0:00, end 20090602 0:00
every two week on day SU,MO,TU,WE,TH,FR,SA (every day of the week)

the exported ics is (I cut timezone and daylight informations):

BEGIN:VCALENDAR
PRODID:-//Mozilla.org/NONSGML Mozilla Calendar V1.1//EN
VERSION:2.0
BEGIN:VEVENT
CREATED:20090530T073422Z
LAST-MODIFIED:20090530T073743Z
DTSTAMP:20090530T073743Z
UID:ab33d635-0c38-46b8-88c5-6782993fccd6
SUMMARY:New Event
RRULE:FREQ=WEEKLY;INTERVAL=2;BYDAY=SU,MO,TU,WE,TH,FR,SA
DTSTART;TZID=Europe/Berlin:20090601T000000
DTEND;TZID=Europe/Berlin:20090602T000000
SEQUENCE:2
X-MOZ-GENERATION:2
END:VEVENT
END:VCALENDAR

it displays the event in a wrong way:
 20090601, 20090608-20090614,  20090622-20090628 etc.
but if you export the calendar with that event the re-import it on a new calendar, events are correctly displayed:
 20090601-20090607,  20090615-20090621,  20090629-20090705 etc.

This is for sure a bug (I've searched it but I haven't found nothing), I think this bug causes the issue because the patch, at last, transforms the monthly rule "every day of the month" to a monthly rule with a BYDAY rule " every day of the week".
What's your opinion?

Comment 8

[Stefan Sitter [:ssitter]]

The event in Comment #6 is an all-day event starting/ending 01-June-2009 and set to repeat 'every day of the month' with your patch applied.

Maybe the issue is caused by my Operating System setting to start the week on Monday? Adjusting the same setting in Sunbird doesn't change anything regarding the incorrect display mentioned in Comment #6.

Comment 9

[Decathlon]

(In reply to comment #8)
> The event in Comment #6 is an all-day event starting/ending 01-June-2009 and
> set to repeat 'every day of the month' with your patch applied.

Obviously I haven't thought to the simplest thing.

> Maybe the issue is caused by my Operating System setting to start the week on
> Monday? Adjusting the same setting in Sunbird doesn't change anything regarding
> the incorrect display mentioned in Comment #6.

I'm not able to change settings like the start of the week on WinXP, maybe you can suggest me how to do it to do some other testing?

For the testing I've done it seems that the problem happens if starting day is before the first Sunday of the month and it seems more related to some problems of Calendar in reading the couple of rules monthly-byday than to the patch.
The patch sets the rule with the setComponent() function:

recRule.setComponent("BYDAY", 7, [1, 2, 3, 4, 5, 6, 7]);

and generates a rule like this:

RRULE:FREQ=MONTHLY;BYDAY=SU,MO,TU,WE,TH,FR,SA

that, afaik, means every weekday of every month, but if starting day is inside the first week of the month, occurrences start only from the first Sunday following the starting day, instead, if recurring event starts after the first Sunday, every occurrence is in the right place.
Unless I missed something, it seems to me a Calendar problem since the rule is the same in both cases.

I can make the patch working, even the first week of the month, if I change the order of weekday in the array argument of the setComponent() function, adjusting it to the day following the event start
e.g. 
recurring event starting the third of June (2009/06/03 is the Wednesday of the first week) displays fine if I set weekdays in the setComponent() function, starting from 5 because Wednesday is the 4th day of the week (1=SU ... 7=SA) i.e. if I write the patch in this way:

recRule.setComponent("BYDAY", 7, [5, 6, 7, 1, 2, 3, 4]);

that generates a rule like this:
RRULE:FREQ=MONTHLY;BYDAY=TH,FR,SA,SU,MO,TU,WE

I'm not an expert, but reading iCalendar specifications (http://tools.ietf.org/html/rfc2445#section-4.1.1) neither the order in the list of values in BYDAY rule nor the week of the starting day should matter, so that rule should be equivalent to that one generated by the patch without modifies but this isn't true.

To avoid problems, I'm going to attach a patch with a BYMONTHDAY rule instead of a BYDAY (as described in comment #2) that wouldn't seem having this issue (I hope).

Comment 10

[Decathlon]

Attachment: patch with BYMONTHDAY rule

This patch works like the previous one, but generates the rule "every day of the month" by using a monthly rule and a BYMONTHDAY tag to generate an iCalendar rule like this:

RRULE:FREQ=MONTHLY;BYMONTHDAY=1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18
 ,19,20,21,22,23,24,25,26,27,28,29,30,31

It doesn't use the daypicker in Edit Recurrence dialog, but only the dropdown menu (reading and writing the rule) like the previous patch.

This time events starting in the first week seem working fine, I hope there aren't others problems.

Comment 11

[Stefan Sitter [:ssitter]]

Comment on attachment 382386 [details] [diff] [review]
patch with BYMONTHDAY rule

r=ssitter
This one works better for me.

Comment 12

[Decathlon]

Attachment: final patch

I've only changed position of some code lines inside the property file 'calendar-event-dialog.properties', where I've moved "every day of the month" string near others strings about monthly rules. Nothing else has changed.
No need for review.

Comment 13

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

I realize the strings for this are late, but I think its worth it.

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

-> FIXED

Comment 14

[Damian Szczepanik]

verifying with
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.9pre) Gecko/20100305 Calendar/1.0b2pre

Comment 15

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