Bug 1617022 - Make compose.onBeforeSend listener actually change composed message. r=mkmelin a=wsmwk
authorGeoff Lankow <geoff@darktrojan.net>
Fri, 21 Feb 2020 12:54:47 +0200
changeset 37299 8c986f1b128d6aa100d9dd94df1a7b31d3cce3b3
parent 37298 f6cd636ceb740dde5d108647c5a68da77725a710
child 37300 757c02a7bdfc54ec27088ff712385b748f312b1d
push id2559
push userthunderbird@calypsoblue.org
push dateFri, 21 Feb 2020 22:57:54 +0000
treeherdercomm-beta@8c986f1b128d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmkmelin, wsmwk
bugs1617022
Bug 1617022 - Make compose.onBeforeSend listener actually change composed message. r=mkmelin a=wsmwk
mail/components/extensions/parent/ext-compose.js
mail/components/extensions/test/browser/browser_ext_compose_onBeforeSend.js
mail/components/extensions/test/browser/head.js
--- a/mail/components/extensions/parent/ext-compose.js
+++ b/mail/components/extensions/parent/ext-compose.js
@@ -97,34 +97,34 @@ async function openComposeWindow(related
       }
     }
   }
 
   params.composeFields = composeFields;
   MailServices.compose.OpenComposeWindowWithParams(null, params);
 }
 
-function getComposeState(composeWindow) {
+function getComposeDetails(composeWindow) {
   let composeFields = composeWindow.GetComposeDetails();
 
   let details = {
     to: composeFields.splitRecipients(composeFields.to, false),
     cc: composeFields.splitRecipients(composeFields.cc, false),
     bcc: composeFields.splitRecipients(composeFields.bcc, false),
     replyTo: composeFields.splitRecipients(composeFields.replyTo, false),
     followupTo: composeFields.splitRecipients(composeFields.followupTo, false),
     newsgroups: composeFields.newsgroups
       ? composeFields.newsgroups.split(",")
       : [],
     subject: composeFields.subject,
   };
   return details;
 }
 
-async function setComposeState(composeWindow, details) {
+async function setComposeDetails(composeWindow, details) {
   for (let field of ["to", "cc", "bcc", "replyTo", "followupTo"]) {
     if (field in details) {
       details[field] = await parseComposeRecipientList(details[field]);
     }
   }
   if (Array.isArray(details.newsgroups)) {
     details.newsgroups = details.newsgroups.join(",");
   }
@@ -158,29 +158,30 @@ var composeEventTracker = new (class ext
     let msgType = event.detail;
     let composeWindow = event.target;
 
     composeWindow.ToggleWindowLock(true);
 
     let results = await this.emit(
       "compose-before-send",
       tabTracker.getId(composeWindow),
-      getComposeState(composeWindow)
+      getComposeDetails(composeWindow)
     );
     if (results) {
       for (let result of results) {
         if (!result) {
           continue;
         }
         if (result.cancel) {
           composeWindow.ToggleWindowLock(false);
           return;
         }
         if (result.details) {
-          setComposeState(composeWindow, result.details);
+          await setComposeDetails(composeWindow, result.details);
+          composeWindow.GetComposeDetails();
         }
       }
     }
 
     composeWindow.ToggleWindowLock(false);
     composeWindow.CompleteGenericSendMessage(msgType);
   }
 })();
@@ -241,18 +242,18 @@ this.compose = class extends ExtensionAP
             Services.prefs.getIntPref("mail.forward_message_mode") == 0
           ) {
             type = Ci.nsIMsgCompType.ForwardAsAttachment;
           }
           openComposeWindow(messageId, type, composeParams);
         },
         getComposeDetails(tabId) {
           let tab = getComposeTab(tabId);
-          return getComposeState(tab.nativeTab);
+          return getComposeDetails(tab.nativeTab);
         },
         setComposeDetails(tabId, details) {
           let tab = getComposeTab(tabId);
-          return setComposeState(tab.nativeTab, details);
+          return setComposeDetails(tab.nativeTab, details);
         },
       },
     };
   }
 };
--- a/mail/components/extensions/test/browser/browser_ext_compose_onBeforeSend.js
+++ b/mail/components/extensions/test/browser/browser_ext_compose_onBeforeSend.js
@@ -1,20 +1,20 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, you can obtain one at http://mozilla.org/MPL/2.0/. */
 
 var { ExtensionSupport } = ChromeUtils.import(
   "resource:///modules/ExtensionSupport.jsm"
 );
 
