Bug 1497434 - Support calendar notification times relative to both start and end. r=darktrojan

Redefine the calendar.notifications.times pref:
- use `END:` to mean relative to the end
- use comma as separator of entries

An example is `PT2M,END:-PT3M`.

Differential Revision: https://phabricator.services.mozilla.com/D117391

--HG--
extra : amend_source : 45d82d2baec1ffe2f7f75390bb26f7934bf69614

calendar/base/calendar.js

@@ -51,9 +51,9 @@ pref("calendar.alarms.todoalarmunit", "minutes");
 pref("calendar.alarms.loglevel", "Warn");
 
 // The default timeouts to show notifications for calendar items. The value
-// should be in the form of "PT1D PT1H PT0M", which means to show three
-// notifications: one at 1 day before, one at 1 hour before, one at 0 minute
-// before the event/task starts.
+// should be in the form of "-PT1D,PT2M,END:-PT3M", which means to show
+// notifications at: 1 day before the start, 2 minutes after the start, 3
+// minutes before the end.
 pref("calendar.notifications.times", "");
 
 // open invitations autorefresh settings

calendar/base/src/CalAlarmService.jsm

@@ -442,29 +442,6 @@ CalAlarmService.prototype = {
     this.removeNotificationForItem(aItem);
   },
 
-  /**
-   * Parse a .notifications.times pref value to an array of seconds. The pref
-   * value is expected to be in the form of "PT1D PT2H PT3M".
-   * @param {string} prefValue - The pref value to the parsed.
-   * @returns {number[]} An array of seconds.
-   */
-  parseNotificationTimeToSeconds(prefValue) {
-    return prefValue
-      .split(" ")
-      .map(entry => {
-        if (!entry.trim()) {
-          return null;
-        }
-        try {
-          return cal.createDuration(entry).inSeconds;
-        } catch (e) {
-          this._logger.error(`Failed to parse ${prefValue}`, e);
-          return null;
-        }
-      })
-      .filter(x => x != null);
-  },
-
   /**
    * Get the timeouts before notifications are fired for an item.
    * @param {calIItemBase} item - A calendar item instance.
@@ -478,24 +455,45 @@ CalAlarmService.prototype = {
     if (!cal.item.checkIfInRange(item, now, until)) {
       return [];
     }
-    let startDate;
-    if (item.isEvent) {
-      startDate = item.startDate;
-    } else {
-      startDate = item.entryDate || item.dueDate;
+    let startDate = item[cal.dtz.startDateProp(item)];
+    let endDate = item[cal.dtz.endDateProp(item)];
+    let timeouts = [];
+    // The calendar level notifications setting overrides the global setting.
+    let prefValue = (item.calendar.getProperty("notifications.times") || gNotificationsTimes).split(
+      ","
+    );
+    for (let entry of prefValue) {
+      entry = entry.trim();
+      if (!entry) {
+        continue;
+      }
+      let [tag, value] = entry.split(":");
+      if (!value) {
+        value = tag;
+        tag = "";
+      }
+      let duration;
+      try {
+        duration = cal.createDuration(value);
+      } catch (e) {
+        this._logger.error(`Failed to parse ${entry}`, e);
+        continue;
+      }
+      let fireDate;
+      if (tag == "END" && endDate) {
+        fireDate = endDate.clone();
+      } else if (startDate) {
+        fireDate = startDate.clone();
+      } else {
+        continue;
+      }
+      fireDate.addDuration(duration);
+      let timeout = fireDate.subtractDate(now).inSeconds * 1000;
+      if (timeout > 0) {
+        timeouts.push(timeout);
+      }
     }
-    return this.parseNotificationTimeToSeconds(
-      // The calendar level notifications setting overrides the global setting.
-      item.calendar.getProperty("notifications.times") || gNotificationsTimes
-    )
-      .map(seconds => {
-        let fireDate = startDate.clone();
-        fireDate.second -= seconds;
-        let timeout = fireDate.subtractDate(now).inSeconds * 1000;
-        return timeout > 0 ? timeout : null;
-      })
-      .filter(x => x != null)
-      .sort((x, y) => x - y);
+    return timeouts.sort((x, y) => x - y);
   },
 
   /**

calendar/test/unit/test_alarmservice.js

@@ -504,10 +504,10 @@ function test_modifyItems() {
  * @param {number[]} expected - Expected delays in seconds.
  */
 function matchTimers(timers, expected) {
-  let delays = timers.map(timer => timer.delay);
+  let delays = timers.map(timer => timer.delay / 1000);
   let matched = true;
   for (let i = 0; i < delays.length; i++) {
-    if (Math.abs(delays[i] - expected[i] * 1000) > 1000) {
+    if (Math.abs(delays[i] - expected[i]) > 1) {
       matched = false;
       break;
     }
@@ -534,7 +534,7 @@ function test_notificationTimers() {
     );
 
     // Set the pref to have one notifiaction.
-    Services.prefs.setCharPref("calendar.notifications.times", "PT1H");
+    Services.prefs.setCharPref("calendar.notifications.times", "-PT1H");
     oldItem = item.clone();
     date.hour += 1;
     item.startDate = date.clone();
@@ -544,24 +544,28 @@ function test_notificationTimers() {
     matchTimers(alarmObserver.service.mNotificationTimerMap[item.calendar.id][item.hashId], [3600]);
 
     // Set the pref to have three notifiactions.
-    Services.prefs.setCharPref("calendar.notifications.times", "PT5M PT0M PT30M");
+    Services.prefs.setCharPref("calendar.notifications.times", "END:PT2M,PT0M,END:-PT30M,-PT5M");
     oldItem = item.clone();
     date.hour -= 1;
     item.startDate = date.clone();
+    date.hour += 1;
+    item.endDate = date.clone();
     item.generation++;
     memory.modifyItem(item, oldItem, null);
-    // Should have three notification timers.
+    // Should have four notification timers.
     matchTimers(alarmObserver.service.mNotificationTimerMap[item.calendar.id][item.hashId], [
-      1800, // 30 minutes
       3300, // 55 minutes
       3600, // 60 minutes
+      5400, // 90 minutes, which is 30 minutes before the end (END:-PT30M)
+      7320, // 122 minutes, which is 2 minutes after the end (END:PT2M)
     ]);
 
     alarmObserver.service.removeFiredNotificationTimer(item);
-    // Should have two notification timers.
+    // Should have three notification timers.
     matchTimers(alarmObserver.service.mNotificationTimerMap[item.calendar.id][item.hashId], [
-      3300,
       3600,
+      5400,
+      7320,
     ]);
 
     memory.deleteItem(item, null);
@@ -586,7 +590,7 @@ function test_calendarLevelNotificationTimers() {
     if (!loaded) {
       loaded = true;
       // Set the global pref to have one notifiaction.
-      Services.prefs.setCharPref("calendar.notifications.times", "PT1H");
+      Services.prefs.setCharPref("calendar.notifications.times", "-PT1H");
 
       // Add an item.
       let date = cal.dtz.now();
@@ -599,7 +603,7 @@ function test_calendarLevelNotificationTimers() {
         3600,
       ]);
       // Set the calendar level pref to have two notification timers.
-      memory.setProperty("notifications.times", "PT5M PT0M");
+      memory.setProperty("notifications.times", "-PT5M,PT0M");
     }
 
     await TestUtils.waitForCondition(