Lazy load attachment preview (#4568)

chrome/content/zotero/elements/attachmentBox.js

@@ -32,7 +32,6 @@
 		content = MozXULElement.parseXULToFragment(`
 			<collapsible-section data-l10n-id="section-attachment-info" data-pane="attachment-info">
 				<html:div class="body">
-					<attachment-preview id="attachment-preview" tabindex="0" data-l10n-id="attachment-preview"/>
 					<html:div style="display: grid;">
 						<label id="url" is="zotero-text-link" crop="end" tabindex="0"
 							ondragstart="let dt = event.dataTransfer; dt.setData('text/x-moz-url', this.value); dt.setData('text/uri-list', this.value); dt.setData('text/plain', this.value);"/>
@@ -76,6 +75,16 @@
 			</collapsible-section>
 		`);
 
+		_body = null;
+
+		_preview = null;
+
+		_lastPreviewRenderId = "";
+
+		_discardPreviewTimeout = 60000;
+
+		_previewDiscarded = false;
+
 		constructor() {
 			super();
 
@@ -91,7 +100,6 @@
 			this._item = null;
 
 			this._section = null;
-			this._preview = null;
 
 			this._asyncRendering = false;
 			
@@ -212,17 +220,25 @@
 			if (val.isAttachment()) {
 				this._item = val;
 				this.hidden = false;
-				this._preview.disableResize = false;
 			}
 			else {
 				this.hidden = true;
-				this._preview.disableResize = true;
 			}
+			if (this._preview) this._preview.disableResize = !!this.hidden;
+		}
+
+		get previewElem() {
+			if (!this._preview) {
+				this._initPreview();
+			}
+			return this._preview;
 		}
 
 		init() {
 			this.initCollapsibleSection();
 
+			this._body = this.querySelector('.body');
+
 			this._id('url').addEventListener('contextmenu', (event) => {
 				this._id('url-menu').openPopupAtScreen(event.screenX, event.screenY, true);
 			});
@@ -236,8 +252,6 @@
 				this._isEditingFilename = false;
 			});
 
-			this._preview = this._id("attachment-preview");
-
 			let noteButton = this._id('note-button');
 			noteButton.addEventListener("command", () => {
 				this.convertAttachmentNote();
@@ -301,7 +315,14 @@
 			if (!this.item) return;
 			if (this._asyncRendering) return;
 			if (!this._section.open) return;
-			if (this._isAlreadyRendered("async")) return;
+			if (this._isAlreadyRendered("async")) {
+				if (this._previewDiscarded) {
+					this._previewDiscarded = false;
+					this.previewElem.render();
+				}
+				this._lastPreviewRenderTime = Date.now();
+				return;
+			}
 
 			Zotero.debug('Refreshing attachment box');
 			this._asyncRendering = true;
@@ -450,11 +471,24 @@
 			}
 
 			if (this.usePreview) {
-				this._preview.item = this.item;
+				this.previewElem.item = this.item;
-				await this._preview.render();
+				await this.previewElem.render();
 			}
 
 			this._asyncRendering = false;
+
+			this._lastPreviewRenderTime = `${Date.now()}-${Math.random()}`;
+		}
+
+		discard() {
+			if (!this._preview) return;
+			let lastRenderTime = this._lastPreviewRenderId;
+			setTimeout(() => {
+				if (!this._asyncRendering && this._lastPreviewRenderId === lastRenderTime) {
+					this._preview?.discard();
+					this._previewDiscarded = true;
+				}
+			}, this._discardPreviewTimeout);
 		}
 
 		onViewClick(event) {
@@ -676,6 +710,14 @@
 			if (finished) resolve();
 			else reject(new Error("AttachmentBox#_waitForRender timeout"));
 		}
+
+		_initPreview() {
+			this._preview = document.createXULElement('attachment-preview');
+			this._preview.setAttribute('tabindex', '0');
+			this._preview.setAttribute('data-l10n-id', 'attachment-preview');
+			this._body.prepend(this._preview);
+			this._preview.disableResize = !!this.hidden;
+		}
 	}
 
 	customElements.define("attachment-box", AttachmentBox);

chrome/content/zotero/elements/attachmentsBox.js