-add_task(async () => {
-  let account = createAccount();
-  addIdentity(account);
+let account = createAccount();
+addIdentity(account);
 
+add_task(async function testCancel() {
   let extension = ExtensionTestUtils.loadExtension({
     background: async () => {
       function waitForEvent(eventName) {
         return new Promise(resolve => {
           let listener = window => {
             browser.windows[eventName].removeListener(listener);
             resolve(window);
           };
@@ -124,81 +124,47 @@ add_task(async () => {
       browser.compose.onBeforeSend.addListener(listener4);
       await beginSend(false, true);
       browser.test.assertEq(tab.id, listener4.tabId, "listener4 was fired");
       listener4.resolve({ cancel: true });
       await checkIfSent(false, false);
       browser.compose.onBeforeSend.removeListener(listener4);
       await beginSend(true); // Removing the listener worked.
 
-      // Add a listener that changes the subject. Sending should continue and
-      // the subject should change. This is largely the same code as tested in
-      // browser_ext_compose_details.js, so just test that the change happens.
-
-      // First check that the original headers are unmodified.
-      await checkWindow({ to: ["test@test.invalid"], subject: "Test" });
-
-      let listener5 = (tabId, details) => {
-        listener5.tabId = tabId;
-        listener5.details = details;
-        return {
-          details: {
-            subject: "Changed by listener5",
-          },
-        };
-      };
-      browser.compose.onBeforeSend.addListener(listener5);
-      await beginSend(true);
-      browser.test.assertEq(tab.id, listener5.tabId, "listener5 was fired");
-      browser.test.assertEq(1, listener5.details.to.length);
-      browser.test.assertEq(
-        "test@test.invalid",
-        listener5.details.to[0],
-        "listener5 recipient correct"
-      );
-      browser.test.assertEq(
-        "Test",
-        listener5.details.subject,
-        "listener5 subject correct"
-      );
-      // First check that the subject has changed but recipient hasn't.
-      await checkWindow({
-        to: ["test@test.invalid"],
-        subject: "Changed by listener5",
-      });
-      browser.compose.onBeforeSend.removeListener(listener5);
-
       // Clean up.
 
       let removedWindowPromise = waitForEvent("onRemoved");
       browser.windows.remove(createdWindow.id);
       await removedWindowPromise;
 
       browser.test.notifyPass("finished");
     },
     manifest: { permissions: ["accountsRead", "compose", "messagesRead"] },
   });
 
   // We can't allow sending to actually happen, this is a test. For every
   // compose window that opens, replace the function which does the actual
   // sending with one that only records when it has been called.
   let didTryToSendMessage = false;
+  let windowListenerRemoved = false;
   ExtensionSupport.registerWindowListener("xpcshell", {
     chromeURLs: [
       "chrome://messenger/content/messengercompose/messengercompose.xhtml",
     ],
     onLoadWindow(window) {
       window.CompleteGenericSendMessage = function(msgType) {
         didTryToSendMessage = true;
       };
     },
   });
-  registerCleanupFunction(() =>
-    ExtensionSupport.unregisterWindowListener("xpcshell")
-  );
+  registerCleanupFunction(() => {
+    if (!windowListenerRemoved) {
+      ExtensionSupport.unregisterWindowListener("xpcshell");
+    }
+  });
 
   extension.onMessage("beginSend", async () => {
     let composeWindows = [...Services.wm.getEnumerator("msgcompose")];
     is(composeWindows.length, 1);
 
     composeWindows[0].GenericSendMessage(Ci.nsIMsgCompDeliverMode.Now);
     extension.sendMessage();
   });
@@ -219,9 +185,182 @@ add_task(async () => {
   extension.onMessage("checkWindow", async expected => {
     await checkComposeHeaders(expected);
     extension.sendMessage();
   });
 
   await extension.startup();
   await extension.awaitFinish("finished");
   await extension.unload();
+
+  ExtensionSupport.unregisterWindowListener("xpcshell");
+  windowListenerRemoved = true;
 });
+
+add_task(async function testChangeDetails() {
+  let extension = ExtensionTestUtils.loadExtension({
+    background: async () => {
+      function waitForEvent(eventName) {
+        return new Promise(resolve => {
+          let listener = window => {
+            browser.windows[eventName].removeListener(listener);
+            resolve(window);
+          };
+          browser.windows[eventName].addListener(listener);
+        });
+      }
+
+      async function beginSend() {
+        await new Promise(resolve => {
+          browser.test.onMessage.addListener(function listener() {
+            browser.test.onMessage.removeListener(listener);
+            resolve();
+          });
+          browser.test.sendMessage("beginSend");
+        });
+      }
+
+      function checkWindow(expected) {
+        return new Promise(resolve => {
+          browser.test.onMessage.addListener(function listener() {
+            browser.test.onMessage.removeListener(listener);
+            resolve();
+          });
+          browser.test.sendMessage("checkWindow", expected);
+        });
+      }
+
+      // Add a listener that changes the headers. Sending should continue and
+      // the headers should change. This is largely the same code as tested in
+      // browser_ext_compose_details.js, so just test that the changes happen.
+
+      let createdWindowPromise = waitForEvent("onCreated");
+      await browser.compose.beginNew({
+        to: ["test@test.invalid"],
+        subject: "Test",
+      });
+      let createdWindow = await createdWindowPromise;
+      browser.test.assertEq("messageCompose", createdWindow.type);
+
+      await checkWindow({ to: ["test@test.invalid"], subject: "Test" });
+
+      let [tab] = await browser.tabs.query({ windowId: createdWindow.id });
+
+      let listener5 = (tabId, details) => {
+        listener5.tabId = tabId;
+        listener5.details = details;
+        return {
+          details: {
+            to: ["to@test5.invalid"],
+            cc: ["cc@test5.invalid"],
+            subject: "Changed by listener5",
+          },
+        };
+      };
+      browser.compose.onBeforeSend.addListener(listener5);
+      await beginSend();
+      browser.test.assertEq(tab.id, listener5.tabId, "listener5 was fired");
+      browser.test.assertEq(1, listener5.details.to.length);
+      browser.test.assertEq(
+        "test@test.invalid",
+        listener5.details.to[0],
+        "listener5 recipient correct"
+      );
+      browser.test.assertEq(
+        "Test",
+        listener5.details.subject,
+        "listener5 subject correct"
+      );
+      browser.compose.onBeforeSend.removeListener(listener5);
+
+      // Do the same thing, but this time with a Promise.
+
+      createdWindowPromise = waitForEvent("onCreated");
+      await browser.compose.beginNew({
+        to: ["test@test.invalid"],
+        subject: "Test",
+      });
+      createdWindow = await createdWindowPromise;
+      browser.test.assertEq("messageCompose", createdWindow.type);
+
+      await checkWindow({ to: ["test@test.invalid"], subject: "Test" });
+
+      [tab] = await browser.tabs.query({ windowId: createdWindow.id });
+
+      let listener6 = (tabId, details) => {
+        listener6.tabId = tabId;
+        listener6.details = details;
+        return new Promise(resolve => {
+          listener6.resolve = resolve;
+        });
+      };
+      browser.compose.onBeforeSend.addListener(listener6);
+      await beginSend();
+      browser.test.assertEq(tab.id, listener6.tabId, "listener6 was fired");
+      browser.test.assertEq(1, listener6.details.to.length);
+      browser.test.assertEq(
+        "test@test.invalid",
+        listener6.details.to[0],
+        "listener6 recipient correct"
+      );
+      browser.test.assertEq(
+        "Test",
+        listener6.details.subject,
+        "listener6 subject correct"
+      );
+      listener6.resolve({
+        details: {
+          to: ["to@test6.invalid"],
+          cc: ["cc@test6.invalid"],
+          subject: "Changed by listener6",
+        },
+      });
+      browser.compose.onBeforeSend.removeListener(listener6);
+
+      browser.test.notifyPass("finished");
+    },
+    manifest: { permissions: ["accountsRead", "compose", "messagesRead"] },
+  });
+
+  extension.onMessage("beginSend", async () => {
+    let composeWindows = [...Services.wm.getEnumerator("msgcompose")];
+    is(composeWindows.length, 1);
+
+    composeWindows[0].GenericSendMessage(Ci.nsIMsgCompDeliverMode.Later);
+    extension.sendMessage();
+  });
+
+  extension.onMessage("checkWindow", async expected => {
+    await checkComposeHeaders(expected);
+    extension.sendMessage();
+  });
+
+  await extension.startup();
+  await extension.awaitFinish("finished");
+  await extension.unload();
+
+  let outbox = account.incomingServer.rootFolder.getChildNamed("outbox");
+  let outboxMessages = outbox.messages;
+  ok(outboxMessages.hasMoreElements());
+  let sentMessage5 = outboxMessages.getNext();
+  is(sentMessage5.subject, "Changed by listener5", "subject was changed");
+  is(sentMessage5.recipients, "to@test5.invalid", "to was changed");
+  is(sentMessage5.ccList, "cc@test5.invalid", "cc was changed");
+
+  ok(outboxMessages.hasMoreElements());
+  let sentMessage6 = outboxMessages.getNext();
+  is(sentMessage6.subject, "Changed by listener6", "subject was changed");
+  is(sentMessage6.recipients, "to@test6.invalid", "to was changed");
+  is(sentMessage6.ccList, "cc@test6.invalid", "cc was changed");
+
+  ok(!outboxMessages.hasMoreElements());
+
+  await new Promise(resolve => {
+    outbox.deleteMessages(
+      toXPCOMArray([sentMessage5, sentMessage6], Ci.nsIMutableArray),
+      null,
+      true,
+      false,
+      { OnStopCopy: resolve },
+      false
+    );
+  });
+});
--- a/mail/components/extensions/test/browser/head.js
+++ b/mail/components/extensions/test/browser/head.js
@@ -1,16 +1,19 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 var { MailServices } = ChromeUtils.import(
   "resource:///modules/MailServices.jsm"
 );
+var { toXPCOMArray } = ChromeUtils.import(
+  "resource:///modules/iteratorUtils.jsm"
+);
 
 // There are shutdown issues for which multiple rejections are left uncaught.
 // This bug should be fixed, but for the moment this directory is whitelisted.
 //
 // NOTE: Entire directory whitelisting should be kept to a minimum. Normally you
 //       should use "expectUncaughtRejection" to flag individual failures.
 const { PromiseTestUtils } = ChromeUtils.import(
   "resource://testing-common/PromiseTestUtils.jsm"