From 8c59df435f1f5d243d1f720c1234bc2ad136aa7b Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sun, 25 Feb 2018 15:07:33 -0500 Subject: [PATCH] Fx60: 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 | 3 +- scripts/config.js | 7 ++++ test/tests/utilitiesTest.js | 46 ++++++++++++++++++++++ 6 files changed, 84 insertions(+), 28 deletions(-) diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js index 00b203a954..0db4985781 100644 --- a/chrome/content/zotero/xpcom/attachments.js +++ b/chrome/content/zotero/xpcom/attachments.js @@ -2426,53 +2426,43 @@ Zotero.Attachments = new function(){ this._getFileNameFromURL = function(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 ce5d85e37c..29a58fd813 100644 --- a/chrome/content/zotero/xpcom/utilities.js +++ b/chrome/content/zotero/xpcom/utilities.js @@ -1913,6 +1913,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 d5422d1407..237a2ea520 100644 --- a/package-lock.json +++ b/package-lock.json @@ -5398,8 +5398,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", @@ -6686,7 +6685,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" @@ -6695,8 +6693,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 4faf8096ae..6154b6130f 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,8 @@ "prop-types": "^15.6.2", "react": "^16.8.6", "react-dom": "^16.8.6", - "react-intl": "^2.7.2" + "react-intl": "^2.7.2", + "url": "^0.11.0" }, "devDependencies": { "@babel/cli": "^7.2.3", diff --git a/scripts/config.js b/scripts/config.js index 817a31a861..ec64adb9fc 100644 --- a/scripts/config.js +++ b/scripts/config.js @@ -38,6 +38,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 6d57bd5100..81d035a3fb 100644 --- a/test/tests/utilitiesTest.js +++ b/test/tests/utilitiesTest.js @@ -477,6 +477,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* () {