From 261bb7ee915e3ba8e761de57329c10efb4beabed Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sat, 26 Jun 2021 17:03:52 -0400 Subject: [PATCH] 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 | 21 ++++++++---- chrome/content/zotero/xpcom/dictionaries.js | 37 +++++++++++++++------ chrome/content/zotero/xpcom/file.js | 2 +- test/tests/dictionariesTest.js | 12 ++++--- 4 files changed, 49 insertions(+), 23 deletions(-) diff --git a/chrome/content/zotero/dictionaryManager.js b/chrome/content/zotero/dictionaryManager.js index d85630d740..050b54b2a4 100644 --- a/chrome/content/zotero/dictionaryManager.js +++ b/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; } diff --git a/chrome/content/zotero/xpcom/dictionaries.js b/chrome/content/zotero/xpcom/dictionaries.js index f2c262d0b2..14f38edffc 100644 --- a/chrome/content/zotero/xpcom/dictionaries.js +++ b/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,11 +140,21 @@ Zotero.Dictionaries = new function () { await _loadDirectory(dir); } catch (e) { - if (await OS.File.exists(xpiPath)) { - await OS.File.remove(xpiPath); + try { + if (await OS.File.exists(xpiPath)) { + await OS.File.remove(xpiPath); + } } - if (await OS.File.exists(dir)) { - await OS.File.removeDir(dir); + 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 }); diff --git a/chrome/content/zotero/xpcom/file.js b/chrome/content/zotero/xpcom/file.js index aef260600d..57320b2502 100644 --- a/chrome/content/zotero/xpcom/file.js +++ b/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 + "'"); } diff --git a/test/tests/dictionariesTest.js b/test/tests/dictionariesTest.js index 1a34b832c3..352d1c9fc6 100644 --- a/test/tests/dictionariesTest.js +++ b/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);