elementary / calendar

Desktop calendar app designed for elementary OS
https://elementary.io
GNU General Public License v3.0
130 stars 39 forks source link

Editing event automatically enables Saturday repeat #759

Closed Stephen-Hamilton-C closed 2 years ago

Stephen-Hamilton-C commented 2 years ago

What Happened?

I edited an event I already made that repeats on Monday and Wednesday. I made a change to the first event, and when looking at the Repeat tab, I found Saturday was selected, even though it was never selected in the first place.

Steps to Reproduce

  1. Create a repeating event on any weekday that isn't Saturday
  2. Edit the repeating event
  3. Look in the Repeat tab to find Sat is selected

Expected Behavior

I should have to explicitly click on Sat for it to repeat on Saturday, rather than this happen automatically

OS Version

Other Linux

Software Version

Latest release (I have run all updates)

Log Output

No response

Hardware Info

OS: Fedora Linux 36 (Workstation Edition) Kernel: 5.18.16-200.fc36.x86_64 DE: GNOME 42.4 WM: Mutter CPU: 11th Gen Intel i7-1165G7 (8) @ 4.700GHz GPU: Intel TigerLake-LP GT2 [Iris Xe Graphics] Memory: 16GB

Stephen-Hamilton-C commented 2 years ago

Ah, something I should add - this is with a CalDAV calendar on Nextcloud

mcclurgm commented 2 years ago

Hmm, that's interesting. Can confirm: I tested two weekly events on Thursday and Friday, and Calendar selected Saturday. I'll see if I can have a look. Both of these are on Google, so it appears it's not an issue with Nextcloud.

mcclurgm commented 2 years ago

I think I found the cause. It seems to be in libical's handling of recurrence rules. In my case, the recurrence rule was formatted as FREQ=WEEKLY;INTERVAL=2. When the day isn't specified, it's supposed to be set by the DTSTART property. But somehow, in all the cases I've found, libical returns the weekday of recurrence as Saturday. I'm unclear whether this is a bug or not. It seems to me that this should be ICAL_NO_WEEKDAY, but since there seems to be absolutely no documentation of libical recurrence rules, I don't know.

This also brings up a further question: for cases where the weekday isn't specified in the component, do we want to display a weekday selected, or select none for something like "automatic"? They seem the same on the surface, but there is a subtle difference: if you change the start date, no specified day will shift accordingly; if we force it to specify a day then all recurrences will not shift to follow the start date.

mcclurgm commented 2 years ago

Ok, after digging I've found the culprit. ICal.Recurrence.get_by_day_array only actually fills as many days as there are set in the reccurrence rule (i.e. the BYDAY iCalendar property). Any remaining elements are filled with the value ICAL_RECURRENCE_ARRAY_MAX, which is hard coded to 32639. When you decode that, it returns a Saturday, even though it is actually a sentinel value.

I guess this is a case of "if it's not documented, it's broken." 😬 It seems like all we need to do is add a check for this sentinel value wherever we use these arrays.