1335447 - Event Dialog: error "rule.untilDate is null" when changing recurrence rule from a "custom" rule

Status: RESOLVED FIXED

(Calendar :: Dialogs, defect)

Description

[Decathlon]

Steps to reproduce:

- create a new event and set a "Custom" recurrence rule that will make appear the recurrence details string
  in the dialog (e.g. a custom monthly rule);
- leave the default until date to "no end date" in the Recurrence dialog;
- close the Recurrence dialog, now the Event dialog shows the string that describes the recurrence rule;
- change the recurrence rule from "Custom" to any other rule in the menulist;

--> the string remains in the dialog instead of being replaced by the datepicker that should show
    the until date.
    An error "rule.untilDate is null" appears in the Error Console.

Comment 1

[Decathlon]

Attachment: patch - v1

This is not a regression, it comes directly form the initial implementation of the datepicker for the until date.

Comment 2

[[:MakeMyDay]]

Comment on attachment 8832089 [details] [diff] [review]
patch - v1

Review of attachment 8832089 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks. r=me with the below nit considered.

::: calendar/lightning/content/lightning-item-iframe.js
@@ +2659,4 @@
>              if (aItemRepeatCall && repeatDeck.selectedIndex == 1) {
>                  if (!rule.isByCount || !rule.isFinite) {
>                      setElementValue("repeat-until-datepicker",
> +                                    rule.isFinite ? cal.dateTimeToJsDate(rule.untilDate.getInTimezone(cal.floating()))

Can you move rule.untilDate.getInTimezone(cal.floating()) to a separate variable and pass that to cal.dateTimeToJsDate() to make that line not that long?

Comment 3

[[:MakeMyDay]]

Comment on attachment 8832089 [details] [diff] [review]
patch - v1

Even if it's not a regression, this is a candidate for 5.4

Comment 4

[Decathlon]

Attachment: patch - v2

(In reply to [:MakeMyDay] from comment #2)
> Can you move rule.untilDate.getInTimezone(cal.floating()) to a separate
> variable and pass that to cal.dateTimeToJsDate() to make that line not that
> long?

I've also done it for another line below that is even longer.
Please, take a look if it's OK for you.

Comment 5

[[:MakeMyDay]]

Comment on attachment 8833731 [details] [diff] [review]
patch - v2

Review of attachment 8833731 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks.

Comment 6

[Jorg K (CEST = GMT+2)]

https://hg.mozilla.org/comm-central/rev/f4a143f3ced4dccd1479e61c3f523e85880a20ca

Comment 7

[Jorg K (CEST = GMT+2)]

Approval?

Comment 8

[Jorg K (CEST = GMT+2)]

Aurora (TB 53, Calendar 5.5):
https://hg.mozilla.org/releases/comm-aurora/rev/7658bb89511a454e6da6780e770ce2122c13ab04

Comment 9

[Jorg K (CEST = GMT+2)]

Beta (TB 52, Calendar 5.4):
https://hg.mozilla.org/releases/comm-beta/rev/bdb83c35964e602cbf5f604e2fc3ed3d93ffda1c