From 0352fa35b4cd1048cc87132016331fd080289efa Mon Sep 17 00:00:00 2001 From: abaevbog Date: Mon, 5 Aug 2024 22:31:47 -0700 Subject: [PATCH] OpenURL resolver pref: Fix glitchy drop-down keyboard navigation (#4506) - minor refactoring to avoid deleting the first ("Custom") menuitem of the resolver menulist. It is the first item that is selected when popup opens and deleting it confuses keyboard navigation, so that arrowDown/Up won't navigate the menu (unless the menu is hovered over with a mouse) - explicitly re-select the first item when the menulist closes. Otherwise, in case of having navigated the menus with arrows without changing selection and closing the popup, next time resolver selector appears, arrowUp/Down will not navigate the list. Only occurs on Windows. - clear the resolver menus (except for the 1st item) when the popup closes so that arrowUp/Down on focused dropdown don't select invalid top-level menus (e.g. "North America"). - make sure that if the URL has been edited, the resolver dropdown's value will switch to "Custom" even if the resolvers were not loaded fixes: #4491 --- .../zotero/preferences/preferences_general.js | 63 +++++++++++++------ .../preferences/preferences_general.xhtml | 1 + scss/preferences/_general.scss | 9 +++ 3 files changed, 53 insertions(+), 20 deletions(-) diff --git a/chrome/content/zotero/preferences/preferences_general.js b/chrome/content/zotero/preferences/preferences_general.js index e54ccf6927..1af48f465d 100644 --- a/chrome/content/zotero/preferences/preferences_general.js +++ b/chrome/content/zotero/preferences/preferences_general.js @@ -343,21 +343,52 @@ Zotero_Preferences.General = { if (event.target.id != 'openurl-primary-popup') { return; } + var openURLMenu = document.getElementById('openurl-menu'); + let openURLMenuFirstItem = openURLMenu.menupopup.firstChild; if (!this._openURLResolvers) { - let menupopup = document.getElementById('openurl-primary-popup'); - menupopup.firstChild.setAttribute('label', Zotero.getString('general.loading')); + openURLMenuFirstItem.setAttribute('label', Zotero.getString('general.loading')); try { this._openURLResolvers = await Zotero.Utilities.Internal.OpenURL.getResolvers(); } catch (e) { Zotero.logError(e); - menupopup.firstChild.setAttribute('label', "Error loading resolvers"); + openURLMenu.menupopup.firstChild.setAttribute('label', "Error loading resolvers"); return; } } + // Set top-most item to "Custom" once the menu appears + openURLMenuFirstItem.setAttribute('label', Zotero.getString('general.custom')); + openURLMenuFirstItem.setAttribute('value', 'custom'); this.updateOpenURLResolversMenu(); }, + handleOpenURLPopupHidden(event) { + if (event.target.id != 'openurl-primary-popup') { + return; + } + // Clear the menu so that on Windows arrowUp/Down does not select an invalid + // top-level entry (e.g. North America) + this.emptyOpenURLMenu(); + // Set the proper values on the first item to be displayed when dropdown closes + let firstItem = document.getElementById('openurl-menu').menupopup.firstChild; + if (Zotero.Prefs.get('openURL.resolver')) { + firstItem.setAttribute("value", Zotero.Prefs.get('openURL.resolver')); + } + if (Zotero.Prefs.get('openURL.name')) { + firstItem.setAttribute("label", Zotero.Prefs.get('openURL.name')); + } + // Ensures that arrow keys will navigate the menu in case of subsequent opening on windows + document.getElementById('openurl-menu').selectedItem = firstItem; + }, + + // Clear all menus, except for the top-most "Custom" menuitem. That item is selected + // when menu opens and, if removed while still selected, keyboard navigation may break. + emptyOpenURLMenu() { + var openURLMenu = document.getElementById('openurl-menu'); + var menupopup = openURLMenu.firstChild; + while (menupopup.childNodes.length > 1) menupopup.removeChild(menupopup.lastChild); + }, + updateOpenURLResolversMenu: function () { if (!this._openURLResolvers) { @@ -369,13 +400,8 @@ Zotero_Preferences.General = { var openURLMenu = document.getElementById('openurl-menu'); var menupopup = openURLMenu.firstChild; - menupopup.innerHTML = ''; - - var customMenuItem = document.createXULElement('menuitem'); - customMenuItem.setAttribute('label', Zotero.getString('general.custom')); - customMenuItem.setAttribute('value', 'custom'); - customMenuItem.setAttribute('type', 'checkbox'); - menupopup.appendChild(customMenuItem); + let firstItem = menupopup.firstChild; + this.emptyOpenURLMenu(); menupopup.appendChild(document.createXULElement('menuseparator')); @@ -429,11 +455,12 @@ Zotero_Preferences.General = { openURLMenu.setAttribute('label', selectedName); // If we found a match, update stored name Zotero.Prefs.set('openURL.name', selectedName); + firstItem.setAttribute('checked', false); } // Custom else { openURLMenu.setAttribute('label', Zotero.getString('general.custom')); - customMenuItem.setAttribute('checked', true); + firstItem.setAttribute('checked', true); Zotero.Prefs.clear('openURL.name'); } }, @@ -463,18 +490,14 @@ Zotero_Preferences.General = { Zotero.Prefs.set('openURL.name', openURLServerField.value = event.target.label); Zotero.Prefs.set('openURL.resolver', openURLServerField.value = event.target.value); } - - openURLMenu.firstChild.hidePopup(); - - setTimeout(() => { - this.updateOpenURLResolversMenu(); - }); }, onOpenURLCustomized: function () { - setTimeout(() => { - this.updateOpenURLResolversMenu(); - }); + // Change resolver preference to "custom" + let firstItem = document.getElementById('openurl-menu').menupopup.firstChild; + firstItem.setAttribute('label', Zotero.getString('general.custom')); + firstItem.setAttribute('value', 'custom'); + Zotero.Prefs.clear('openURL.name'); }, EBOOK_FONT_STACKS: { diff --git a/chrome/content/zotero/preferences/preferences_general.xhtml b/chrome/content/zotero/preferences/preferences_general.xhtml index 7362ac79e4..19210ebf11 100644 --- a/chrome/content/zotero/preferences/preferences_general.xhtml +++ b/chrome/content/zotero/preferences/preferences_general.xhtml @@ -245,6 +245,7 @@ diff --git a/scss/preferences/_general.scss b/scss/preferences/_general.scss index a582ee84ab..013ef418ab 100644 --- a/scss/preferences/_general.scss +++ b/scss/preferences/_general.scss @@ -33,3 +33,12 @@ margin-block: 4px; } } + +// "Custom" option of OpenURL resolver is initially selected for better keyboard +// navigation on Windows, but we don't want to display the checkmark in that +// case. Only show checkmark if "checked=true". +#zotero-prefpane-general #openurl-menu { + menuitem:is([selected="true"]):not([checked="true"])::before { + visibility: hidden; + } +}