Bug 1593280 - Fix icons for message and URL attachments, remove setting defunct image attribute for attachment items. r=aleca a=jorgk
authorJorg K <jorgk@jorgk.com>
Wed, 06 Nov 2019 09:43:26 +0100
changeset 36382 a67b00fab3979f9442ff788d2c4c531499beb57d
parent 36381 29842ec3bacabba2a72aa3795c86b428369f0210
child 36383 db4ef1385165f1b87148d1bc467a14d0e1c543cb
push id2521
push usermozilla@jorgk.com
push dateWed, 06 Nov 2019 09:31:04 +0000
treeherdercomm-beta@308f336efa3e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaleca, jorgk
bugs1593280
Bug 1593280 - Fix icons for message and URL attachments, remove setting defunct image attribute for attachment items. r=aleca a=jorgk
mail/base/content/mailWidgets.js
mail/components/compose/content/MsgComposeCommands.js
--- a/mail/base/content/mailWidgets.js
+++ b/mail/base/content/mailWidgets.js
@@ -1594,27 +1594,37 @@
       sizeLabel.setAttribute("value", size);
 
       // Pick out some nice icons (small and large) for the attachment
       if (attachment.contentType == "text/x-moz-deleted") {
         let base = "chrome://messenger/skin/icons/";
         item.setAttribute("image16", base + "attachment-deleted.png");
         item.setAttribute("image32", base + "attachment-deleted-large.png");
       } else {
+        let iconName = attachment.name;
+        if (iconName.toLowerCase().endsWith(".eml")) {
+          // Discard message names derived from crazy subject headers.
+          iconName = "message.eml";
+        } else if (attachment.url) {
+          let url = Services.io.newURI(attachment.url);
+          if (url instanceof Ci.nsIURL && url.fileName && !url.schemeIs("file")) {
+            iconName = url.fileName;
+          }
+        }
         item.setAttribute(
           "image16",
           "moz-icon://" +
-            attachment.name +
+            iconName +
             "?size=16&contentType=" +
             attachment.contentType
         );
         item.setAttribute(
           "image32",
           "moz-icon://" +
-            attachment.name +
+            iconName +
             "?size=32&contentType=" +
             attachment.contentType
         );
       }
 
       let imageSize = this.sizes[this.getAttribute("view")] || 16;
       item.setAttribute("imagesize", imageSize);
       item.setAttribute("context", this.getAttribute("itemcontext"));
