1266797 - Wrong first occurrence for MONTHLY rule with BYMONTHDAY greater than the last day of the month

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect)

Description

[Decathlon]

This is reproducible with both libical and ical.js

Steps to reproduce:

- create an event on any day of a month with less than 31 (or 30) days;
- select Repeat: "Custom" -> "Monthly" -> In the daypicker "Recur on day(s)" set "31" and deselect the
  day of the start date;
- save and close;

--> the event starts on the day of the start date, but misses the first recurrence on the 31 of the next
    month, instead it is moved on an early day of the month.
    For example starting in February, the first occurrence is the the 3th (or the 2nd) of March instead
    of the 31st of March.

Comment 1

[Decathlon]

Attachment: patch-icaljs-libical-tests-v1.patch

The issue is caused by the assignment of BYMONTHDAYs negative or greater than the last day of the month to the date "last.day", along with the automatic normalization in ical.js.

I opened a pull request for ical.js on GitHub:
https://github.com/mozilla-comm/ical.js/pull/232

This patch includes ical.js part and tests for this bug and bug 386516 as well because ical.js still had that bug and an explicit test was missing. I also added a few test that should be in the tests list.

Comment 2

[Philipp Preißer-Kewisch [:Fallen][☁️📆]]

Sorry for the delay on this, but I'm going to have to take a closer look at this one. Hope to get to that soon! Just wanted to let you know I haven't forgotten!

Comment 3

[Decathlon]

Comment on attachment 8749385 [details] [diff] [review]
patch-icaljs-libical-tests-v1.patch

Removing the review request because it needs to take a deeper look to wrong formatted rules apart from those ones "standard" identified by the parser.

Comment 4

[Martin Schröder [:mschroeder]]

Has this also been fixed with the ical.js changes in bug 1789362?

Comment 5

[Geoff Lankow (:darktrojan)]

For negative values yes, otherwise no. Now that I'm aware of it I think I could fix it in a fairly similar way, so I'll leave the NI open.

Comment 6

[Geoff Lankow (:darktrojan)]

Attachment: Bug 1266797 - Fix recurrence iteration where there is a BYMONTHDAY rule greater than the number of days in the start month. r=#thunderbird-reviewers

https://github.com/kewisch/ical.js/pull/531

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/56e2951ec642
Fix recurrence iteration where there is a BYMONTHDAY rule greater than the number of days in the start month. r=#thunderbird-reviewers,mkmelin

Comment 8

[Rob Lemley [:rjl]]

This needs a follow-up patch to mention the pull request in comment 6 in the header referring to the upstream source.

Comment 9

[Rob Lemley [:rjl]]

Attachment: Bug 1266797 - Follow-up: Include PR information in reference to upstream source. r=darktrojan

Depends on D158614

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/be2044f63ec8
Follow-up: Include PR information in reference to upstream source. r=darktrojan DONTBUILD

Comment 11

[Geoff Lankow (:darktrojan)]

Comment on attachment 9296927 [details]
Bug 1266797 - Fix recurrence iteration where there is a BYMONTHDAY rule greater than the number of days in the start month. r=#thunderbird-reviewers

[Approval Request Comment]
Regression caused by (bug #): ical.js
User impact if declined: events recur on the wrong day, that's quite bad
Testing completed (on c-c, etc.): in 107b1
Risk to taking this patch (and alternatives if risky): low

Comment 12

[Wayne Mery (:wsmwk)]

Comment on attachment 9296927 [details]
Bug 1266797 - Fix recurrence iteration where there is a BYMONTHDAY rule greater than the number of days in the start month. r=#thunderbird-reviewers

[Triage Comment]
Approved for esr102

Comment 13

[Rob Lemley [:rjl]]

Thunderbird 102.4.1:
https://hg.mozilla.org/releases/comm-esr102/rev/ed7fd759d02c
https://hg.mozilla.org/releases/comm-esr102/rev/b4c44609dc43