From 181afb92acedd3cbeb6d36af78512dc7022ba4b9 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Thu, 8 Aug 2024 10:50:28 -0600 Subject: [PATCH] Bidi improvements and fixes (#4534) --- .../content/zotero/elements/editableText.js | 58 +++++++++++++---- chrome/content/zotero/elements/itemBox.js | 26 +++++--- chrome/content/zotero/itemTree.jsx | 15 ++++- .../content/zotero/xpcom/data/itemFields.js | 62 +++++++++++++++++++ scss/elements/_editableText.scss | 1 + 5 files changed, 141 insertions(+), 21 deletions(-) diff --git a/chrome/content/zotero/elements/editableText.js b/chrome/content/zotero/elements/editableText.js index ca7122a04f..d405b4a5e8 100644 --- a/chrome/content/zotero/elements/editableText.js +++ b/chrome/content/zotero/elements/editableText.js @@ -26,6 +26,16 @@ "use strict"; { + const { XPCOMUtils } = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm"); + + const lazy = {}; + XPCOMUtils.defineLazyPreferenceGetter( + lazy, + "BIDI_BROWSER_UI", + "bidi.browser.ui", + false + ); + class EditableText extends XULElementBase { _input; @@ -43,7 +53,8 @@ 'nowrap', 'autocomplete', 'min-lines', - 'max-lines' + 'max-lines', + 'dir' ]; static get _textMeasurementSpan() { @@ -128,8 +139,8 @@ } set value(value) { - this.setAttribute('value', value || ''); this.resetTextDirection(); + this.setAttribute('value', value || ''); } get initialValue() { @@ -163,15 +174,21 @@ } } + get dir() { + return this.getAttribute('dir'); + } + + set dir(dir) { + this.setAttribute('dir', dir); + } + get ref() { return this._input; } resetTextDirection() { this._textDirection = null; - if (this._input) { - this._input.dir = null; - } + this._input?.removeAttribute('dir'); } sizeToContent = () => { @@ -297,13 +314,32 @@ } // Set text direction automatically if user has enabled bidi utilities - if ((!this._input.dir || this._input.dir === 'auto') && Zotero.Prefs.get('bidi.browser.ui', true)) { - if (!this._textDirection) { - this._textDirection = window.windowUtils.getDirectionFromText(this._input.value) === Ci.nsIDOMWindowUtils.DIRECTION_RTL - ? 'rtl' - : 'ltr'; + if ((!this._input.dir || this._input.dir === 'auto') && lazy.BIDI_BROWSER_UI) { + // If we haven't already guessed (or been given) a direction, + // see if we can guess from the text + if (!this.dir) { + switch (window.windowUtils.getDirectionFromText(this._input.value)) { + case Ci.nsIDOMWindowUtils.DIRECTION_RTL: + this.dir = 'rtl'; + break; + case Ci.nsIDOMWindowUtils.DIRECTION_LTR: + this.dir = 'ltr'; + break; + case Ci.nsIDOMWindowUtils.DIRECTION_NOT_SET: + default: + this.dir = 'auto'; + break; + } + } + // If all we have is 'auto' but the input has no text, use the + // app locale + if (this.dir === 'auto' && !this._input.value) { + this._input.dir = Zotero.dir; + } + // Otherwise, use the direction we guessed/were given + else { + this._input.dir = this.dir; } - this._input.dir = this._textDirection; } } diff --git a/chrome/content/zotero/elements/itemBox.js b/chrome/content/zotero/elements/itemBox.js index 5d5f6c2c79..92b35123cc 100644 --- a/chrome/content/zotero/elements/itemBox.js +++ b/chrome/content/zotero/elements/itemBox.js @@ -26,6 +26,16 @@ "use strict"; { + const { XPCOMUtils } = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm"); + + const lazy = {}; + XPCOMUtils.defineLazyPreferenceGetter( + lazy, + "BIDI_BROWSER_UI", + "bidi.browser.ui", + false + ); + class ItemBox extends ItemPaneSectionElementBase { constructor() { super(); @@ -1512,17 +1522,15 @@ valueElement.value = valueText; - // Attempt to make bidi things work automatically: - // If we have text to work off of, let the layout engine try to guess the text direction - if (valueText) { - valueElement.dir = 'auto'; - } - // If not, assume it follows the locale's direction - else { - valueElement.dir = Zotero.dir; + if (lazy.BIDI_BROWSER_UI) { + // Attempt to guess text direction automatically + let language = this.item.getField('language'); + valueElement.dir = Zotero.ItemFields.getDirection( + this.item.itemTypeID, fieldName, language + ); } - // Regardless, align the text in the label consistently, following the locale's direction + // Regardless, align the text in unfocused fields consistently, following the locale's direction if (Zotero.rtl) { valueElement.style.textAlign = 'right'; } diff --git a/chrome/content/zotero/itemTree.jsx b/chrome/content/zotero/itemTree.jsx index 5aef911d2b..29c97f2886 100644 --- a/chrome/content/zotero/itemTree.jsx +++ b/chrome/content/zotero/itemTree.jsx @@ -35,6 +35,15 @@ const { getCSSIcon, getCSSItemTypeIcon } = Icons; const { COLUMNS } = require("zotero/itemTreeColumns"); const { Cc, Ci, Cu, ChromeUtils } = require('chrome'); const { OS } = ChromeUtils.importESModule("chrome://zotero/content/osfile.mjs"); +const { XPCOMUtils } = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm"); + +const lazy = {}; +XPCOMUtils.defineLazyPreferenceGetter( + lazy, + "BIDI_BROWSER_UI", + "bidi.browser.ui", + false +); /** * @typedef {import("./itemTreeColumns.jsx").ItemTreeColumnOptions} ItemTreeColumnOptions @@ -2851,7 +2860,11 @@ var ItemTree = class ItemTree extends LibraryTree { } let textSpanAriaLabel = [textWithFullStop, itemTypeAriaLabel, tagAriaLabel, retractedAriaLabel].join(' '); textSpan.className = "cell-text"; - textSpan.dir = 'auto'; + if (lazy.BIDI_BROWSER_UI) { + textSpan.dir = Zotero.ItemFields.getDirection( + item.itemTypeID, column.dataKey, item.getField('language') + ); + } textSpan.setAttribute('aria-label', textSpanAriaLabel); if (Zotero.Prefs.get('ui.tagsAfterTitle')) { diff --git a/chrome/content/zotero/xpcom/data/itemFields.js b/chrome/content/zotero/xpcom/data/itemFields.js index b35a40b2a9..29dcee2065 100644 --- a/chrome/content/zotero/xpcom/data/itemFields.js +++ b/chrome/content/zotero/xpcom/data/itemFields.js @@ -418,6 +418,68 @@ Zotero.ItemFields = new function() { } + /** + * Guess the text direction of a field, using the item's language field if available. + * + * @param {number} itemTypeID + * @param {string | number} field + * @param {string} [itemLanguage] + * @returns {'auto' | 'ltr' | 'rtl'} + */ + this.getDirection = function (itemTypeID, field, itemLanguage) { + field = this.getName(this.getBaseIDFromTypeAndField(itemTypeID, field) || field); + switch (field) { + // Certain fields containing IDs, numbers, and data: always LTR + case 'ISBN': + case 'ISSN': + case 'DOI': + case 'url': + case 'callNumber': + case 'volume': + case 'numberOfVolumes': + case 'issue': + case 'runningTime': + case 'number': + case 'versionNumber': + case 'applicationNumber': + case 'priorityNumbers': + case 'codeNumber': + case 'pages': + case 'numPages': + case 'seriesNumber': + case 'edition': + case 'citationKey': + case 'language': + case 'extra': + return 'ltr'; + // Primary fields: follow app locale + case 'dateAdded': + case 'dateModified': + case 'accessDate': + return Zotero.dir; + // Everything else: guess based on the language if we have one; otherwise auto + default: + if (itemLanguage) { + let languageCode = Zotero.Utilities.Item.languageToISO6391(itemLanguage); + try { + let locale = new Intl.Locale(languageCode).maximize(); + // https://www.w3.org/International/questions/qa-scripts#directions + // TODO: Remove this once Fx supports Intl.Locale#getTextInfo() + if (['Adlm', 'Arab', 'Aran', 'Rohg', 'Hebr', 'Mand', 'Mend', 'Nkoo', 'Hung', 'Samr', 'Syrc', 'Thaa', 'Yezi'] + .includes(locale.script)) { + return 'rtl'; + } + } + catch (e) { + Zotero.logError(e); + } + return 'ltr'; + } + return 'auto'; + } + }; + + /** * Check whether a field is valid, throwing an exception if not * (since it should never actually happen) diff --git a/scss/elements/_editableText.scss b/scss/elements/_editableText.scss index 01bf7a1959..5f00863bd1 100644 --- a/scss/elements/_editableText.scss +++ b/scss/elements/_editableText.scss @@ -93,6 +93,7 @@ editable-text { &:read-only, &:not(:focus) { appearance: none; background: transparent; + text-align: inherit; } &:hover:not(:read-only, :focus) {