Bug 1613534 - If a message body is specified, do not insert a paragraph at the start. r=mkmelin
authorGeoff Lankow <geoff@darktrojan.net>
Wed, 04 Mar 2020 22:19:50 +1300
changeset 37511 42930fd343fe9fdcee6000d6f7be316d42b55d17
parent 37510 381b9496d886a8f8a217a0e7fd2ed727a71e3f97
child 37512 58a1cb2582f5f286cd9139d1aeb30544009f81c2
push id2566
push userclokep@gmail.com
push dateMon, 09 Mar 2020 19:20:31 +0000
treeherdercomm-beta@a352facfa0a4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmkmelin
bugs1613534
Bug 1613534 - If a message body is specified, do not insert a paragraph at the start. r=mkmelin
mail/components/compose/content/MsgComposeCommands.js
mail/components/extensions/test/browser/browser_ext_compose_begin.js
mail/components/extensions/test/browser/browser_ext_compose_details.js
--- a/mail/components/compose/content/MsgComposeCommands.js
+++ b/mail/components/compose/content/MsgComposeCommands.js
@@ -510,27 +510,33 @@ var stateListener = {
     );
     let insertParagraph = gMsgCompose.composeHTML && useParagraph;
 
     let mailBody = getBrowser().contentDocument.querySelector("body");
     if (insertParagraph && gBodyFromArgs) {
       // Check for "empty" body before allowing paragraph to be inserted.
       // Non-empty bodies in a new message can occur when clicking on a
       // mailto link or when using the command line option -compose.
-      // An "empty" body can be one of these two cases:
+      // An "empty" body can be one of these three cases:
       // 1) <br> and nothing follows (no next sibling)
       // 2) <div/pre class="moz-signature">
+      // 3) No elements, just text
       // Note that <br><div/pre class="moz-signature"> doesn't happen in
       // paragraph mode.
+      let firstChild = mailBody.firstChild;
       let firstElementChild = mailBody.firstElementChild;
-      if (
-        (firstElementChild.nodeName != "BR" ||
-          firstElementChild.nextElementSibling) &&
-        !isSignature(firstElementChild)
-      ) {
+      if (firstElementChild) {
+        if (
+          (firstElementChild.nodeName != "BR" ||
+            firstElementChild.nextElementSibling) &&
+          !isSignature(firstElementChild)
+        ) {
+          insertParagraph = false;
+        }
+      } else if (firstChild && firstChild.nodeType == Node.TEXT_NODE) {
         insertParagraph = false;
       }
     }
 
     // Control insertion of line breaks.
     if (insertParagraph) {
       let editor = GetCurrentEditor();
       editor.enableUndo(false);
@@ -3033,16 +3039,17 @@ function ComposeStartup(aParams) {
   gBodyFromArgs = false;
 
   if (aParams) {
     params = aParams;
   } else if (window.arguments && window.arguments[0]) {
     try {
       if (window.arguments[0] instanceof Ci.nsIMsgComposeParams) {
         params = window.arguments[0];
+        gBodyFromArgs = params.composeFields && params.composeFields.body;
       } else {
         params = handleMailtoArgs(window.arguments[0]);
       }
     } catch (ex) {
       dump("ERROR with parameters: " + ex + "\n");
     }
 
     // if still no dice, try and see if the params is an old fashioned list of string attributes
--- a/mail/components/extensions/test/browser/browser_ext_compose_begin.js
+++ b/mail/components/extensions/test/browser/browser_ext_compose_begin.js
@@ -178,17 +178,18 @@ add_task(async function testBody() {
             browser.windows[eventName].removeListener(listener);
             resolve(window);
           };
           browser.windows[eventName].addListener(listener);
         });
       }
 
       let emptyHTML = "<body>\n<p><br>\n</p>\n";
-
+      let plainTextBodyTag =
+        '<body style="font-family: -moz-fixed; white-space: pre-wrap; width: 72ch;">';
       let tests = [
         {
           // No arguments.
           expected: {
             isHTML: true,
             htmlIncludes: emptyHTML,
             plainTextIs: "\n",
           },
@@ -225,38 +226,38 @@ add_task(async function testBody() {
           arguments: { plainTextBody: "", isPlainText: true },
           expected: { isHTML: false, plainTextIs: "" },
         },
         {
           // Non-empty HTML.
           arguments: { body: "<p>I'm an HTML message!</p>" },
           expected: {
             isHTML: true,
-            htmlIncludes: emptyHTML + "<p>I'm an HTML message!</p>",
-            plainTextIs: "\nI'm an HTML message!",
+            htmlIncludes: "<body>\n<p>I'm an HTML message!</p>\n</body>",
+            plainTextIs: "I'm an HTML message!",
           },
         },
         {
           // Non-empty plain text.
           arguments: { plainTextBody: "I'm a plain text message!" },
           expected: {
             isHTML: true,
-            htmlIncludes: emptyHTML + "I'm a plain text message!<",
-            plainTextIs: "\nI'm a plain text message!",
+            htmlIncludes: "<body>I'm a plain text message!</body>",
+            plainTextIs: "I'm a plain text message!",
           },
         },
         {
           // Non-empty plain text and isPlainText.
           arguments: {
             plainTextBody: "I'm a plain text message!",
             isPlainText: true,
           },
           expected: {
             isHTML: false,
-            htmlIncludes: ">I'm a plain text message!<",
+            htmlIncludes: plainTextBodyTag + "I'm a plain text message!</body>",
             plainTextIs: "I'm a plain text message!",
           },
         },
         {
           // HTML and plain text. Invalid.
           arguments: { body: "", plainTextBody: "" },
           throws: true,
         },
@@ -304,26 +305,27 @@ add_task(async function testBody() {
     },
   });
 
   extension.onMessage("checkBody", async expected => {
     let composeWindows = [...Services.wm.getEnumerator("msgcompose")];
     is(composeWindows.length, 1);
     await new Promise(resolve => composeWindows[0].setTimeout(resolve));
 
-    is(composeWindows[0].IsHTMLEditor(), expected.isHTML, "isHTML");
+    is(composeWindows[0].IsHTMLEditor(), expected.isHTML, "composition mode");
 
     let editor = composeWindows[0].GetCurrentEditor();
     let actualHTML = editor.outputToString("text/html", 0);
     let actualPlainText = editor.outputToString("text/plain", 0);
     if ("htmlIncludes" in expected) {
-      ok(actualHTML.includes(expected.htmlIncludes), "html");
+      info(actualHTML);
+      ok(actualHTML.includes(expected.htmlIncludes), "HTML content is correct");
     }
     if ("plainTextIs" in expected) {
-      is(actualPlainText, expected.plainTextIs, "plainText");
+      is(actualPlainText, expected.plainTextIs, "plainText content is correct");
     }
 
     extension.sendMessage();
   });
 
   await extension.startup();
   await extension.awaitFinish("finished");
   await extension.unload();
--- a/mail/components/extensions/test/browser/browser_ext_compose_details.js
+++ b/mail/components/extensions/test/browser/browser_ext_compose_details.js
@@ -291,54 +291,59 @@ add_task(async function testBody() {
   let extension = ExtensionTestUtils.loadExtension({
     background: async () => {
       let windows = await browser.windows.getAll({
         populate: true,
         windowTypes: ["messageCompose"],
       });
       let [htmlTabId, plainTextTabId] = windows.map(w => w.tabs[0].id);
 
+      let plainTextBodyTag =
+        '<body style="font-family: -moz-fixed; white-space: pre-wrap; width: 72ch;">';
+
       // Get details, HTML message.
 
       let htmlDetails = await browser.compose.getComposeDetails(htmlTabId);
       browser.test.log(JSON.stringify(htmlDetails));
       browser.test.assertTrue(!htmlDetails.isPlainText);
       browser.test.assertTrue(
-        htmlDetails.body.includes(">This is some <i>HTML</i> text.<")
+        htmlDetails.body.includes("<p>This is some <i>HTML</i> text.</p>")
       );
       browser.test.assertEq(
-        "\nThis is some HTML text.",
+        "This is some HTML text.",
         htmlDetails.plainTextBody
       );
 
       // Set details, HTML message.
 
       await browser.compose.setComposeDetails(htmlTabId, {
         body: htmlDetails.body.replace("<i>HTML</i>", "<code>HTML</code>"),
       });
       htmlDetails = await browser.compose.getComposeDetails(htmlTabId);
       browser.test.log(JSON.stringify(htmlDetails));
       browser.test.assertTrue(!htmlDetails.isPlainText);
       browser.test.assertTrue(
-        htmlDetails.body.includes(">This is some <code>HTML</code> text.<")
+        htmlDetails.body.includes("<p>This is some <code>HTML</code> text.</p>")
       );
       browser.test.assertTrue(
         "This is some HTML text.",
         htmlDetails.plainTextBody
       );
 
       // Get details, plain text message.
 
       let plainTextDetails = await browser.compose.getComposeDetails(
         plainTextTabId
       );
       browser.test.log(JSON.stringify(plainTextDetails));
       browser.test.assertTrue(plainTextDetails.isPlainText);
       browser.test.assertTrue(
-        plainTextDetails.body.includes(">This is some plain text.<")
+        plainTextDetails.body.includes(
+          plainTextBodyTag + "This is some plain text.</body>"
+        )
       );
       browser.test.assertEq(
         "This is some plain text.",
         plainTextDetails.plainTextBody
       );
 
       // Set details, plain text message.
 
@@ -348,17 +353,18 @@ add_task(async function testBody() {
       });
       plainTextDetails = await browser.compose.getComposeDetails(
         plainTextTabId
       );
       browser.test.log(JSON.stringify(plainTextDetails));
       browser.test.assertTrue(plainTextDetails.isPlainText);
       browser.test.assertTrue(
         plainTextDetails.body.includes(
-          ">This is some plain text.<br>Indeed, it is plain.<"
+          plainTextBodyTag +
+            "This is some plain text.<br>Indeed, it is plain.</body>"
         )
       );
       browser.test.assertEq(
         "This is some plain text.\nIndeed, it is plain.",
         plainTextDetails.plainTextBody
       );
 
       // Some things that should fail.