From 5d6ad703c172e13dca87e8edceb70f38f09fc115 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Fri, 2 Sep 2022 11:01:37 -0400 Subject: [PATCH] Preferences: Fix tests, improve clarity, and more - Fix sync and advanced preferences tests; use a new waitForFirstPaneLoad() functions instead of the old paneload event listener - Remove empty preferences_searchTest.js - Rename some Zotero_Preferences members/functions for better clarity and public/private differentiation - Reorder, also for clarity - Fix tabIndex parameter causing an error if invalid - Remove window.sizeToContent() call in Sync.displayFields() - So *that's* why the window resized every time the sync pane was loaded... - Deprecate openURL() and simplify openInViewer() --- .../content/zotero/preferences/preferences.js | 301 ++++++++---------- .../preferences/preferences_advanced.js | 2 +- .../zotero/preferences/preferences_sync.jsx | 2 - .../content/zotero/xpcom/preferencePanes.js | 2 +- test/tests/preferences_advancedTest.js | 25 +- test/tests/preferences_searchTest.js | 5 - test/tests/preferences_syncTest.js | 12 +- 7 files changed, 144 insertions(+), 205 deletions(-) delete mode 100644 test/tests/preferences_searchTest.js diff --git a/chrome/content/zotero/preferences/preferences.js b/chrome/content/zotero/preferences/preferences.js index 9c41c9de7a..72fab3bde8 100644 --- a/chrome/content/zotero/preferences/preferences.js +++ b/chrome/content/zotero/preferences/preferences.js @@ -26,15 +26,18 @@ "use strict"; var Zotero_Preferences = { + panes: new Map(), + + _firstPaneLoadDeferred: Zotero.Promise.defer(), + init: function () { - this.observerSymbols = []; - this.panes = new Map(); + this._observerSymbols = []; this.navigation = document.getElementById('prefs-navigation'); this.content = document.getElementById('prefs-content'); this.navigation.addEventListener('select', () => this._onNavigationSelect()); document.getElementById('prefs-search').addEventListener('command', - event => this.search(event.target.value)); + event => this._search(event.target.value)); document.getElementById('prefs-subpane-back-button').addEventListener('command', () => { let parent = this.panes.get(this.navigation.value).parent; @@ -45,10 +48,14 @@ var Zotero_Preferences = { document.getElementById('prefs-search').focus(); - this.initPanes(); + this.clearAndAddPanes(); }, - - initPanes: function () { + + /** + * (Re)initialize the preference window, adding all panes registered with Zotero.PreferencePanes. + * Current selected pane and scroll position are saved and restored after load. + */ + clearAndAddPanes: function () { // Save positions and clear content in case we're reinitializing // because of a plugin lifecycle event let navigationValue = this.navigation.value; @@ -59,12 +66,12 @@ var Zotero_Preferences = { let contentScrollLeft = this.content.scrollLeft; this.content.replaceChildren(); - Zotero.PreferencePanes.builtInPanes.forEach(pane => this.addPane(pane)); + Zotero.PreferencePanes.builtInPanes.forEach(pane => this._addPane(pane)); if (Zotero.PreferencePanes.pluginPanes.length) { this.navigation.append(document.createElement('hr')); Zotero.PreferencePanes.pluginPanes .sort((a, b) => Zotero.localeCompare(a.rawLabel, b.rawLabel)) - .forEach(pane => this.addPane(pane)); + .forEach(pane => this._addPane(pane)); } if (navigationValue) { @@ -92,7 +99,10 @@ var Zotero_Preferences = { } // Select tab within pane by index else if (tabIndex !== undefined) { - pane.querySelector('tabbox').selectedIndex = tabIndex; + let tabBox = pane.querySelector('tabbox'); + if (tabBox) { + tabBox.selectedIndex = tabIndex; + } } } } @@ -110,13 +120,63 @@ var Zotero_Preferences = { }, onUnload: function () { - if (Zotero_Preferences.Debug_Output) { - Zotero_Preferences.Debug_Output.onUnload(); + while (this._observerSymbols.length) { + Zotero.Prefs.unregisterObserver(this._observerSymbols.shift()); } + }, + + waitForFirstPaneLoad: async function () { + await this._firstPaneLoadDeferred.promise; + }, - while (this.observerSymbols.length) { - Zotero.Prefs.unregisterObserver(this.observerSymbols.shift()); + /** + * Select a pane in the navigation sidebar, displaying its content. + * Clears the current search and hides all other panes' content. + * + * @param {String} id + */ + navigateToPane(id) { + this.navigation.value = id; + }, + + openHelpLink: function () { + // TODO: Re-add help and update this for new preferences architecture + + var url = "http://www.zotero.org/support/preferences/"; + var helpTopic = document.getElementsByTagName("prefwindow")[0].currentPane.helpTopic; + url += helpTopic; + + Zotero.launchURL(url); + }, + + /** + * Opens a URI in the basic viewer + */ + openInViewer: function (uri) { + var wm = Components.classes["@mozilla.org/appshell/window-mediator;1"] + .getService(Components.interfaces.nsIWindowMediator); + var win = wm.getMostRecentWindow("zotero:basicViewer"); + if (win) { + win.loadURI(uri); } + else { + window.openDialog("chrome://zotero/content/standalone/basicViewer.xhtml", + "basicViewer", "chrome,resizable,centerscreen,menubar,scrollbars", uri); + } + }, + + _onNavigationSelect() { + for (let child of this.content.children) { + child.setAttribute('hidden', true); + } + let paneID = this.navigation.value; + if (paneID) { + this.content.scrollTop = 0; + document.getElementById('prefs-search').value = ''; + this._search(''); + this._loadAndDisplayPane(paneID); + } + Zotero.Prefs.set('lastSelectedPrefPane', paneID); }, /** @@ -138,7 +198,7 @@ var Zotero_Preferences = { * namespaced under `html:`. Default behavior is the opposite: whitespace nodes are preserved, * HTML is the default namespace, and XUL tags are under `xul:`. */ - async addPane(options) { + _addPane(options) { let { id, parent, label, rawLabel, image } = options; let listItem = document.createXULElement('richlistitem'); @@ -184,16 +244,6 @@ var Zotero_Preferences = { }); }, - /** - * Select a pane in the navigation sidebar, displaying its content. - * Clears the current search and hides all other panes' content. - * - * @param {String} id - */ - navigateToPane(id) { - this.navigation.value = id; - }, - /** * Display a pane's content, alongside any other panes already showing. * If the pane is not yet loaded, it will be loaded first. @@ -216,7 +266,7 @@ var Zotero_Preferences = { ]; let contentFragment = pane.defaultXUL ? MozXULElement.parseXULToFragment(markup, dtdFiles) - : this.parseXHTMLToFragment(markup, dtdFiles); + : this._parseXHTMLToFragment(markup, dtdFiles); contentFragment = document.importNode(contentFragment, true); pane.container.append(contentFragment); pane.imported = true; @@ -230,7 +280,7 @@ var Zotero_Preferences = { backButton.hidden = !pane.parent; }, - parseXHTMLToFragment(str, entities = []) { + _parseXHTMLToFragment(str, entities = []) { // Adapted from MozXULElement.parseXULToFragment /* eslint-disable indent */ @@ -260,6 +310,59 @@ ${str} return range.extractContents(); }, + _initImportedNodes(root) { + // Activate `preference` attributes + for (let elem of root.querySelectorAll('[preference]')) { + let preference = elem.getAttribute('preference'); + if (root.querySelector('preferences > preference#' + preference)) { + Zotero.warn(' is deprecated -- `preference` attribute values ' + + 'should be full preference keys, not IDs'); + preference = root.querySelector('preferences > preference#' + preference) + .getAttribute('name'); + } + + let useChecked = (elem instanceof HTMLInputElement && elem.type == 'checkbox') + || elem.tagName == 'checkbox'; + + elem.addEventListener(elem instanceof XULElement ? 'command' : 'input', () => { + let value = useChecked ? elem.checked : elem.value; + Zotero.Prefs.set(preference, value, true); + elem.dispatchEvent(new Event('synctopreference')); + }); + + let syncFromPref = () => { + let value = Zotero.Prefs.get(preference, true); + if (useChecked) { + elem.checked = value; + } + else { + elem.value = value; + } + elem.dispatchEvent(new Event('syncfrompreference')); + }; + + // Set timeout so pane can add listeners first + setTimeout(() => { + syncFromPref(); + this._observerSymbols.push(Zotero.Prefs.registerObserver(preference, syncFromPref, true)); + + // If this is the first pane to be loaded, notify anyone waiting + // (for tests) + this._firstPaneLoadDeferred.resolve(); + }); + } + + // parseXULToFragment() doesn't convert oncommand attributes into actual + // listeners, so we'll do it here + for (let elem of root.querySelectorAll('[oncommand]')) { + elem.oncommand = elem.getAttribute('oncommand'); + } + + for (let child of root.children) { + child.dispatchEvent(new Event('load')); + } + }, + /** * If term is falsy, clear the current search and show the first pane. * If term is truthy, execute a search: @@ -270,7 +373,7 @@ ${str} * * @param {String} [term] */ - search(term) { + _search(term) { // Initial housekeeping: // Clear existing highlights @@ -328,7 +431,7 @@ ${str} if (!root) continue; for (let child of root.children) { - let matches = this._searchRecursively(child, term); + let matches = this._findNodesMatching(child, term); if (matches.length) { let touchedTabPanels = new Set(); for (let node of matches) { @@ -394,7 +497,7 @@ ${str} * @param {String} term Must be normalized (normalizeSearch()) * @return {Node[]} */ - _searchRecursively(root, term) { + _findNodesMatching(root, term) { const EXCLUDE_SELECTOR = 'input, [hidden="true"], [no-highlight]'; let matched = new Set(); @@ -496,142 +599,12 @@ ${str} } return node?.closest(selector); }, - - _onNavigationSelect() { - for (let child of this.content.children) { - child.setAttribute('hidden', true); - } - let paneID = this.navigation.value; - if (paneID) { - this.content.scrollTop = 0; - document.getElementById('prefs-search').value = ''; - this.search(''); - this._loadAndDisplayPane(paneID); - } - Zotero.Prefs.set('lastSelectedPrefPane', paneID); - }, - - _initImportedNodes(root) { - // Activate `preference` attributes - for (let elem of root.querySelectorAll('[preference]')) { - let preference = elem.getAttribute('preference'); - if (root.querySelector('preferences > preference#' + preference)) { - Zotero.debug(' is deprecated -- `preference` attribute values ' - + 'should be full preference keys, not IDs'); - preference = root.querySelector('preferences > preference#' + preference) - .getAttribute('name'); - } - - let useChecked = (elem instanceof HTMLInputElement && elem.type == 'checkbox') - || elem.tagName == 'checkbox'; - - elem.addEventListener(elem instanceof XULElement ? 'command' : 'input', () => { - let value = useChecked ? elem.checked : elem.value; - Zotero.Prefs.set(preference, value, true); - elem.dispatchEvent(new Event('synctopreference')); - }); - - let syncFromPref = () => { - let value = Zotero.Prefs.get(preference, true); - if (useChecked) { - elem.checked = value; - } - else { - elem.value = value; - } - elem.dispatchEvent(new Event('syncfrompreference')); - }; - - // Set timeout so pane can add listeners first - setTimeout(() => { - syncFromPref(); - this.observerSymbols.push(Zotero.Prefs.registerObserver(preference, syncFromPref, true)); - }); - } - - // parseXULToFragment() doesn't convert oncommand attributes into actual - // listeners, so we'll do it here - for (let elem of root.querySelectorAll('[oncommand]')) { - elem.oncommand = elem.getAttribute('oncommand'); - } - - for (let child of root.children) { - child.dispatchEvent(new Event('load')); - } - }, - - openURL: function (url, windowName) { - // Non-instantApply prefwindows are usually modal, so we can't open in the topmost window, - // since it's probably behind the window - var instantApply = Zotero.Prefs.get("browser.preferences.instantApply", true); - - if (instantApply) { - window.opener.ZoteroPane_Local.loadURI(url, { shiftKey: true, metaKey: true }); - } - else { - if (Zotero.isStandalone) { - var io = Components.classes['@mozilla.org/network/io-service;1'] - .getService(Components.interfaces.nsIIOService); - var uri = io.newURI(url, null, null); - var handler = Components.classes['@mozilla.org/uriloader/external-protocol-service;1'] - .getService(Components.interfaces.nsIExternalProtocolService) - .getProtocolHandlerInfo('http'); - handler.preferredAction = Components.interfaces.nsIHandlerInfo.useSystemDefault; - handler.launchWithURI(uri, null); - } - else { - var ww = Components.classes["@mozilla.org/embedcomp/window-watcher;1"] - .getService(Components.interfaces.nsIWindowWatcher); - var win = ww.openWindow( - window, - url, - windowName ? windowName : null, - "chrome=no,menubar=yes,location=yes,toolbar=yes,personalbar=yes,resizable=yes,scrollbars=yes,status=yes", - null - ); - } - } - }, - - openHelpLink: function () { - var url = "http://www.zotero.org/support/preferences/"; - var helpTopic = document.getElementsByTagName("prefwindow")[0].currentPane.helpTopic; - url += helpTopic; - - this.openURL(url, "helpWindow"); - }, - /** - * Opens a URI in the basic viewer in Standalone, or a new window in Firefox + * @deprecated Use {@link Zotero.launchURL} */ - openInViewer: function (uri, newTab) { - var wm = Components.classes["@mozilla.org/appshell/window-mediator;1"] - .getService(Components.interfaces.nsIWindowMediator); - const features = "menubar=yes,toolbar=no,location=no,scrollbars,centerscreen,resizable"; - - if(Zotero.isStandalone) { - var win = wm.getMostRecentWindow("zotero:basicViewer"); - if(win) { - win.loadURI(uri); - } else { - window.openDialog("chrome://zotero/content/standalone/basicViewer.xhtml", - "basicViewer", "chrome,resizable,centerscreen,menubar,scrollbars", uri); - } - } else { - var win = wm.getMostRecentWindow("navigator:browser"); - if(win) { - if(newTab) { - win.gBrowser.selectedTab = win.gBrowser.addTab(uri); - } else { - win.open(uri, null, features); - } - } - else { - var ww = Components.classes["@mozilla.org/embedcomp/window-watcher;1"] - .getService(Components.interfaces.nsIWindowWatcher); - var win = ww.openWindow(null, uri, null, features + ",width=775,height=575", null); - } - } + openURL: function (url) { + Zotero.warn("Zotero_Preferences.openURL() is deprecated -- use Zotero.launchURL()"); + Zotero.launchURL(url); } -}; \ No newline at end of file +}; diff --git a/chrome/content/zotero/preferences/preferences_advanced.js b/chrome/content/zotero/preferences/preferences_advanced.js index 9b9845de0b..a0d8a913cf 100644 --- a/chrome/content/zotero/preferences/preferences_advanced.js +++ b/chrome/content/zotero/preferences/preferences_advanced.js @@ -399,7 +399,7 @@ Zotero_Preferences.Advanced = { yield Zotero.DataDirectory.choose( true, !newUseDataDir, - () => Zotero_Preferences.openURL('https://www.zotero.org/support/zotero_data') + () => Zotero.launchURL('https://www.zotero.org/support/zotero_data') ); radiogroup.selectedIndex = this._usingDefaultDataDir() ? 0 : 1; }), diff --git a/chrome/content/zotero/preferences/preferences_sync.jsx b/chrome/content/zotero/preferences/preferences_sync.jsx index 9c63136f93..eee6f1d16b 100644 --- a/chrome/content/zotero/preferences/preferences_sync.jsx +++ b/chrome/content/zotero/preferences/preferences_sync.jsx @@ -92,8 +92,6 @@ Zotero_Preferences.Sync = { var img = document.getElementById('sync-status-indicator'); img.removeAttribute('verified'); img.removeAttribute('animated'); - - window.sizeToContent(); }, diff --git a/chrome/content/zotero/xpcom/preferencePanes.js b/chrome/content/zotero/xpcom/preferencePanes.js index 3a1efd86ed..2565dbf130 100644 --- a/chrome/content/zotero/xpcom/preferencePanes.js +++ b/chrome/content/zotero/xpcom/preferencePanes.js @@ -141,7 +141,7 @@ Zotero.PreferencePanes = { _refreshPreferences() { for (let win of Services.wm.getEnumerator("zotero:pref")) { - win.Zotero_Preferences.initPanes(); + win.Zotero_Preferences.clearAndAddPanes(); } }, diff --git a/test/tests/preferences_advancedTest.js b/test/tests/preferences_advancedTest.js index 6544a3f727..2341b21ccd 100644 --- a/test/tests/preferences_advancedTest.js +++ b/test/tests/preferences_advancedTest.js @@ -65,21 +65,11 @@ describe("Advanced Preferences", function () { describe("Linked Attachment Base Directory", function () { var setBaseDirectory = Zotero.Promise.coroutine(function* (basePath) { var win = yield loadWindow("chrome://zotero/content/preferences/preferences.xhtml", { - pane: 'zotero-prefpane-advanced', - tabIndex: 1 + pane: 'zotero-prefpane-advanced' }); // Wait for tab to load - var doc = win.document; - var prefwindow = doc.documentElement; - var defer = Zotero.Promise.defer(); - var pane = doc.getElementById('zotero-prefpane-advanced'); - if (!pane.loaded) { - pane.addEventListener('paneload', function () { - defer.resolve(); - }) - yield defer.promise; - } + yield win.Zotero_Preferences.waitForFirstPaneLoad(); var promise = waitForDialog(); yield win.Zotero_Preferences.Attachment_Base_Directory.changePath(basePath); @@ -95,16 +85,7 @@ describe("Advanced Preferences", function () { }); // Wait for tab to load - var doc = win.document; - var prefwindow = doc.documentElement; - var defer = Zotero.Promise.defer(); - var pane = doc.getElementById('zotero-prefpane-advanced'); - if (!pane.loaded) { - pane.addEventListener('paneload', function () { - defer.resolve(); - }) - yield defer.promise; - } + yield win.Zotero_Preferences.waitForFirstPaneLoad(); var promise = waitForDialog(); yield win.Zotero_Preferences.Attachment_Base_Directory.clearPath(); diff --git a/test/tests/preferences_searchTest.js b/test/tests/preferences_searchTest.js deleted file mode 100644 index f4a203ae05..0000000000 --- a/test/tests/preferences_searchTest.js +++ /dev/null @@ -1,5 +0,0 @@ -describe("Search Preferences", function () { - describe("PDF Indexing", function () { - - }) -}) diff --git a/test/tests/preferences_syncTest.js b/test/tests/preferences_syncTest.js index de73bc157a..673364e61e 100644 --- a/test/tests/preferences_syncTest.js +++ b/test/tests/preferences_syncTest.js @@ -3,18 +3,10 @@ describe("Sync Preferences", function () { before(function* () { // Load prefs with sync pane win = yield loadWindow("chrome://zotero/content/preferences/preferences.xhtml", { - pane: 'zotero-prefpane-sync', - tabIndex: 0 + pane: 'zotero-prefpane-sync' }); doc = win.document; - let defer = Zotero.Promise.defer(); - let pane = doc.getElementById('zotero-prefpane-sync'); - if (!pane.loaded) { - pane.addEventListener('paneload', function () { - defer.resolve(); - }); - yield defer.promise; - } + yield win.Zotero_Preferences.waitForFirstPaneLoad(); }); after(function() {