From 087a9859b406e3ee8ea10e4d1a90bdf0dd043c93 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Wed, 24 May 2023 20:45:37 +0300 Subject: [PATCH] Fix preference binding issues and "Include Zotero Links" checkboxes - Set zotero-noteQuickCopy-menu's preference attribute to the correct key - Warn about all ID-ish preference attribute values - zotero-noteQuickCopy-menu's preference attribute was being set to the ID of a now-nonexistent element. preference attribute values should be preference keys now, but we were only warning if the associated element was actually there - We can't warn in all cases where the preference doesn't yet exist, because some preferences don't have default values, and we shouldn't limit to preferences that don't exist, because then the warning will stop showing after the preference is persisted once - When a ID is replaced by the associated key, update the preference attribute so future syncFromPref() and syncToPrefOnModify() calls will set the correct preference - Listen to a range of events on all bound nodes, no matter their type - Don't resolve _firstPaneLoadDeferred until actually done loading Fixes #3131. --- .../content/zotero/preferences/preferences.js | 52 +++++++++++-------- .../zotero/preferences/preferences_export.jsx | 2 +- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/chrome/content/zotero/preferences/preferences.js b/chrome/content/zotero/preferences/preferences.js index 4a2cfcd23b..f50760c7fe 100644 --- a/chrome/content/zotero/preferences/preferences.js +++ b/chrome/content/zotero/preferences/preferences.js @@ -330,14 +330,32 @@ ${str} // We use a single listener function shared between all elements so we can easily detach it later let syncToPrefOnModify = (event) => { - if (event.currentTarget.getAttribute('preference')) { + if (event.target.getAttribute('preference')) { let value = useChecked(event.currentTarget) ? event.currentTarget.checked : event.currentTarget.value; Zotero.Prefs.set(event.currentTarget.getAttribute('preference'), value, true); event.currentTarget.dispatchEvent(new Event('synctopreference')); } }; - let attachToPreference = (elem, preference) => { + let attachToPreference = (elem) => { + let preference = elem.getAttribute('preference'); + try { + if (container.querySelector('preferences > preference#' + preference)) { + Zotero.warn(' is deprecated -- `preference` attribute values ' + + 'should be full preference keys, not IDs'); + preference = container.querySelector('preferences > preference#' + preference) + .getAttribute('name'); + elem.setAttribute('preference', preference); + } + else if (!preference.includes('.')) { + Zotero.warn('`preference` attribute value `' + preference + '` looks like a ID, ' + + 'although no element with that ID exists. Its value should be a preference key.'); + } + } + catch (e) { + // Ignore + } + Zotero.debug(`Attaching <${elem.tagName}> element to ${preference}`); let symbol = Zotero.Prefs.registerObserver( preference, @@ -356,6 +374,10 @@ ${str} subtree: true }); } + + elem.addEventListener('command', syncToPrefOnModify); + elem.addEventListener('input', syncToPrefOnModify); + elem.addEventListener('change', syncToPrefOnModify); }; let detachFromPreference = (elem) => { @@ -368,25 +390,11 @@ ${str} // Activate `preference` attributes for (let elem of container.querySelectorAll('[preference]')) { - let preference = elem.getAttribute('preference'); - if (container.querySelector('preferences > preference#' + preference)) { - Zotero.warn(' is deprecated -- `preference` attribute values ' - + 'should be full preference keys, not IDs'); - preference = container.querySelector('preferences > preference#' + preference) - .getAttribute('name'); - } - - attachToPreference(elem, preference); - - elem.addEventListener(elem instanceof XULElement ? 'command' : 'input', syncToPrefOnModify); + attachToPreference(elem); // Set timeout before populating the value so the pane can add listeners first setTimeout(() => { - syncFromPref(elem, preference); - - // If this is the first pane to be loaded, notify anyone waiting - // (for tests) - this._firstPaneLoadDeferred.resolve(); + syncFromPref(elem, elem.getAttribute('preference')); }); } @@ -396,8 +404,7 @@ ${str} let target = mutation.target; detachFromPreference(target); if (target.hasAttribute('preference')) { - attachToPreference(target, target.getAttribute('preference')); - target.addEventListener(target instanceof XULElement ? 'command' : 'input', syncToPrefOnModify); + attachToPreference(target); } } else if (mutation.type == 'childList') { @@ -407,7 +414,6 @@ ${str} for (let node of mutation.addedNodes) { if (node.nodeType == Node.ELEMENT_NODE && node.hasAttribute('preference')) { attachToPreference(node); - node.addEventListener(node instanceof XULElement ? 'command' : 'input', syncToPrefOnModify); } } } @@ -427,6 +433,10 @@ ${str} for (let child of container.children) { child.dispatchEvent(new Event('load')); } + + // If this is the first pane to be loaded, notify anyone waiting + // (for tests) + this._firstPaneLoadDeferred.resolve(); }, /** diff --git a/chrome/content/zotero/preferences/preferences_export.jsx b/chrome/content/zotero/preferences/preferences_export.jsx index 0dac222ef3..60ccff9d78 100644 --- a/chrome/content/zotero/preferences/preferences_export.jsx +++ b/chrome/content/zotero/preferences/preferences_export.jsx @@ -88,7 +88,7 @@ Zotero_Preferences.Export = { var format = Zotero.Prefs.get("export.noteQuickCopy.setting"); format = Zotero.QuickCopy.unserializeSetting(format); var menulist = document.getElementById("zotero-noteQuickCopy-menu"); - menulist.setAttribute('preference', "pref-noteQuickCopy-setting"); + menulist.setAttribute('preference', "extensions.zotero.export.noteQuickCopy.setting"); menulist.removeEventListener('command', this.updateNoteQuickCopyUI); menulist.addEventListener('command', this.updateNoteQuickCopyUI);