From 0c9a7d8c03109cc6a29c9ea398dec0bd152ad4b0 Mon Sep 17 00:00:00 2001 From: Ping Chen Date: Thu, 17 Jun 2021 13:06:35 +0300 Subject: [PATCH] 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 | 6 +- calendar/base/src/CalAlarmService.jsm | 78 ++++++++++++------------- calendar/test/unit/test_alarmservice.js | 24 ++++---- 3 files changed, 55 insertions(+), 53 deletions(-) diff --git a/calendar/base/calendar.js b/calendar/base/calendar.js index 0590f958b0..c991799f4d 100644 --- a/calendar/base/calendar.js +++ b/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 diff --git a/calendar/base/src/CalAlarmService.jsm b/calendar/base/src/CalAlarmService.jsm index c7f9cc7575..ab71e1e622 100644 --- a/calendar/base/src/CalAlarmService.jsm +++ b/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); }, /** diff --git a/calendar/test/unit/test_alarmservice.js b/calendar/test/unit/test_alarmservice.js index 8b1fefaeca..b066d97ecc 100644 --- a/calendar/test/unit/test_alarmservice.js +++ b/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(