@@ -30,7 +30,6 @@
 		content = MozXULElement.parseXULToFragment(`
 			<collapsible-section data-l10n-id="section-attachments" data-pane="attachments" extra-buttons="add">
 				<html:div class="body">
-					<attachment-preview tabindex="0" data-l10n-id="attachment-preview"/>
 					<html:div class="attachments-container"></html:div>
 				</html:div>
 			</collapsible-section>
@@ -45,8 +44,16 @@
 
 		_attachmentIDs = [];
 
+		_body = null;
+
 		_preview = null;
 
+		_lastPreviewRenderTime = "";
+
+		_discardPreviewTimeout = 60000;
+
+		_previewDiscarded = false;
+
 		get item() {
 			return this._item;
 		}
@@ -59,7 +66,7 @@
 			super.item = item;
 			let hidden = !item?.isRegularItem() || item?.isFeedItem;
 			this.hidden = hidden;
-			this._preview.disableResize = !!hidden;
+			if (this._preview) this._preview.disableResize = !!hidden;
 		}
 
 		get inTrash() {
@@ -81,10 +88,19 @@
 			}
 		}
 
+		get previewElem() {
+			if (!this._preview) {
+				this._initPreview();
+			}
+			return this._preview;
+		}
+
 		init() {
 			this.initCollapsibleSection();
 			this._section.addEventListener('add', this._handleAdd);
 
+			this._body = this.querySelector('.body');
+
 			this._attachments = this.querySelector('.attachments-container');
 			
 			this._addPopup = this.querySelector('.add-popup');
@@ -106,7 +122,6 @@
 			});
 			
 			this.usePreview = Zotero.Prefs.get('showAttachmentPreview');
-			this._preview = this.querySelector('attachment-preview');
 
 			this._notifierID = Zotero.Notifier.registerObserver(this, ['item'], 'attachmentsBox');
 
@@ -169,7 +184,14 @@
 
 		async asyncRender() {
 			if (!this._item) return;
-			if (this._isAlreadyRendered("async")) return;
+			if (this._isAlreadyRendered("async")) {
+				if (this._previewDiscarded) {
+					this._previewDiscarded = false;
+					this.previewElem.render();
+				}
+				this._lastPreviewRenderTime = `${Date.now()}-${Math.random()}`;
+				return;
+			}
 			this._renderStage = "final";
 			this._asyncRendering = true;
 			
@@ -184,6 +206,17 @@
 			await this.updatePreview();
 			this._asyncRendering = false;
 		}
+
+		discard() {
+			if (!this._preview) return;
+			let lastRenderTime = this._lastPreviewRenderTime;
+			setTimeout(() => {
+				if (!this._asyncRendering && this._lastPreviewRenderTime === lastRenderTime) {
+					this._preview?.discard();
+					this._previewDiscarded = true;
+				}
+			}, this._discardPreviewTimeout);
+		}
 		
 		updateCount() {
 			if (!this._item?.isRegularItem()) {
@@ -211,8 +244,9 @@
 				|| (this._attachmentIDs.length && !this._section.open)) {
 				return;
 			}
-			this._preview.item = attachment;
+			this.previewElem.item = attachment;
-			await this._preview.render();
+			await this.previewElem.render();
+			this._lastPreviewRenderTime = `${Date.now()}-${Math.random()}`;
 		}
 
 		async _getPreviewAttachment() {
@@ -225,6 +259,14 @@
 			return attachment;
 		}
 
+		_initPreview() {
+			this._preview = document.createXULElement('attachment-preview');
+			this._preview.setAttribute('tabindex', '0');
+			this._preview.setAttribute('data-l10n-id', 'attachment-preview');
+			this._body.prepend(this._preview);
+			this._preview.disableResize = !!this.hidden;
+		}
+
 		_handleAdd = (event) => {
 			this._section.open = true;
 			this._addPopup.openPopup(event.detail.button, 'after_end');
@@ -238,7 +280,7 @@
 			menu.dataset.l10nArgs = `{ "type": "${this.usePreview ? "open" : "collapsed"}" }`;
 			
 			if (toOpen) {
-				this._preview.render();
+				this.previewElem.render();
 			}
 		};
 

chrome/content/zotero/elements/itemDetails.js

@@ -275,11 +275,13 @@
 				box.tabType = this.tabType;
 				box.item = item;
 				box.collectionTreeRow = this.collectionTreeRow;
+				// Discard hidden panes
+				if (box.hidden && box.discard) {
+					box.discard();
+				}
 				// Execute sync render immediately
 				if (!box.hidden && box.render) {
-					if (box.render) {
+					box.render();
-						box.render();
-					}
 				}
 			}
 
@@ -604,7 +606,8 @@
 			let needsDiscard = [];
 			entries.forEach((entry) => {
 				let targetPaneElem = entry.target;
-				if (entry.isIntersecting && targetPaneElem.render) {
+				if (entry.isIntersecting
+						&& (targetPaneElem.render || targetPaneElem.asyncRender)) {
 					needsRefresh.push(targetPaneElem);
 				}
 				else if (targetPaneElem.discard) {

test/tests/itemPaneTest.js

@@ -12,7 +12,8 @@ describe("Item pane", function () {
 	}
 
 	async function waitForPreviewBoxReader(box, itemID) {
-		let preview = box._preview;
+		let preview = await getBoxPreview(box);
+		if (!preview) return false;
 		await waitForPreviewBoxRender(box);
 		let res = await waitForCallback(
 			() => preview._reader?.itemID == itemID
@@ -25,9 +26,26 @@ describe("Item pane", function () {
 		return true;
 	}
 
-	function isPreviewDisplayed(box) {
+	async function isPreviewDisplayed(box) {
-		return !!(box._preview.hasPreview
+		let preview = await getBoxPreview(box);
-			&& win.getComputedStyle(box._preview).display !== "none");
+		if (!preview) return false;
+		return !!(preview.hasPreview
+			&& win.getComputedStyle(preview).display !== "none");
+	}
+
+	async function getBoxPreview(box) {
+		try {
+			// Since we are lazy loading the preview, should wait for the preview to be initialized
+			await waitForCallback(
+				() => !!box._preview
+				, 10, 0.5);
+		}
+		catch (e) {
+			Zotero.logError(e);
+			// Return false if waitForCallback fails
+			return false;
+		}
+		return box._preview;
 	}
 	
 	before(function* () {
@@ -497,13 +515,14 @@ describe("Item pane", function () {
 			});
 			await ZoteroPane.viewItems([attachment]);
 			let tabID = Zotero_Tabs.selectedID;
+			ZoteroContextPane.splitter.setAttribute("state", "open");
 			let itemDetails = ZoteroContextPane.context._getItemContext(tabID);
 			let attachmentsBox = itemDetails.getPane(paneID);
 			assert.isFalse(attachmentsBox.hidden);
 
 			await waitForScrollToPane(itemDetails, paneID);
 			
-			assert.isFalse(isPreviewDisplayed(attachmentsBox));
+			assert.isFalse(await isPreviewDisplayed(attachmentsBox));
 		});
 
 		it("should not show attachments pane in reader standalone attachment item", async function () {
@@ -541,7 +560,7 @@ describe("Item pane", function () {
 
 			await waitForScrollToPane(itemDetails, paneID);
 			await waitForPreviewBoxRender(attachmentsBox);
-			assert.isTrue(isPreviewDisplayed(attachmentsBox));
+			assert.isTrue(await isPreviewDisplayed(attachmentsBox));
 		});
 
 		it("should not render attachments pane preview when show preview is disabled", async function () {
@@ -556,7 +575,7 @@ describe("Item pane", function () {
 
 			await waitForScrollToPane(itemDetails, paneID);
 
-			assert.isFalse(isPreviewDisplayed(attachmentsBox));
+			assert.isFalse(await isPreviewDisplayed(attachmentsBox));
 		});
 
 		it("should only render after attachments pane becomes visible", async function () {
@@ -566,7 +585,7 @@ describe("Item pane", function () {
 
 			let itemDetails = ZoteroPane.itemPane._itemDetails;
 			let attachmentsBox = itemDetails.getPane(paneID);
-			let preview = attachmentsBox._preview;
+			let preview = attachmentsBox.previewElem;
 			// Force discard previous preview
 			await preview.discard(true);
 			
@@ -584,7 +603,7 @@ describe("Item pane", function () {
 			// Do not use _isAlreadyRendered, since that changes the render flag state
 			assert.equal(attachmentsBox._syncRenderItemID, item.id);
 			assert.notEqual(attachmentsBox._asyncRenderItemID, item.id);
-			assert.isFalse(isPreviewDisplayed(attachmentsBox));
+			assert.isFalse(await isPreviewDisplayed(attachmentsBox));
 
 			await waitForScrollToPane(itemDetails, paneID);
 			await waitForPreviewBoxRender(attachmentsBox);
@@ -594,7 +613,7 @@ describe("Item pane", function () {
 			assert.equal(attachmentsBox._syncRenderItemID, item.id);
 			assert.equal(attachmentsBox._asyncRenderItemID, item.id);
 
-			assert.isTrue(isPreviewDisplayed(attachmentsBox));
+			assert.isTrue(await isPreviewDisplayed(attachmentsBox));
 			assert.isTrue(preview.hasPreview);
 			win.resizeTo(null, height);
 		});
@@ -604,7 +623,7 @@ describe("Item pane", function () {
 
 			let itemDetails = ZoteroPane.itemPane._itemDetails;
 			let attachmentsBox = itemDetails.getPane(paneID);
-			let preview = attachmentsBox._preview;
+			let preview = attachmentsBox.previewElem;
 			// Force discard previous preview
 			await preview.discard(true);
 
@@ -617,7 +636,7 @@ describe("Item pane", function () {
 			await ZoteroPane.selectItem(item.id);
 			assert.isTrue(await waitForPreviewBoxRender(attachmentsBox));
 			// No preview
-			assert.isFalse(isPreviewDisplayed(attachmentsBox));
+			assert.isFalse(await isPreviewDisplayed(attachmentsBox));
 			// No row
 			assert.equal(attachmentsBox.querySelectorAll("attachment-row").length, 0);
 
@@ -632,7 +651,7 @@ describe("Item pane", function () {
 			await itemDetails._renderPromise;
 			await waitForPreviewBoxRender(attachmentsBox);
 			// Image preview for item with image attachment
-			assert.isTrue(isPreviewDisplayed(attachmentsBox));
+			assert.isTrue(await isPreviewDisplayed(attachmentsBox));
 			assert.equal(preview.previewType, "image");
 			// 1 row
 			assert.equal(attachmentsBox.querySelectorAll("attachment-row").length, 1);
@@ -647,7 +666,7 @@ describe("Item pane", function () {
 			await waitForPreviewBoxReader(attachmentsBox, attachment2.id);
 			await Zotero.Promise.delay(100);
 			// PDF preview
-			assert.isTrue(isPreviewDisplayed(attachmentsBox));
+			assert.isTrue(await isPreviewDisplayed(attachmentsBox));
 			assert.equal(preview.previewType, "pdf");
 			// 2 rows
 			assert.equal(attachmentsBox.querySelectorAll("attachment-row").length, 2);
@@ -690,7 +709,7 @@ describe("Item pane", function () {
 			await attachment2.eraseTx();
 			await Zotero.Promise.delay(100);
 			// Image preview for item with image attachment
-			assert.isTrue(isPreviewDisplayed(attachmentsBox));
+			assert.isTrue(await isPreviewDisplayed(attachmentsBox));
 			assert.equal(preview.previewType, "image");
 			// 1 row
 			assert.equal(attachmentsBox.querySelectorAll("attachment-row").length, 1);
@@ -727,7 +746,7 @@ describe("Item pane", function () {
 			assert.isFalse(attachmentsBox.hidden);
 
 			await waitForScrollToPane(itemDetails, paneID);
-			assert.isFalse(isPreviewDisplayed(attachmentsBox));
+			assert.isFalse(await isPreviewDisplayed(attachmentsBox));
 
 			// Select library tab
 			Zotero_Tabs.select("zotero-pane");
@@ -746,7 +765,7 @@ describe("Item pane", function () {
 			Zotero_Tabs.select(tabID);
 
 			// Make sure the preview status is not changed in reader
-			assert.isFalse(isPreviewDisplayed(attachmentsBox));
+			assert.isFalse(await isPreviewDisplayed(attachmentsBox));
 		});
 
 		/**
@@ -758,7 +777,7 @@ describe("Item pane", function () {
 		it("should keep attachments pane status after changing selection", async function () {
 			let itemDetails = ZoteroPane.itemPane._itemDetails;
 			let attachmentsBox = itemDetails.getPane(paneID);
-			let preview = attachmentsBox._preview;
+			let preview = attachmentsBox.previewElem;
 
 			// Pin the pane to avoid always scrolling to the section
 			itemDetails.pinnedPane = paneID;
@@ -898,7 +917,7 @@ describe("Item pane", function () {
 		it("should open attachment on double-clicking attachments pane preview", async function () {
 			let itemDetails = ZoteroPane.itemPane._itemDetails;
 			let attachmentsBox = itemDetails.getPane(paneID);
-			let preview = attachmentsBox._preview;
+			let preview = attachmentsBox.previewElem;
 
 			let item = new Zotero.Item('book');
 			await item.saveTx();
@@ -928,7 +947,7 @@ describe("Item pane", function () {
 		it("should render preview robustly after making dense calls to render and discard", async function () {
 			let itemDetails = ZoteroPane.itemPane._itemDetails;
 			let attachmentsBox = itemDetails.getPane(paneID);
-			let preview = attachmentsBox._preview;
+			let preview = attachmentsBox.previewElem;
 
 			// Pin the pane to avoid always scrolling to the section
 			itemDetails.pinnedPane = paneID;
@@ -970,14 +989,77 @@ describe("Item pane", function () {
 			// Should be able to render the correct preview
 			await ZoteroPane.selectItem(item1.id);
 			await waitForPreviewBoxReader(attachmentsBox, attachment1.id);
-			assert.isTrue(isPreviewDisplayed(attachmentsBox));
+			assert.isTrue(await isPreviewDisplayed(attachmentsBox));
 
 			await ZoteroPane.selectItem(item2.id);
 			await waitForPreviewBoxReader(attachmentsBox, attachment2.id);
-			assert.isTrue(isPreviewDisplayed(attachmentsBox));
+			assert.isTrue(await isPreviewDisplayed(attachmentsBox));
 
 			itemDetails.pinnedPane = "";
 		});
+
+		it("should not load preview iframe before becoming visible", async function () {
+			let itemDetails = ZoteroPane.itemPane._itemDetails;
+			let attachmentsBox = itemDetails.getPane(paneID);
+
+			// Resize to very small height to ensure the attachment box is not in view
+			let height = doc.documentElement.clientHeight;
+			win.resizeTo(null, 100);
+
+			// Remove any existing preview to ensure the test is valid
+			attachmentsBox._preview?.remove();
+			attachmentsBox._preview = null;
+
+			let item = await createDataObject('item');
+			await importFileAttachment('test.pdf', { parentID: item.id });
+
+			await ZoteroPane.selectItem(item.id);
+
+			assert.notExists(attachmentsBox._preview);
+			assert.notExists(attachmentsBox.querySelector("#preview"));
+
+			await waitForScrollToPane(itemDetails, paneID);
+			await waitForPreviewBoxRender(attachmentsBox);
+			
+			assert.exists(await getBoxPreview(attachmentsBox));
+
+			win.resizeTo(null, height);
+		});
+
+		it("should discard attachments pane preview after becoming invisible", async function () {
+			let itemDetails = ZoteroPane.itemPane._itemDetails;
+			let attachmentsBox = itemDetails.getPane(paneID);
+
+			// Resize to very small height to ensure the attachment box is not in view
+			let height = doc.documentElement.clientHeight;
+			win.resizeTo(null, 100);
+
+			const discardTimeout = 50;
+
+			// Temporarily set discard timeout to 100ms for testing
+			let currentDiscardTimeout = attachmentsBox._discardPreviewTimeout;
+			attachmentsBox._discardPreviewTimeout = discardTimeout;
+
+			let item = await createDataObject('item');
+			await importFileAttachment('test.pdf', { parentID: item.id });
+
+			await ZoteroPane.selectItem(item.id);
+			await waitForScrollToPane(itemDetails, paneID);
+			await waitForPreviewBoxRender(attachmentsBox);
+
+			assert.isTrue(attachmentsBox._preview._isReaderInitialized);
+			
+			// Scroll the attachments pane out of view
+			await waitForScrollToPane(itemDetails, 'info');
+
+			// Wait a bit for the preview to be discarded
+			await Zotero.Promise.delay(discardTimeout + 100);
+			
+			assert.isFalse(attachmentsBox._preview._isReaderInitialized);
+
+			win.resizeTo(null, height);
+			attachmentsBox._discardPreviewTimeout = currentDiscardTimeout;
+		});
 	});
 	
 	
@@ -1147,6 +1229,11 @@ describe("Item pane", function () {
 			await item.saveTx();
 			
 			await promise;
+
+			// Wait for section to finish rendering
+			let box = ZoteroPane.itemPane._itemDetails.getPane(paneID);
+			await waitForPreviewBoxRender(box);
+			
 			assert.equal(label.value, newTitle);
 		});
 
@@ -1154,6 +1241,11 @@ describe("Item pane", function () {
 			// Regular item: hide
 			let itemDetails = ZoteroPane.itemPane._itemDetails;
 			let box = itemDetails.getPane(paneID);
+
+			// TEMP: Force abort any pending renders
+			box._preview?.remove();
+			box._preview = null;
+
 			let item = new Zotero.Item('book');
 			await item.saveTx();
 			await ZoteroPane.selectItem(item.id);
@@ -1172,7 +1264,7 @@ describe("Item pane", function () {
 			await waitForPreviewBoxReader(box, attachment.id);
 			assert.isFalse(box.hidden);
 			await Zotero.Promise.delay(100);
-			assert.isTrue(isPreviewDisplayed(box));
+			assert.isTrue(await isPreviewDisplayed(box));
 
 			// Standalone attachment: show
 			let attachment1 = await importFileAttachment('test.pdf');
@@ -1181,7 +1273,7 @@ describe("Item pane", function () {
 			await waitForPreviewBoxReader(box, attachment1.id);
 			assert.isFalse(box.hidden);
 			await Zotero.Promise.delay(100);
-			assert.isTrue(isPreviewDisplayed(box));
+			assert.isTrue(await isPreviewDisplayed(box));
 		});
 
 		it("should show attachment pane without preview in reader for standalone attachment item", async function () {
@@ -1210,7 +1302,7 @@ describe("Item pane", function () {
 
 			await waitForScrollToPane(itemDetails, paneID);
 			// No preview
-			assert.isFalse(isPreviewDisplayed(box));
+			assert.isFalse(await isPreviewDisplayed(box));
 		});
 
 		it("should only show attachment note container when exists", async function () {
@@ -1238,6 +1330,36 @@ describe("Item pane", function () {
 			// Should be readonly
 			assert.equal(noteEditor.mode, "view");
 		});
+
+		it("should discard attachment pane preview after becoming invisible", async function () {
+			let itemDetails = ZoteroPane.itemPane._itemDetails;
+			let attachmentBox = itemDetails.getPane(paneID);
+
+			const discardTimeout = 50;
+
+			// Temporarily set discard timeout to 100ms for testing
+			let currentDiscardTimeout = attachmentBox._discardPreviewTimeout;
+			attachmentBox._discardPreviewTimeout = discardTimeout;
+
+			let item = await createDataObject('item');
+			let attachment = await importFileAttachment('test.pdf', { parentID: item.id });
+
+			await ZoteroPane.selectItem(attachment.id);
+			await waitForScrollToPane(itemDetails, paneID);
+			await waitForPreviewBoxRender(attachmentBox);
+
+			assert.isTrue(attachmentBox._preview._isReaderInitialized);
+			
+			// Select a regular item to hide the attachment pane
+			await ZoteroPane.selectItem(item.id);
+
+			// Wait a bit for the preview to be discarded
+			await Zotero.Promise.delay(discardTimeout + 100);
+			
+			assert.isFalse(attachmentBox._preview._isReaderInitialized);
+
+			attachmentBox._discardPreviewTimeout = currentDiscardTimeout;
+		});
 	});