From 77a6af9e5b99ecea11d8edfc121b5822e4fef583 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sun, 25 Feb 2018 15:07:33 -0500 Subject: [PATCH] Fx60 compatibility: Fix snapshot filenames nsIURL doesn't seem to work anymore, so add Zotero.Utilities.parseURL(), which uses the `url` package from NPM and adds fileName, fileExtension, and fileBaseName. --- chrome/content/zotero/xpcom/attachments.js | 34 ++++++---------- chrome/content/zotero/xpcom/utilities.js | 15 +++++++ package-lock.json | 7 +--- package.json | 1 + scripts/config.js | 7 ++++ test/tests/utilitiesTest.js | 46 ++++++++++++++++++++++ 6 files changed, 83 insertions(+), 27 deletions(-) diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js index 68a3e1b5c..ad0d287d6 100644 --- a/chrome/content/zotero/xpcom/attachments.js +++ b/chrome/content/zotero/xpcom/attachments.js @@ -1131,53 +1131,43 @@ Zotero.Attachments = new function(){ function _getFileNameFromURL(url, contentType){ - var nsIURL = Components.classes["@mozilla.org/network/standard-url;1"] - .createInstance(Components.interfaces.nsIURL); - nsIURL.spec = url; + url = Zotero.Utilities.parseURL(url); - var ext = Zotero.MIME.getPrimaryExtension(contentType, nsIURL.fileExtension); + var fileBaseName = url.fileBaseName; + var fileExt = Zotero.MIME.getPrimaryExtension(contentType, url.fileExtension); - if (!nsIURL.fileName) { - var matches = nsIURL.directory.match(/\/([^\/]+)\/$/); + if (!fileBaseName) { + let matches = url.pathname.match(/\/([^\/]+)\/$/); // If no filename, use the last part of the path if there is one if (matches) { - nsIURL.fileName = matches[1]; + fileBaseName = matches[1]; } // Or just use the host else { - nsIURL.fileName = nsIURL.host; - var tld = nsIURL.fileExtension; + fileBaseName = url.hostname; } } - // If we found a better extension, use that - if (ext && (!nsIURL.fileExtension || nsIURL.fileExtension != ext)) { - nsIURL.fileExtension = ext; - } - - // If we replaced the TLD (which would've been interpreted as the extension), add it back - if (tld && tld != nsIURL.fileExtension) { - nsIURL.fileBaseName = nsIURL.fileBaseName + '.' + tld; - } - // Test unencoding fileBaseName try { - decodeURIComponent(nsIURL.fileBaseName); + decodeURIComponent(fileBaseName); } catch (e) { if (e.name == 'URIError') { // If we got a 'malformed URI sequence' while decoding, // use MD5 of fileBaseName - nsIURL.fileBaseName = Zotero.Utilities.Internal.md5(nsIURL.fileBaseName, false); + fileBaseName = Zotero.Utilities.Internal.md5(fileBaseName, false); } else { throw e; } } + var fileName = fileBaseName + (fileExt ? '.' + fileExt : ''); + // Pass unencoded name to getValidFileName() so that percent-encoded // characters aren't stripped to just numbers - return Zotero.File.getValidFileName(decodeURIComponent(nsIURL.fileName)); + return Zotero.File.getValidFileName(decodeURIComponent(fileName)); } diff --git a/chrome/content/zotero/xpcom/utilities.js b/chrome/content/zotero/xpcom/utilities.js index 22cd66710..4ed115ddf 100644 --- a/chrome/content/zotero/xpcom/utilities.js +++ b/chrome/content/zotero/xpcom/utilities.js @@ -1861,6 +1861,21 @@ Zotero.Utilities = { } }, + + parseURL: function (url) { + var parts = require('url').parse(url); + // fileName + parts.fileName = parts.pathname.split('/').pop(); + // fileExtension + var pos = parts.fileName.lastIndexOf('.'); + parts.fileExtension = pos == -1 ? '' : parts.fileName.substr(pos + 1); + // fileBaseName + parts.fileBaseName = parts.fileName + // filename up to the period before the file extension, if there is one + .substr(0, parts.fileName.length - (parts.fileExtension ? parts.fileExtension.length + 1 : 0)); + return parts; + }, + /** * Get the real target URL from an intermediate URL */ diff --git a/package-lock.json b/package-lock.json index 35e8d9866..932b4f1e5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3637,8 +3637,7 @@ "querystring": { "version": "0.2.0", "resolved": "https://registry.npmjs.org/querystring/-/querystring-0.2.0.tgz", - "integrity": "sha1-sgmEkgO7Jd+CDadW50cAWHhSFiA=", - "dev": true + "integrity": "sha1-sgmEkgO7Jd+CDadW50cAWHhSFiA=" }, "querystring-es3": { "version": "0.2.1", @@ -4553,7 +4552,6 @@ "version": "0.11.0", "resolved": "https://registry.npmjs.org/url/-/url-0.11.0.tgz", "integrity": "sha1-ODjpfPxgUh63PFJajlW/3Z4uKPE=", - "dev": true, "requires": { "punycode": "1.3.2", "querystring": "0.2.0" @@ -4562,8 +4560,7 @@ "punycode": { "version": "1.3.2", "resolved": "https://registry.npmjs.org/punycode/-/punycode-1.3.2.tgz", - "integrity": "sha1-llOgNvt8HuQjQvIyXM7v6jkmxI0=", - "dev": true + "integrity": "sha1-llOgNvt8HuQjQvIyXM7v6jkmxI0=" } } }, diff --git a/package.json b/package.json index 6c5bf8d5c..a9358f0a7 100644 --- a/package.json +++ b/package.json @@ -18,6 +18,7 @@ "bluebird": "^3.5.1", "react": "^15.6.2", "react-dom": "^15.6.2", + "url": "^0.11.0", "zotero-web-library": "^0.9.4-alpha" }, "devDependencies": { diff --git a/scripts/config.js b/scripts/config.js index 7b3b234d1..f9bbe5e24 100644 --- a/scripts/config.js +++ b/scripts/config.js @@ -31,6 +31,13 @@ const symlinkFiles = [ // these files will be browserified during the build const browserifyConfigs = [ + { + src: 'node_modules/url/url.js', + dest: 'resource/url.js', + config: { + standalone: 'url' + } + }, { src: 'node_modules/sinon/lib/sinon.js', dest: 'test/resource/sinon.js', diff --git a/test/tests/utilitiesTest.js b/test/tests/utilitiesTest.js index a12f665b4..74c39dba5 100644 --- a/test/tests/utilitiesTest.js +++ b/test/tests/utilitiesTest.js @@ -430,6 +430,52 @@ describe("Zotero.Utilities", function() { }) }); + describe("#parseURL()", function () { + var f; + before(() => { + f = Zotero.Utilities.parseURL; + }); + + describe("#fileName", function () { + it("should contain filename", function () { + assert.propertyVal(f('http://example.com/abc/def.html?foo=bar'), 'fileName', 'def.html'); + }); + + it("should be empty if no filename", function () { + assert.propertyVal(f('http://example.com/abc/'), 'fileName', ''); + }); + }); + + describe("#fileExtension", function () { + it("should contain extension", function () { + assert.propertyVal(f('http://example.com/abc/def.html?foo=bar'), 'fileExtension', 'html'); + }); + + it("should be empty if no extension", function () { + assert.propertyVal(f('http://example.com/abc/def'), 'fileExtension', ''); + }); + + it("should be empty if no filename", function () { + assert.propertyVal(f('http://example.com/abc/'), 'fileExtension', ''); + }); + }); + + describe("#fileBaseName", function () { + it("should contain base name", function () { + assert.propertyVal(f('http://example.com/abc/def.html?foo=bar'), 'fileBaseName', 'def'); + }); + + it("should equal filename if no extension", function () { + assert.propertyVal(f('http://example.com/abc/def'), 'fileBaseName', 'def'); + }); + + it("should be empty if no filename", function () { + assert.propertyVal(f('http://example.com/abc/'), 'fileBaseName', ''); + }); + }); + }); + + describe("#ellipsize()", function () { describe("with wordBoundary", function () { it("should truncate at word boundary", function* () {