Dictionary handling improvements

- Fix installation of dictionaries on Windows
- Use version in XPI download URL (zotero/zotero-build@c3308c7a4)
- Improve display of download errors

chrome/content/zotero/dictionaryManager.js

@@ -39,7 +39,7 @@ var Zotero_Dictionary_Manager = new function () {
 		var installedLocales = new Set(Zotero.Dictionaries.dictionaries.map(d => d.locale));
 		var availableDictionaries = await Zotero.Dictionaries.fetchDictionariesList();
 		var availableUpdates = await Zotero.Dictionaries.getAvailableUpdates(availableDictionaries);
-		updateMap = new Map(availableUpdates.map(x => [x.old.id, x.new.id]));
+		updateMap = new Map(availableUpdates.map(x => [x.old.id, { id: x.new.id, version: x.new.version }]));
 		
 		var { InlineSpellChecker } = ChromeUtils.import("resource://gre/modules/InlineSpellChecker.jsm", {});
 		var isc = new InlineSpellChecker();
@@ -83,6 +83,7 @@ var Zotero_Dictionary_Manager = new function () {
 			checkbox.dataset.dictId = d.id;
 			checkbox.dataset.dictLocale = d.locale;
 			checkbox.dataset.dictName = d.name;
+			checkbox.dataset.dictVersion = d.version;
 			// en-US is always checked and disabled
 			checkbox.checked = d.locale == 'en-US' || installed.has(d.id);
 			if (d.locale == 'en-US') {
@@ -133,10 +134,18 @@ var Zotero_Dictionary_Manager = new function () {
 			if (updateMap.has(id)) {
 				// If id is changing, delete the old one first
 				toRemove.push(id);
-				toDownload.push({ id: updateMap.get(id), name: elem.dataset.dictName });
+				toDownload.push({
+					id: updateMap.get(id).id,
+					name: elem.dataset.dictName,
+					version: updateMap.get(id).version
+				});
 			}
 			else if (!installed.has(id)) {
-				toDownload.push({ id, name: elem.dataset.dictName });
+				toDownload.push({
+					id,
+					name: elem.dataset.dictName,
+					version: elem.dataset.dictVersion
+				});
 			}
 		}
 		if (toRemove.length) {
@@ -145,10 +154,10 @@ var Zotero_Dictionary_Manager = new function () {
 			}
 		}
 		if (toDownload.length) {
-			for (let { id, name } of toDownload) {
+			for (let { id, name, version } of toDownload) {
 				_updateStatus(Zotero.getString('general.downloading.quoted', name));
 				try {
-					await Zotero.Dictionaries.install(id);
+					await Zotero.Dictionaries.install(id, version);
 				}
 				catch (e) {
 					Zotero.logError(e);
@@ -156,7 +165,7 @@ var Zotero_Dictionary_Manager = new function () {
 						null,
 						Zotero.getString('general.error'),
 						Zotero.getString('spellCheck.dictionaryManager.error.unableToInstall', name)
-							+ "\n\n" + e.message
+							+ "\n\n" + (e.message ? (e.message + "\n\n" + e.stack) : e)
 					);
 					return;
 				}

chrome/content/zotero/xpcom/dictionaries.js

@@ -92,15 +92,19 @@ Zotero.Dictionaries = new function () {
 	 * e.g., `en-NZ@dictionaries.addons.mozilla.org`
 	 *
 	 * @param {String} id - Dictionary extension id
+	 * @param {String} version - Dictionary extension version
 	 * @return {Promise}
 	 */
-	this.install = async function (id) {
+	this.install = async function (id, version) {
 		if (id == '@unitedstatesenglishdictionary') {
 			throw new Error("en-US dictionary is bundled");
 		}
+		if (!version) {
+			throw new Error("Version not provided");
+		}
 		await this.remove(id);
 		Zotero.debug("Installing dictionaries from " + id);
-		let url = this.baseURL + id + '.xpi';
+		let url = this.baseURL + id + '-' + version + '.xpi';
 		let xpiPath = OS.Path.join(Zotero.getTempDirectory().path, id);
 		let dir = OS.Path.join(Zotero.Profile.dir, 'dictionaries', id);
 		let zipReader = Components.classes['@mozilla.org/libjar/zip-reader;1']
@@ -115,7 +119,7 @@ Zotero.Dictionaries = new function () {
 			let entries = zipReader.findEntries('*/');
 			while (entries.hasMore()) {
 				let entry = entries.getNext();
-				let destPath = OS.Path.join(dir, entry);
+				let destPath = OS.Path.join(dir, ...entry.split(/\//));
 				await Zotero.File.createDirectoryIfMissingAsync(destPath, { from: Zotero.Profile.dir });
 			}
 
@@ -126,7 +130,8 @@ Zotero.Dictionaries = new function () {
 				if (entry.substr(-1) === '/') {
 					continue;
 				}
-				let destPath = OS.Path.join(dir, entry);
+				Zotero.debug("Extracting " + entry);
+				let destPath = OS.Path.join(dir, ...entry.split(/\//));
 				zipReader.extract(entry, Zotero.File.pathToFile(destPath));
 			}
 
@@ -135,12 +140,22 @@ Zotero.Dictionaries = new function () {
 			await _loadDirectory(dir);
 		}
 		catch (e) {
+			try {
 				if (await OS.File.exists(xpiPath)) {
 					await OS.File.remove(xpiPath);
 				}
+			}
+			catch (e) {
+				Zotero.logError(e);
+			}
+			try {
 				if (await OS.File.exists(dir)) {
 					await OS.File.removeDir(dir);
 				}
+			}
+			catch (e) {
+				Zotero.logError(e);
+			}
 			throw e;
 		}
 	};
@@ -163,7 +178,7 @@ Zotero.Dictionaries = new function () {
 			manifest = JSON.parse(manifest);
 			for (let locale in manifest.dictionaries) {
 				let dicPath = manifest.dictionaries[locale];
-				let affPath = OS.Path.join(dictionary.dir, dicPath.slice(0, -3) + 'aff');
+				let affPath = OS.Path.join(dictionary.dir, ...dicPath.split(/\//)).slice(0, -3) + 'aff';
 				Zotero.debug(`Removing ${locale} dictionary`);
 				_spellChecker.removeDictionary(locale, Zotero.File.pathToFile(affPath));
 			}
@@ -248,7 +263,7 @@ Zotero.Dictionaries = new function () {
 		for (let update of updates) {
 			try {
 				await this.remove(update.old.id);
-				await this.install(update.new.id);
+				await this.install(update.new.id, update.new.version);
 				updated++;
 			}
 			catch (e) {
@@ -280,7 +295,7 @@ Zotero.Dictionaries = new function () {
 		for (let locale in manifest.dictionaries) {
 			locales.push(locale);
 			let dicPath = manifest.dictionaries[locale];
-			let affPath = OS.Path.join(dir, dicPath.slice(0, -3) + 'aff');
+			let affPath = OS.Path.join(dir, ...dicPath.split(/\//)).slice(0, -3) + 'aff';
 			Zotero.debug(`Adding ${locale} dictionary`);
 			_spellChecker.addDictionary(locale, Zotero.File.pathToFile(affPath));
 			_dictionaries.push({ id, locale, version, dir });

chrome/content/zotero/xpcom/file.js

@@ -53,7 +53,7 @@ Zotero.File = new function(){
 		catch (e) {
 			Zotero.logError(e);
 		}
-		throw new Error("Unexpected value '" + pathOrFile + "'");
+		throw new Error("Unexpected path value '" + pathOrFile + "'");
 	}
 	
 	

test/tests/dictionariesTest.js

@@ -78,6 +78,7 @@ describe("Dictionaries", function () {
 		}
 		
 		sandbox.stub(Zotero.File, 'download').callsFake(async (url, downloadPath) => {
+			Zotero.debug("Fake downloading " + url);
 			if (url.includes('en-UK')) {
 				return OS.File.copy(enUKXPIOld, downloadPath);
 			}
@@ -89,9 +90,9 @@ describe("Dictionaries", function () {
 			}
 			throw new Error("Unexpected URL " + url);
 		});
-		await Zotero.Dictionaries.install('@fake-en-UK-dictionary');
-		await Zotero.Dictionaries.install('@fake-fr-FR-dictionary');
-		await Zotero.Dictionaries.install('@fake-xx-UN-dictionary');
+		await Zotero.Dictionaries.install('@fake-en-UK-dictionary', "5");
+		await Zotero.Dictionaries.install('@fake-fr-FR-dictionary', "1");
+		await Zotero.Dictionaries.install('@fake-xx-UN-dictionary', "5");
 		sandbox.restore();
 		
 		// Create metadata response for available dictionaries
@@ -112,10 +113,11 @@ describe("Dictionaries", function () {
 			]);
 		
 		sandbox.stub(Zotero.File, 'download').callsFake(async (url, downloadPath) => {
-			if (url.includes('en-UK')) {
+			Zotero.debug("Fake downloading " + url);
+			if (url.includes('en-UK') && url.includes("-1.xpi")) {
 				return OS.File.copy(enUKXPINew, downloadPath);
 			}
-			if (url.includes('fr-FR')) {
+			if (url.includes('fr-FR') && url.includes("-2.xpi")) {
 				return OS.File.copy(frFRv2XPI, downloadPath);
 			}
 			throw new Error("Unexpected URL " + url);