--- a/mail/components/compose/content/MsgComposeCommands.js
+++ b/mail/components/compose/content/MsgComposeCommands.js
@@ -1612,17 +1612,16 @@ function addAttachCloudMenuItems(aParent
     let iconURL = account.iconURL;
     item.cloudFileAccount = account;
     item.setAttribute(
       "label",
       cloudFileAccounts.getDisplayName(account) + "\u2026"
     );
     if (iconURL) {
       item.setAttribute("class", `${item.localName}-iconic`);
-      item.setAttribute("image", iconURL);
     }
     aParentMenu.appendChild(item);
 
     let previousUploads = account.getPreviousUploads();
     for (let upload of previousUploads) {
       let file = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
       file.initWithPath(upload.path);
 
@@ -1631,17 +1630,16 @@ function addAttachCloudMenuItems(aParent
         continue;
       }
 
       let fileItem = document.createXULElement("menuitem");
       fileItem.cloudFileUpload = upload;
       fileItem.cloudFileAccount = account;
       fileItem.setAttribute("label", file.leafName);
       fileItem.setAttribute("class", "menuitem-iconic");
-      fileItem.setAttribute("image", "moz-icon://" + file.leafName);
       aParentMenu.appendChild(fileItem);
     }
   }
 }
 
 function addConvertCloudMenuItems(aParentMenu, aAfterNodeId, aRadioGroup) {
   let attachment = document.getElementById("attachmentBucket").selectedItem;
   let afterNode = document.getElementById(aAfterNodeId);
@@ -1666,17 +1664,16 @@ function addConvertCloudMenuItems(aParen
 
     if (
       attachment.cloudFileAccount &&
       attachment.cloudFileAccount.accountKey == account.accountKey
     ) {
       item.setAttribute("checked", "true");
     } else if (iconURL) {
       item.setAttribute("class", "menu-iconic");
-      item.setAttribute("image", iconURL);
     }
 
     aParentMenu.appendChild(item);
   }
 }
 
 async function uploadCloudAttachment(attachment, file, cloudFileAccount) {
   // Notify the UI that we're starting the upload process: disable send commands
@@ -1684,17 +1681,16 @@ async function uploadCloudAttachment(att
   attachment.sendViaCloud = true;
   gNumUploadingAttachments++;
   updateSendCommands(true);
 
   let bucket = document.getElementById("attachmentBucket");
   let attachmentItem = bucket.findItemForAttachment(attachment);
   if (attachmentItem) {
     let itemIcon = attachmentItem.querySelector(".attachmentcell-icon");
-    attachmentItem.image = "chrome://global/skin/icons/loading.png";
     itemIcon.setAttribute("src", "chrome://global/skin/icons/loading.png");
     attachmentItem.setAttribute(
       "tooltiptext",
       getComposeBundle().getFormattedString("cloudFileUploadingTooltip", [
         cloudFileAccounts.getDisplayName(cloudFileAccount),
       ])
     );
     attachmentItem.uploading = true;
@@ -1726,22 +1722,20 @@ async function uploadCloudAttachment(att
         ])
       );
       attachmentItem.uploading = false;
 
       // Set the icon for the attachment.
       let iconURL = cloudFileAccount.iconURL;
       let itemIcon = attachmentItem.querySelector(".attachmentcell-icon");
       if (iconURL) {
-        attachmentItem.image = iconURL;
         itemIcon.setAttribute("src", iconURL);
       } else {
         // Should we use a generic "cloud" icon here? Or an overlay icon?
         // I think the provider should provide an icon, end of story.
-        attachmentItem.image = null;
         itemIcon.setAttribute("src", "");
       }
 
       attachmentItem.dispatchEvent(
         new CustomEvent("attachment-uploaded", {
           bubbles: true,
           cancelable: true,
         })
@@ -1824,17 +1818,16 @@ async function uploadCloudAttachment(att
         )
       ) {
         openLinkExternally(url);
       }
     }
 
     if (attachmentItem) {
       // Remove the loading throbber.
-      attachmentItem.image = null;
       attachmentItem.setAttribute("tooltiptext", attachmentItem.attachment.url);
       attachmentItem.uploading = false;
       attachmentItem.attachment.sendViaCloud = false;
       delete attachmentItem.cloudFileAccount;
 
       let event = document.createEvent("CustomEvent");
       event.initEvent("attachment-upload-failed", true, true, statusCode);
       attachmentItem.dispatchEvent(event);
@@ -1894,22 +1887,20 @@ function attachToCloudRepeat(upload, acc
     item.setAttribute("name", upload.leafName);
     itemLabel.setAttribute("value", upload.leafName);
     item.cloudFileUpload = {
       ...upload,
       repeat: true,
     };
     let iconURL = account.iconURL;
     if (iconURL) {
-      item.image = iconURL;
       itemIcon.setAttribute("src", iconURL);
     } else {
       // Should we use a generic "cloud" icon here? Or an overlay icon?
       // I think the provider should provide an icon, end of story.
-      item.image = null;
       itemIcon.setAttribute("src", "");
     }
     item.dispatchEvent(
       new CustomEvent("attachment-uploaded", {
         bubbles: true,
         cancelable: true,
       })
     );
@@ -2066,17 +2057,16 @@ function convertListItemsToRegularAttach
     }
 
     item.attachment.url = item.originalUrl;
     item.setAttribute("tooltiptext", item.attachment.url);
     item.attachment.sendViaCloud = false;
 
     delete item.cloudFileAccount;
     delete item.originalUrl;
-    item.image = null;
 
     convertedAttachments.appendElement(item.attachment);
   }
 
   dispatchAttachmentBucketEvent("attachments-converted", convertedAttachments);
 
   // We leave the content location in for the notifications because
   // it may be needed to identify the attachment. But clear it out now.
@@ -4984,42 +4974,20 @@ function AddAttachments(aAttachments, aC
     item.addEventListener("command", OpenSelectedAttachment);
 
     if (attachment.sendViaCloud) {
       try {
         let account = cloudFileAccounts.getAccount(
           attachment.cloudFileAccountKey
         );
         item.cloudFileAccount = account;
-        item.image = account.iconURL;
         item.originalUrl = attachment.url;
       } catch (ex) {
         dump(ex);
       }
-
-      // For local file urls, we are better off using the full file url because
-      // moz-icon will actually resolve the file url and get the right icon from
-      // the file url. All other urls, we should try to extract the file name from
-      // them. This fixes issues where an icon wasn't showing up if you dragged a
-      // web url that had a query or reference string after the file name and for
-      // mailnews urls where the filename is hidden in the url as a &filename=
-      // part.
-    } else if (
-      /^mailbox-message:|^imap-message:|^news-message:/i.test(attachment.url)
-    ) {
-      // We're attaching a message. Pretend that is comes from a file,
-      // so we get the icon that matches .eml files.
-      item.image = "moz-icon://message.eml";
-    } else {
-      let url = Services.io.newURI(attachment.url);
-      if (url instanceof Ci.nsIURL && url.fileName && !url.schemeIs("file")) {
-        item.image = "moz-icon://" + url.fileName;
-      } else {
-        item.image = "moz-icon:" + attachment.url;
-      }
     }
 
     items.push(item);
 
     if (aCallback) {
       aCallback(item);
     }
   }