Include icons in xpi base path (fixes #197)

lib/rdf.js

@@ -42,9 +42,9 @@ function createRDF (manifest) {
     "em:version": manifest.version || "0.0.0",
     "em:name": manifest.title || manifest.name || "Untitled",
     "em:description": manifest.description || "",
-    "em:creator": formatAuthor(manifest.author),
-    "em:iconURL": manifest.icon || "icon.png",
-    "em:icon64URL": manifest.icon64 || "icon64.png"
+    "em:iconURL": manifest.icon,
+    "em:icon64URL": manifest.icon64,
+    "em:creator": formatAuthor(manifest.author)
   };
 
   if (manifest.homepage) {

lib/xpi.js

@@ -50,12 +50,21 @@ function xpi (manifest, options) {
       }
       return null;
     })
+    .then(function(){
+      return utils.checkIcons(options, manifest);
+    })
     .then(function() {
       return utils.createFallbacks(options);
     })
+    .then(function() {
+      return utils.copyIcons(options);
+    })
     .then(function() {
       return utils.createZip(options);
     })
+    .then(function() {
+      return utils.removeIcons(options);
+    })
     .then(function() {
       return utils.removeFallbacks(options);
     })

lib/xpi/utils.js

@@ -79,7 +79,7 @@ function createFallbacks (options) {
     return when.all([
       options.needsInstallRDF ? fs.writeFile(rdfPath, createRDF(manifest)) : when.resolve(),
       options.needsBootstrapJS ? fs.copy(bootstrapSrc, bsPath) : when.resolve(),
-    ])
+    ]);
   });
 }
 exports.createFallbacks = createFallbacks;
@@ -100,6 +100,61 @@ function removeFallbacks (options) {
 }
 exports.removeFallbacks = removeFallbacks;
 
