Bidi improvements and fixes (#4534)

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;
 			}
 		}
 		

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';
 			}

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')) {

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)

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) {