Update tab titles independently of the reader (#4813)

Move handling of tabs' renaming when an item is
modified from Reader into Zotero_Tabs, so that the titles of
unloaded tabs can get properly renamed.

Have the Reader and Zotero_Tabs share Zotero.Item#getTabTitle()
to handle title updates in both tabs and standalone reader windows.

Fixes: #4646

chrome/content/zotero/tabs.js

@@ -84,6 +84,19 @@ var Zotero_Tabs = new function () {
 	this._tabsMenuFocusedIndex = 0;
 	this._tabsMenuIgnoreMouseover = false;
 
+	// Keep track of item modifications to update the title
+	Zotero.Notifier.registerObserver(this, ['item'], 'tabs');
+
+	// Update the title when pref of title format is changed
+	Zotero.Prefs.registerObserver('tabs.title.reader', async () => {
+		for (let tab of this._tabs) {
+			if (!tab.data.itemID) continue;
+			let item = Zotero.Items.get(tab.data.itemID);
+			let title = await item.getTabTitle();
+			this.rename(tab.id, title);
+		}
+	});
+
 	this._unloadInterval = setInterval(() => {
 		this.unloadUnusedTabs();
 	}, 60000); // Trigger every minute
@@ -174,6 +187,26 @@ var Zotero_Tabs = new function () {
 		);
 	};
 
+	// When an item is modified, update the title accordingly
+	this.notify = async (event, type, ids, _) => {
+		if (event !== "modify") return;
+		for (let id of ids) {
+			let item = Zotero.Items.get(id);
+			// If a top-level item is updated, update all tabs that have its attachments
+			// Otherwise, just update the tab with the updated attachment
+			let attachmentIDs = item.isAttachment() ? [id] : item.getAttachments();
+			for (let attachmentID of attachmentIDs) {
+				let attachment = Zotero.Items.get(attachmentID);
+				let relevantTabs = this._tabs.filter(tab => tab.data.itemID == attachmentID);
+				if (!relevantTabs.length) continue;
+				for (let tab of relevantTabs) {
+					let title = await attachment.getTabTitle();
+					this.rename(tab.id, title);
+				}
+			}
+		}
+	};
+
 	this.getState = function () {
 		return this._tabs.map((tab) => {
 			let type = tab.type;
@@ -263,6 +296,16 @@ var Zotero_Tabs = new function () {
 				this._prevSelectedID = previousID;
 			}
 		}
+		// When a new tab is opened synchronously by ReaderTab constructor, the title is empty.
+		// However, { id, container } needs to return immediately, so do not wait for the new title
+		// and construct it in async manner below.
+		if (!title && data.itemID) {
+			(async () => {
+				let item = Zotero.Items.get(data.itemID);
+				title = await item.getTabTitle();
+				this.rename(tab.id, title);
+			})();
+		}
 		return { id, container };
 	};
 

chrome/content/zotero/xpcom/data/item.js

@@ -992,6 +992,58 @@ Zotero.Item.prototype.updateDisplayTitle = function () {
 	this._displayTitle = title;
 };
 
+/**
+ * Get title for the reader tab of a given item accounting for "Show tabs as" pref
+ * @param {Number} itemID - itemID of the attachment
+ * @returns {String} title for the tab of this item
+ */
+Zotero.Item.prototype.getTabTitle = async function () {
+	if (!this.isAttachment()) {
+		throw new Error("Can only get tab title for attachments");
+	}
+	let type = Zotero.Prefs.get('tabs.title.reader');
+	let readerTitle = this.getDisplayTitle();
+	let parentItem = this.parentItem;
+	if (type === 'filename') {
+		readerTitle = this.attachmentFilename;
+	}
+	else if (parentItem) {
+		let attachment = await parentItem.getBestAttachment();
+		let isPrimaryAttachment = attachment && attachment.id == this.id;
+
+		let parts = [];
+		// Windows displays bidi control characters as placeholders in window titles, so strip them
+		// See https://github.com/mozilla-services/screenshots/issues/4863
+		let unformatted = Zotero.isWin;
+		let creator = parentItem.getField('firstCreator', unformatted);
+		let year = parentItem.getField('year');
+		if (year == '0000') {
+			year = '';
+		}
+		// Only include parent title if primary attachment
+		let title = isPrimaryAttachment ? parentItem.getDisplayTitle() : false;
+		// If creator is missing fall back to titleCreatorYear
+		if (type === 'creatorYearTitle' && creator) {
+			parts = [creator, year, title];
+		}
+		else if (type === 'title') {
+			parts = [title];
+		}
+		// If type is titleCreatorYear, or is missing, or another type falls back
+		else {
+			parts = [title, creator, year];
+		}
+
+		// If not primary attachment, show attachment title first
+		if (!isPrimaryAttachment) {
+			parts.unshift(this.getDisplayTitle());
+		}
+
+		readerTitle = parts.filter(Boolean).join(' - ');
+	}
+	return readerTitle;
+};
+
 
 /*
  * Returns the number of creators for this item

chrome/content/zotero/xpcom/reader.js

@@ -593,49 +593,8 @@ class ReaderInstance {
 	}
 
 	async updateTitle() {
-		let type = Zotero.Prefs.get('tabs.title.reader');
-		let item = Zotero.Items.get(this._item.id);
-		let readerTitle = item.getDisplayTitle();
-		let parentItem = item.parentItem;
-		if (type === 'filename') {
-			readerTitle = item.attachmentFilename;
-		}
-		else if (parentItem) {
-			let attachment = await parentItem.getBestAttachment();
-			let isPrimaryAttachment = attachment && attachment.id == item.id;
-
-			let parts = [];
-			// Windows displays bidi control characters as placeholders in window titles, so strip them
-			// See https://github.com/mozilla-services/screenshots/issues/4863
-			let unformatted = Zotero.isWin;
-			let creator = parentItem.getField('firstCreator', unformatted);
-			let year = parentItem.getField('year');
-			if (year == '0000') {
-				year = '';
-			}
-			// Only include parent title if primary attachment
-			let title = isPrimaryAttachment ? parentItem.getDisplayTitle() : false;
-			// If creator is missing fall back to titleCreatorYear
-			if (type === 'creatorYearTitle' && creator) {
-				parts = [creator, year, title];
-			}
-			else if (type === 'title') {
-				parts = [title];
-			}
-			// If type is titleCreatorYear, or is missing, or another type falls back
-			else {
-				parts = [title, creator, year];
-			}
-
-			// If not primary attachment, show attachment title first
-			if (!isPrimaryAttachment) {
-				parts.unshift(item.getDisplayTitle());
-			}
-
-			readerTitle = parts.filter(Boolean).join(' - ');
-		}
-		this._title = readerTitle;
-		this._setTitleValue(readerTitle);
+		this._title = await this._item.getTabTitle();
+		this._setTitleValue(this._title);
 	}
 
 	async setAnnotations(items) {
@@ -1360,9 +1319,7 @@ class ReaderTab extends ReaderInstance {
 		}
 	};
 
-	_setTitleValue(title) {
-		this._window.Zotero_Tabs.rename(this.tabID, title);
-	}
+	_setTitleValue() {}
 
 	_addToNote(annotations) {
 		annotations = annotations.map(x => ({ ...x, attachmentItemID: this._item.id }));