Bug 1515959 - Fix reminder dialog to respect default duration, and not overwrite existing reminders when changing the UI; r=Fallen
authorGeoff Lankow <geoff@darktrojan.net>
Tue, 01 Jan 2019 11:21:57 +1300
changeset 34091 40bfc93a1dcc6baec73082c6c39999b787023efb
parent 34090 5753ad5a95c90434a62b55f3a12a3a2612e0d364
child 34092 178da49fe8c830b5bf6e8d7a504feb18b17aadeb
push id389
push userclokep@gmail.com
push dateMon, 18 Mar 2019 19:01:53 +0000
reviewersFallen
bugs1515959
Bug 1515959 - Fix reminder dialog to respect default duration, and not overwrite existing reminders when changing the UI; r=Fallen
calendar/base/content/dialogs/calendar-event-dialog-reminder.js
calendar/test/mozmill/testAlarmDefaultValue.js
--- a/calendar/base/content/dialogs/calendar-event-dialog-reminder.js
+++ b/calendar/base/content/dialogs/calendar-event-dialog-reminder.js
@@ -7,16 +7,17 @@
  */
 
 ChromeUtils.import("resource://gre/modules/PluralForm.jsm");
 ChromeUtils.import("resource://gre/modules/Preferences.jsm");
 
 var { cal } = ChromeUtils.import("resource://calendar/modules/calUtils.jsm", null);
 
 var allowedActionsMap = {};