+// Helper function to copy icons
+function copyIfNotExists (path, newPath) {
+  if (!path) {
+    return;
+  }
+  return fs.exists(newPath).then(function (exists) {
+    if (!exists) {
+      return when.promise(function (resolve) {
+          var reader = fs.createReadStream(path);
+          reader.pipe(fs.createWriteStream(newPath));
+          reader.on('end', resolve.bind(null, newPath));
+      });
+    }
+  });
+}
+
+// Check if icons should be included in the base path
+// instead of adding them to install.rdf
+function checkIcons (options, manifest) {
+  return when.all([
+    manifest.icon && fs.exists(manifest.icon),
+    manifest.icon64 && fs.exists(manifest.icon64)
+  ]).then(function (exists){
+    if (exists[0]) {
+      options.icon = manifest.icon;
+      delete manifest.icon;
+    }
+    if (exists[1]) {
+      options.icon64 = manifest.icon64;
+      delete manifest.icon64;
+    }
+  });
+}
+exports.checkIcons = checkIcons;
+
+function copyIcons (options) {
+  var dir = process.cwd();
+  return when.all([
+    copyIfNotExists(options.icon, join(dir, "icon.png")),
+    copyIfNotExists(options.icon64, join(dir, "icon64.png")),
+  ]).then(function (icons){
+    options.tempIcon = icons[0];
+    options.tempIcon64 = icons[1];
+  });
+}
+exports.copyIcons = copyIcons;
+
+function removeIcons (options) {
+  return when.all([
+    options.tempIcon ? fs.remove(options.tempIcon) : when.resolve(),
+    options.tempIcon64 ? fs.remove(options.tempIcon64) : when.resolve()
+  ]);
+}
+exports.removeIcons = removeIcons;
+
 function createZip (options) {
   var start = Date.now();
   var dir = process.cwd();

test/addons/icon-custom/index.js

@@ -0,0 +1 @@
+console.log("Custom icon paths");

test/addons/icon-custom/package.json

@@ -0,0 +1,11 @@
+{
+  "title": "Custom icon path",
+  "name": "icon-custom",
+  "version": "1.0.0",
+  "description": "",
+  "main": "index.js",
+  "author": "",
+  "license": "MIT",
+  "icon": "img/logo.png",
+  "icon64": "img/logo64.png"
+}

test/addons/icon-root/index.js

@@ -0,0 +1 @@
+console.log("Icon root");

test/addons/icon-root/package.json

@@ -0,0 +1,9 @@
+{
+  "title": "Icons in root",
+  "name": "icon-root",
+  "version": "1.0.0",
+  "description": "",
+  "main": "index.js",
+  "author": "",
+  "license": "MIT"
+}

test/unit/test.rdf.js

@@ -25,10 +25,7 @@ describe("lib/rdf", function () {
       expect(getData(xml, "em:unpack")).to.be.equal("false");
       expect(getData(xml, "em:type")).to.be.equal("2");
       expect(getData(xml, "em:name")).to.be.equal("Untitled");
-      expect(getData(xml, "em:iconURL")).to.be.equal("icon.png");
-      expect(getData(xml, "em:icon64URL")).to.be.equal("icon64.png");
-      expect(str.indexOf("homepageURL")).to.be.equal(-1);
-      ["description", "creator"].forEach(function (field) {
+      ["homepageURL", "description", "creator"].forEach(function (field) {
         expect(nodeEmpty(xml, "em:" + field)).to.be.equal(true);
       });
 
@@ -92,15 +89,21 @@ describe("lib/rdf", function () {
       var xml = createRDF({ name: "Marie Curie <mc@espci.fr>" });
       expect(xml.indexOf("Marie Curie &lt;mc@espci.fr&gt;")).to.be.not.equal(-1);
     });
-
-    it("iconURL uses `icon`", function () {
-      var xml = setupRDF({ icon: "megaman.png" });
-      expect(getData(xml, "em:iconURL")).to.be.equal("megaman.png");
+    it("iconURL uses `icon` for non-local icons", function () {
+      var xml = setupRDF({ icon: "chrome://addon/style/main.css" });
+      expect(getData(xml, "em:iconURL")).to.be.equal("chrome://addon/style/main.css");
     });
-
-    it("icon64URL uses `icon64`", function () {
-      var xml = setupRDF({ icon64: "megaman.png" });
-      expect(getData(xml, "em:icon64URL")).to.be.equal("megaman.png");
+    it("icon64URL uses `icon64` for non-local icons", function () {
+      var xml = setupRDF({ icon64: "chrome://addon/style/main.css" });
+      expect(getData(xml, "em:icon64URL")).to.be.equal("chrome://addon/style/main.css");
+    });
+    it("iconURL uses `icon` for urls", function () {
+      var xml = setupRDF({ icon: "https://example.com/icon.png" });
+      expect(getData(xml, "em:iconURL")).to.be.equal("https://example.com/icon.png");
+    });
+    it("icon64URL uses `icon64` for urls", function () {
+      var xml = setupRDF({ icon64: "https://example.com/icon.png" });
+      expect(getData(xml, "em:icon64URL")).to.be.equal("https://example.com/icon.png");
     });
 
     it("updateURL uses `updateURL`", function () {

test/unit/test.xpi.js

@@ -13,6 +13,8 @@ var xpi = require("../../lib/xpi");
 
 var simpleAddonPath = path.join(__dirname, "..", "addons", "simple-addon");
 var aomUnsupportedPath = path.join(__dirname, "..", "addons", "aom-unsupported");
+var customIconPath = path.join(__dirname, "..", "addons", "icon-custom");
+var rootIconPath = path.join(__dirname, "..", "addons", "icon-root");
 var extraFilesPath = path.join(__dirname, "..", "addons", "extra-files");
 var tmpOutputDir = path.join(__dirname, "../", "tmp");
 
@@ -233,4 +235,59 @@ describe("lib/xpi", function () {
     })
     .catch(done);
   });
+
+  it("Moves custom icon files to the xpi root", function (done) {
+    process.chdir(customIconPath);
+    var manifest = require(path.join(customIconPath, "package.json"));
+    xpi(manifest).then(function (xpiPath) {
+      return utils.unzipTo(xpiPath, tmpOutputDir);
+    })
+    .then(function () {
+      var files = fs.readdirSync(tmpOutputDir);
+      expect(files).to.contain("icon.png");
+      expect(files).to.contain("icon64.png");
+      return when.all([
+        fs.readFile(path.join(customIconPath, "img/logo.png")),
+        fs.readFile(path.join(tmpOutputDir, "icon.png")),
+        fs.readFile(path.join(customIconPath, "img/logo64.png")),
+        fs.readFile(path.join(tmpOutputDir, "icon64.png"))
+      ])
+    })
+    .then(function(icons){
+        expect(icons[0].toString()).to.be.equal(icons[1].toString());
+        expect(icons[2].toString()).to.be.equal(icons[3].toString());
+    })
+    .then(function(){
+      return fs.readFile(path.join(tmpOutputDir, "install.rdf"));
+    })
+    .then(function(data){
+      expect(data.toString().indexOf("logo.png")).to.equal(-1);
+      expect(data.toString().indexOf("logo64.png")).to.equal(-1);
+      done();
+    })
+    .catch(done);
+  });
+
+  it("Removes temporary icon files", function (done) {
+    process.chdir(customIconPath);
+    var manifest = require(path.join(customIconPath, "package.json"));
+    xpi(manifest).then(function (xpiPath) {
+      var files = fs.readdirSync(customIconPath);
+      expect(files).to.not.contain("icon.png");
+      expect(files).to.not.contain("icon64.png");
+    })
+    .then(done, done);
+  });
+
+  it("Does not remove non-temporary user icon files", function (done) {
+    process.chdir(rootIconPath);
+    var manifest = require(path.join(rootIconPath, "package.json"));
+    xpi(manifest).then(function (xpiPath) {
+      var files = fs.readdirSync(rootIconPath);
+      expect(files).to.contain("icon.png");
+      expect(files).to.contain("icon64.png");
+    })
+    .then(done, done);
+  });
+
 });