The default bug view has changed. See this FAQ.

BYSETPOS not considered when getting occurrences

NEW
Unassigned

Status

Calendar
Internal Components
11 years ago
6 years ago

People

(Reporter: Joey Minta, Unassigned)

Tracking

(Blocks: 1 bug, {dataloss, relnote})

unspecified
dataloss, relnote
Bug Flags:
wanted-calendar1.0 +

Details

(Whiteboard: [needs patch], URL)

Attachments

(2 attachments)

(Reporter)

Description

11 years ago
It seems (from limited testing) that the BYSETPOS attribute on RRULEs is not taken into account when we call getOccurrencesBetween.  The calendar at the above URL is supposed to contain a single event repeating on the last weekday (MO,TU,WE,TH,FR) of each month.  Instead, we show it as repeating on the last 5 weekdays of the month.

Updated

11 years ago
Blocks: 298366

Updated

11 years ago
No longer blocks: 298366
*** Bug 332293 has been marked as a duplicate of this bug. ***
(Reporter)

Updated

11 years ago
Keywords: dataloss
taking this one.
Assignee: base → michael.buettner
assigned.
Status: NEW → ASSIGNED

Comment 4

11 years ago
This seems like fairly basic dataloss; setting as blocking 0.3.
Flags: blocking0.3+
Is anything out there really uysing bysetpos?
libical doesn't support it, so it would be quite hard to fix this.
Whiteboard: [needs patch]
(In reply to comment #5)
> libical doesn't support it, so it would be quite hard to fix this.
as far as i can tell libical supports this rule (http://lxr.mozilla.org/seamonkey/source/calendar/libical/src/libical/icalrecur.h#161).
and it is true that for the calendar at the given URL shows the recurring event on the last 5 weekdays of a month. i didn't had much time to further dive into what went wrong inside libical, but i'll do this within the next days.

what really concerns me is the fact that even if we display such rules correctly in the views we run into dataloss issues in case we edit such items with the existing item dialog as we're not correctly roundtrip foreign complex recurrence rules. but this is not related to this specific problem here, but i wanted to mention it anyway.
(In reply to comment #5)
> libical doesn't support it, so it would be quite hard to fix this.
as far as i can tell libical supports this rule (http://lxr.mozilla.org/seamonkey/source/calendar/libical/src/libical/icalrecur.h#161).
and it is true that for the calendar at the given URL shows the recurring event on the last 5 weekdays of a month. i didn't had much time to further dive into what went wrong inside libical, but i'll do this within the next days.

what really concerns me is the fact that even if we display such rules correctly in the views we run into dataloss issues in case we edit such items with the existing item dialog as we're not correctly roundtrip foreign complex recurrence rules. but this is not related to this specific problem here, but i wanted to mention it anyway.
urgs, you're right, BYSETPOS is indeed not supported by libical. the rule is parsed correctly into the internal 'by_set_pos'-array but is silently ignored during icalrecur_iterator_next(). to complicate matters it's not trivial to track the negative parts of the 'by_set_pos'-array, how do we know the ordinal (within the scope of a rule) each element the iterator calculates has? hm, i'll try to come up with a solution, but any additional help is appreciated ;-)

Comment 9

11 years ago
Having dataloss around a construct which might be regularly used to track paydays seems scary.  

We need to get some more brainstorming around how this can be fixed (thus the [needs input]) annotation.

According to an old IETF proceeding found by Google, Outlook uses (used?) this at one point <http://www3.ietf.org/proceedings/00dec/00dec-11.htm>.  It would be a helpful QA exercise to see if Outlook (or iCal) uses this construct today, and to research using search engines to try and get a better handle on how frequently this is used.

Leaving as blocking until we have more data, since given the current knowledge, this seems pretty scary to ship without.
Keywords: qawanted
Whiteboard: [needs patch] → [needs patch][needs input dmose mvl ctalbert][needs testing]

Comment 10

11 years ago
Outlook 2003 (Version 11.0) does still use BYSETPOS. You can find some calendars online by searching google. I also found this post, talking about Outlook using BYSETPOS more often than it should:
http://phpicalendar.net/forums/archive/index.php/t-1591.html

Comment 11

11 years ago
Evolution also uses BYSETPOS for FREQ=MONTHLY.

Here's an example of Evolution specifying the third thursday of every second month:
FREQ=MONTHLY;INTERVAL=2;BYDAY=TH;BYSETPOS=3

Updated

11 years ago
Keywords: qawanted
Whiteboard: [needs patch][needs input dmose mvl ctalbert][needs testing] → [needs patch][needs input dmose mvl ctalbert]
While I agree that we really need to support this, unless someone can pull a gleaming mystical patch out of a hat right now, or unless I'm grossly overestimating the difficulty of this fix, I think we're too late in the release cycle to make this happen. Since this needs to be added at the libical layer, and doesn't already exist, I feel that even if we had it, it would be far too scary to take without some serious baking time.
(In reply to comment #12)
I totally agree with this. I would feel far more comfortable to provide a clean patch for this issue within the next weeks, but after the 0.3 release. It's definately non-trivial to add this rule to libical, so we should probably go ahead without it.
Sold.  Setting blocking0.3-.
Flags: blocking0.3+ → blocking0.3-
Keywords: relnote
Whiteboard: [needs patch][needs input dmose mvl ctalbert] → [needs patch][cal relnote]

Comment 15

11 years ago
(In reply to comment #8)
I've dug this bug back up and have been looking at a solution for it.

Unfortunately, everything I am thinking of involves some substantial refactoring. The problem here, as Michael noted, is the negative part of the bysetpos array. The only way to filter those is to know how many total occurrences a given recurrence pattern will generate, per the FREQ setting of the recurrence. 

As I'm seeing it, in order to support both positive and negative BYSETPOS values, we will need to perform the BYSETPOS filtering on the entire array of generated occurrences for the recurrence over a given FREQ. This means that we would have to use libical to generate the entire array of recurrences rather than calling icalrecur_iterator_next ourselves in GetNextOccurrence and GetOccurrences (http://lxr.mozilla.org/seamonkey/source/calendar/base/src/calRecurrenceRule.cpp#368 and #419).

libical already has such a method: icalrecur_expand_recurrence (http://lxr.mozilla.org/seamonkey/source/calendar/libical/src/libical/icalrecur.c#2426)
However, this method is not sufficient for our needs. You would want to call it with count equaling the number of possible occurrences per the given FREQ setting so that you could then have a set of occurrences on which to make the BYSETPOS determination. Getting that information might involve calculating recurrences that are outside the given recurrence range specified in GetOccurrences/GetNextOccurrence. Furthermore, this method returns its values in UTC time, and that would probably cause calIDateTime to convert each occurrence in the array back into the user's timezone.

I think a better solution would be to create a new method that takes the specified range, iterates from start to end of that range by stepping over the FREQ, and creates an array after determining the BYSETPOS restriction. The array it would return would be an array of icaltimetype structs, which could then be easily converted to a calIDateTime array in GetOccurrences.

Some pseudocode to help explain the libical algorithm we need:
k=0
freqcheck = 0
do {
   next = getNextOccurrence()
   i=0
   while ((next != ical_is_null_time) && (next < (STARTRANGE + FREQ)) {   
       testArray[i] = next;
       next = getNextOccurrence()
       i++
   }
   // checked our first FREQ period
   freqcheck++   

   // Now get the elements from the set of all occurrences in FREQ
   // that were specified by the BYSETPOS attribute in the ICS
   for each j in BysetposValues {
        finalResult[k] = filterBysetpos(BysetposValues[j], testArray);
        k++
   }   
}while(STARTRANGE + (FREQ * freqcheck) < ENDRANGE) 
  
return finalResult

The major advantage of this is that it does not change the existing libical recurrence calculation code. Because of that, we could limit the use of this BYSETPOS recurrence calculation to situations when there is a BYSETPOS attribute specified on the RRULE. 

For RRULES without BYSETPOS, calIRecurrenceRule::GetNextOccurrence and GetOccurrences could continue to function the way that they do today. Of course, it would be more consistent to change these methods so that they always call the "Bysetpos recurrence calculation" method, and simply streamline that method for situations when there is no BYSETPOS attribute specified. In those situations, I could envision it working in the exact same manner as the GetNextOccurrence and GetOccurrences methods.

Comments encouraged!

If this sounds like a plausible solution, I'd be happy to create a patch.

Comment 16

11 years ago
Created attachment 246754 [details]
Simple positive BYSETPOS example from Outlook

Comment 17

11 years ago
Created attachment 246755 [details]
A simple negative BYSETPOS example from Outlook
Clint, as far as I can tell this indeed sounds like a plausible solution. At least I can't find anything wrong with this approach and it has many advantages. Sorry for my late reply :-(

Comment 19

10 years ago
Comment: ScheduleWorld also uses +/- BYSETPOS in the same way Outlook and Evolution do. For full interop BYSETPOS needs to be done. Please implement this.

Updated

10 years ago
Whiteboard: [needs patch][cal relnote] → [needs patch]
Flags: blocking-calendar0.7?
Duplicate of this bug: 173026

Updated

10 years ago
Assignee: michael.buettner → nobody
Status: ASSIGNED → NEW
Blocks: 390620
Flags: blocking-calendar0.7? → wanted-calendar0.8?
Version: Trunk → unspecified
Setting wanted0.8- as the main Calendar developers will not devote any time to
this in the 0.8 timeframe. Patches are, of course, always welcome.
Flags: wanted-calendar0.8? → wanted-calendar0.8-

Updated

9 years ago
Flags: wanted-calendar0.8- → wanted-calendar0.9+

Updated

9 years ago
Flags: wanted-calendar0.9+ → wanted-calendar1.0+
Lack of proper support for the BYSETPOS rule part prevents Lightning/Sunbird
from supporting the RRULE generated by Microsoft Outlook for the following recurrence pattern:

- Monthly the first Monday
- Monthly the first weekday
- Monthly the first weekend day
- Monthly the last day of the month
- Yearly the first Monday of April
- Yearly the first weekday of April
- Yearly the first weekend day of April
- Yearly the first day of April

See: iCalendar Recurrence Rules Interoperability Matrix
http://www.calconnect.org/tests/iCalendar-RRULE-Interop/iCalendar-RRULE-Interop-Matrix.html

Updated

8 years ago
Duplicate of this bug: 479775
From libical sourceforge.net project:

Revision: 927
Author: kecsup
Date: 13:16:21, Dienstag, 3. März 2009
Message:
Upstream patch (Maemo/Nokia): Add BYSETPOS support for recurrence rules
----
Modified : /trunk/libical/src/libical/icalrecur.c

Comment 25

8 years ago
Subird 1.9.1.3567: BYSETPOS is still not working :-(

with todays nightlybuild "sunbird-1.0pre.en-US.win32.installer.exe" dated on "October 7th 2009" BYSETPOS is still not working :-(

my current workaround:

after exporting from msoutlook2007 

i have to do replacements in the ics file like 
replace "BYDAY=TH;BYSETPOS=2" with "BYDAY=2TH" 
replace "BYDAY=MO;BYSETPOS=-1" with "BYDAY=-1MO" 

after that the recurrences are fine with sunbird.

msoutlook2007 can interpret this format, too.
unfortunately msoutlook2007 converts them back to BYSETPOS :-(

Updated

7 years ago
Duplicate of this bug: 560366

Comment 27

7 years ago
The behavior reported in Bug 560366 (event intended to occur monthly occurs daily) also occurs when the intended repeat period is weekly.

Comment 28

7 years ago
(In reply to comment #27)
> The behavior reported in Bug 560366 (event intended to occur monthly occurs
> daily) also occurs when the intended repeat period is weekly.

Whatever its number, this bug has a "Major" effect on me because much of my work life involves events that recur ndays/week for about a quarter of the year or for nmonths on the same relative day (3d Thursday) etc.

Comment 29

6 years ago
RRULE:BYSETPOS=1;FREQ=MONTHLY;BYDAY=TH

happens only on the first Thursday of the month, but Lightning 1.0b2 displays it for every Thursday.

Updated

6 years ago
Duplicate of this bug: 644742

Comment 31

6 years ago
Will this be solved by bug #637064?

Updated

6 years ago
Duplicate of this bug: 584581

Comment 33

6 years ago
I'll need it to because:

"example, a bysetpos of -1 if combined with a MONTHLY frequency, and a byweekday of (MO, TU, WE, TH, FR), will result in the last work day of every month. "

  http://niemeyer.net/python-dateutil#head-cf004ee9a75592797e076752b2a889c10f445418
You need to log in before you can comment on or make changes to this bug.