+var suppressListUpdate = false;
 
 /**
  * Sets up the reminder dialog.
  */
 function onLoad() {
     let calendar = window.arguments[0].calendar;
 
     // Make sure the origin menulist uses the right labels, depending on if the
@@ -249,70 +250,76 @@ function onReminderSelected() {
     let absDate = document.getElementById("reminder-absolute-date");
     let actionType = document.getElementById("reminder-actions-menulist");
     let relationType = document.getElementById("reminder-relation-radiogroup");
 
     let listbox = document.getElementById("reminder-listbox");
     let listitem = listbox.selectedItem;
 
     if (listitem) {
-        let reminder = listitem.reminder;
+        try {
+            suppressListUpdate = true;
+            let reminder = listitem.reminder;
 
-        // Action
-        actionType.value = reminder.action;
+            // Action
+            actionType.value = reminder.action;
 
-        // Absolute/relative things
-        if (reminder.related == Components.interfaces.calIAlarm.ALARM_RELATED_ABSOLUTE) {
-            relationType.value = "absolute";
+            // Absolute/relative things
+            if (reminder.related == Components.interfaces.calIAlarm.ALARM_RELATED_ABSOLUTE) {
+                relationType.value = "absolute";
 
-            // Date
-            absDate.value = cal.dtz.dateTimeToJsDate(reminder.alarmDate || cal.dtz.getDefaultStartDate());
-        } else {
-            relationType.value = "relative";
+                // Date
+                absDate.value = cal.dtz.dateTimeToJsDate(reminder.alarmDate || cal.dtz.getDefaultStartDate());
+            } else {
+                relationType.value = "relative";
 
-            // Unit and length
-            let alarmlen = Math.abs(reminder.offset.inSeconds / 60);
-            if (alarmlen % 1440 == 0) {
-                unit.value = "days";
-                length.value = alarmlen / 1440;
-            } else if (alarmlen % 60 == 0) {
-                unit.value = "hours";
-                length.value = alarmlen / 60;
-            } else {
-                unit.value = "minutes";
-                length.value = alarmlen;
-            }
+                // Unit and length
+                let alarmlen = Math.abs(reminder.offset.inSeconds / 60);
+                if (alarmlen % 1440 == 0) {
+                    unit.value = "days";
+                    length.value = alarmlen / 1440;
+                } else if (alarmlen % 60 == 0) {
+                    unit.value = "hours";
+                    length.value = alarmlen / 60;
+                } else {
+                    unit.value = "minutes";
+                    length.value = alarmlen;
+                }
 
-            // Relation
-            let relation = (reminder.offset.isNegative ? "before" : "after");
+                // Relation
+                let relation = (reminder.offset.isNegative ? "before" : "after");
 
-            // Origin
-            let origin;
-            if (reminder.related == Components.interfaces.calIAlarm.ALARM_RELATED_START) {
-                origin = "START";
-            } else if (reminder.related == Components.interfaces.calIAlarm.ALARM_RELATED_END) {
-                origin = "END";
+                // Origin
+                let origin;
+                if (reminder.related == Components.interfaces.calIAlarm.ALARM_RELATED_START) {
+                    origin = "START";
+                } else if (reminder.related == Components.interfaces.calIAlarm.ALARM_RELATED_END) {
+                    origin = "END";
+                }
+
+                relationOrigin.value = [relation, origin].join("-");
             }
-
-            relationOrigin.value = [relation, origin].join("-");
+        } finally {
+            suppressListUpdate = false;
         }
     } else {
         // no list item is selected, disable elements
         setupRadioEnabledState(true);
     }
 }
 
 /**
  * Handler function to be called when an aspect of the alarm has been changed
  * using the dialog controls.
  *
  * @param event         The DOM event caused by the change.
  */
 function updateReminder(event) {
-    if (event.explicitOriginalTarget.localName == "richlistitem" ||
+    if (suppressListUpdate ||
+        event.explicitOriginalTarget.localName == "richlistitem" ||
         event.explicitOriginalTarget.parentNode.localName == "richlistitem" ||
         event.explicitOriginalTarget.id == "reminder-remove-button" ||
         !document.commandDispatcher.focusedElement) {
         // Do not set things if the select came from selecting or removing an
         // alarm from the list, or from setting when the dialog initially loaded.
         // XXX Quite fragile hack since radio/radiogroup doesn't have the
         // supressOnSelect stuff.
         return;
@@ -381,22 +388,29 @@ function getItemBundleStringName(aPrefix
  * new reminder item.
  */
 function onNewReminder() {
     let itemType = (cal.item.isEvent(window.arguments[0].item) ? "event" : "todo");
     let listbox = document.getElementById("reminder-listbox");
 
     let reminder = cal.createAlarm();
     let alarmlen = Preferences.get("calendar.alarms." + itemType + "alarmlen", 15);
+    let alarmunit = Preferences.get("calendar.alarms." + itemType + "alarmunit", 15);
 
     // Default is a relative DISPLAY alarm, |alarmlen| minutes before the event.
     // If DISPLAY is not supported by the provider, then pick the provider's
     // first alarm type.
     let offset = cal.createDuration();
-    offset.minutes = alarmlen;
+    if (alarmunit == "days") {
+        offset.days = alarmlen;
+    } else if (alarmunit == "hours") {
+        offset.hours = alarmlen;
+    } else {
+        offset.minutes = alarmlen;
+    }
     offset.normalize();
     offset.isNegative = true;
     reminder.related = reminder.ALARM_RELATED_START;
     reminder.offset = offset;
     if ("DISPLAY" in allowedActionsMap) {
         reminder.action = "DISPLAY";
     } else {
         let calendar = window.arguments[0].calendar;
--- a/calendar/test/mozmill/testAlarmDefaultValue.js
+++ b/calendar/test/mozmill/testAlarmDefaultValue.js
@@ -12,28 +12,32 @@ var MODULE_REQUIRES = ["calendar-utils",
 
 var { cal } = ChromeUtils.import("resource://calendar/modules/calUtils.jsm", null);
 ChromeUtils.import("resource://gre/modules/PluralForm.jsm");
 ChromeUtils.import("resource://gre/modules/Preferences.jsm");
 
 const DEFVALUE = 43;
 
 var helpersForController, invokeEventDialog, openLightningPrefs, menulistSelect;
+var plan_for_modal_dialog, wait_for_modal_dialog;
 
 function setupModule(module) {
     controller = mozmill.getMail3PaneController();
     ({
         helpersForController,
         invokeEventDialog,
         openLightningPrefs,
         menulistSelect
     } = collector.getModule("calendar-utils"));
     collector.getModule("calendar-utils").setupModule(controller);
     Object.assign(module, helpersForController(controller));
 
+    ({ plan_for_modal_dialog, wait_for_modal_dialog } =
+        collector.getModule("window-helpers"));
+
     collector.getModule("content-tab-helpers").installInto(module);
 }
 
 function testDefaultAlarms() {
     let localeUnitString = cal.l10n.getCalString("unitDays");
     let unitString = PluralForm.get(DEFVALUE, localeUnitString).replace("#1", DEFVALUE);
     let alarmString = (...args) => cal.l10n.getString("calendar-alarms", ...args);
     let originStringEvent = alarmString("reminderCustomOriginBeginBeforeEvent");
@@ -41,50 +45,58 @@ function testDefaultAlarms() {
     let expectedEventReminder = alarmString("reminderCustomTitle", [unitString, originStringEvent]);
     let expectedTaskReminder = alarmString("reminderCustomTitle", [unitString, originStringTask]);
 
     let detailPath = `
         //*[@id="reminder-details"]/*[local-name()="label" and (not(@hidden) or @hidden="false")]
     `;
 
     // Configure the lightning preferences.
-    openLightningPrefs(handlePrefDialog, controller);
+    openLightningPrefs(handlePrefTab, controller);
 
     // Create New Event.
     controller.click(eid("newMsgButton-calendar-menuitem"));
 
     // Set up the event dialog controller.
     invokeEventDialog(controller, null, (event, iframe) => {
         let { xpath: eventpath, eid: eventid } = helpersForController(event);
 
         // Check if the "custom" item was selected.
         event.assertDOMProperty(eventid("item-alarm"), "value", "custom");
         let reminderDetailsVisible = eventpath(detailPath);
         event.assertDOMProperty(reminderDetailsVisible, "value", expectedEventReminder);
 
+        plan_for_modal_dialog("Calendar:EventDialog:Reminder", handleReminderDialog);
+        event.click(reminderDetailsVisible);
+        wait_for_modal_dialog("Calendar:EventDialog:Reminder");
+
         // Close the event dialog.
         event.window.close();
     });
 
     // Create New Task.
     controller.click(eid("newMsgButton-task-menuitem"));
     invokeEventDialog(controller, null, (task, iframe) => {
         let { xpath: taskpath, eid: taskid } = helpersForController(task);
 
         // Check if the "custom" item was selected.
         task.assertDOMProperty(taskid("item-alarm"), "value", "custom");
         let reminderDetailsVisible = taskpath(detailPath);
         task.assertDOMProperty(reminderDetailsVisible, "value", expectedTaskReminder);
 
+        plan_for_modal_dialog("Calendar:EventDialog:Reminder", handleReminderDialog);
+        task.click(reminderDetailsVisible);
+        wait_for_modal_dialog("Calendar:EventDialog:Reminder");
+
         // Close the task dialog.
         task.window.close();
     });
 }
 
-function handlePrefDialog(tab) {
+function handlePrefTab(tab) {
     // Click on the alarms tab.
     content_tab_e(tab, "calPreferencesTabAlarms").click();
 
     // Turn on alarms for events and tasks.
     menulistSelect(content_tab_eid(tab, "eventdefalarm"), "1", controller);
     menulistSelect(content_tab_eid(tab, "tododefalarm"), "1", controller);
 
     // Selects "days" as a unit.
@@ -94,11 +106,36 @@ function handlePrefDialog(tab) {
     // Sets default alarm length for events to DEFVALUE.
     let eventdefalarmlen = content_tab_eid(tab, "eventdefalarmlen");
     replaceText(eventdefalarmlen, DEFVALUE.toString());
 
     let tododefalarmlen = content_tab_eid(tab, "tododefalarmlen");
     replaceText(tododefalarmlen, DEFVALUE.toString());
 }
 
+function handleReminderDialog(reminders) {
+    let { eid: remindersid, replaceText } = helpersForController(reminders);
+
+    let listbox = remindersid("reminder-listbox");
+    let listboxElement = remindersid("reminder-listbox").getNode();
+    reminders.waitFor(() => listboxElement.selectedCount == 1);
+    reminders.assertJS(listboxElement.selectedItem.reminder.offset.days == DEFVALUE);
+
+    reminders.click(remindersid("reminder-new-button"));
+    reminders.waitFor(() => listboxElement.itemCount == 2);
+    reminders.assertJS(listboxElement.selectedCount == 1);
+    reminders.assertJS(listboxElement.selectedItem.reminder.offset.days == DEFVALUE);
+
+    replaceText(remindersid("reminder-length"), "20");
+    reminders.assertJS(listboxElement.selectedItem.reminder.offset.days == 20);
+
+    reminders.click(listbox);
+    reminders.keypress(listbox, "VK_UP", {});
+    reminders.waitFor(() => listboxElement.selectedIndex == 0);
+
+    reminders.assertJS(listboxElement.selectedItem.reminder.offset.days == DEFVALUE);
+
+    reminders.window.close();
+}
+
 function teardownTest(module) {
     Preferences.resetBranch("calendar.alarms");
 }