From 3f45def928633be3dd4f5d8fcbedb6b589b46236 Mon Sep 17 00:00:00 2001 From: abaevbog Date: Thu, 20 Jun 2024 05:41:14 -0400 Subject: [PATCH] tabs startup loading optimization (#4174) - if an unloaded tab is being opened, do not close the tab and have reader re-open a fresh tab. Instead, keep the tab as is, have reader use the tab as a container and just change the tab's type. It should fix a glitch on windows when if you click on a tab during initial loading, it will disappear until reader re-adds it. A few tweaks to allow unloaded tabs be generally "selectable". Fixes: #4149 - during startup loading, select the yet-unloaded tab right away. That way, a tab is almost immediately selected, instead of being stuck on the library tab until the reader is ready. - if the items are not loaded yet, use a placeholder icon for tabs. Fixes: #4150 - special handling for reader loading message during initial load. When an unloaded tab is selected, the loading message needs to be displayed but, since there is no reader yet, we add it in Zotero_Tabs.select and try to remove by the reader when it's loaded. --- chrome/content/zotero/elements/contextPane.js | 10 +-- chrome/content/zotero/tabs.js | 63 ++++++++++--------- chrome/content/zotero/xpcom/reader.js | 40 +++++++----- 3 files changed, 61 insertions(+), 52 deletions(-) diff --git a/chrome/content/zotero/elements/contextPane.js b/chrome/content/zotero/elements/contextPane.js index 75df9412d3..6b8ee474d8 100644 --- a/chrome/content/zotero/elements/contextPane.js +++ b/chrome/content/zotero/elements/contextPane.js @@ -171,7 +171,7 @@ ZoteroContextPane.showLoadingMessage(false); this._sidenav.hidden = true; } - else if (Zotero_Tabs.selectedType == 'reader') { + else if (Zotero_Tabs.selectedType.includes('reader')) { let reader = Zotero.Reader.getByTabID(Zotero_Tabs.selectedID); this._handleReaderReady(reader); @@ -195,9 +195,6 @@ if (!reader) { return; } - ZoteroContextPane.showLoadingMessage(true); - await reader._initPromise; - ZoteroContextPane.showLoadingMessage(false); // Focus reader pages view if context pane note editor is not selected if (Zotero_Tabs.selectedID == reader.tabID && !Zotero_Tabs.isTabsMenuVisible() @@ -305,9 +302,8 @@ itemDetails.sidenav = this._sidenav; if (previousPinnedPane) itemDetails.pinnedPane = previousPinnedPane; - // `unloaded` tabs are never selected and shouldn't be rendered on creation. - // Use `includes` here for forward compatibility. - if (!tabType.includes("unloaded")) { + // Make sure that the context pane of the selected tab is rendered + if (tabID == Zotero_Tabs.selectedID) { this._selectItemContext(tabID); } } diff --git a/chrome/content/zotero/tabs.js b/chrome/content/zotero/tabs.js index c15465ee75..8d6b2b1a73 100644 --- a/chrome/content/zotero/tabs.js +++ b/chrome/content/zotero/tabs.js @@ -108,7 +108,9 @@ var Zotero_Tabs = new function () { icon = ; } catch (e) { - // item might not yet be loaded, we will get the icon on the next update + // item might not yet be loaded, we will get the right icon on the next update + // but until then use a default placeholder + icon = ; } } @@ -199,25 +201,13 @@ var Zotero_Tabs = new function () { } else if (tab.type === 'reader') { if (Zotero.Items.exists(tab.data.itemID)) { - if (tab.selected) { - Zotero.Reader.open(tab.data.itemID, - null, - { - title: tab.title, - tabIndex: i, - openInBackground: !tab.selected, - secondViewState: tab.data.secondViewState - } - ); - } - else { - this.add({ - type: 'reader-unloaded', - title: tab.title, - index: i, - data: tab.data - }); - } + this.add({ + type: 'reader-unloaded', + title: tab.title, + index: i, + data: tab.data, + select: tab.selected + }); } } } @@ -462,16 +452,22 @@ var Zotero_Tabs = new function () { selectedTab.lastFocusedElement = document.activeElement; } if (tab.type === 'reader-unloaded') { - this.close(tab.id); - Zotero.Reader.open(tab.data.itemID, options && options.location, { - tabID: tab.id, - title: tab.title, - tabIndex, - allowDuplicate: true, - secondViewState: tab.data.secondViewState, - preventJumpback: true - }); - return; + // Make sure the loading message is displayed first. + // Then, open reader and hide the loading message once it has loaded. + ZoteroContextPane.showLoadingMessage(true); + let hideMessageWhenReaderLoaded = async () => { + let reader = await Zotero.Reader.open(tab.data.itemID, options && options.location, { + tabID: tab.id, + title: tab.title, + tabIndex, + allowDuplicate: true, + secondViewState: tab.data.secondViewState, + preventJumpback: true + }); + await reader._initPromise; + ZoteroContextPane.showLoadingMessage(false); + }; + hideMessageWhenReaderLoaded(); } this._prevSelectedID = reopening ? this._selectedID : null; this._selectedID = id; @@ -532,6 +528,13 @@ var Zotero_Tabs = new function () { data: tab.data }); }; + + // Mark a tab as loaded + this.markAsLoaded = function (id) { + let { tab } = this._getTab(id); + if (!tab) return; + tab.type = "reader"; + }; this.unloadUnusedTabs = function () { for (let tab of this._tabs) { diff --git a/chrome/content/zotero/xpcom/reader.js b/chrome/content/zotero/xpcom/reader.js index 9156502bd7..2581396ffb 100644 --- a/chrome/content/zotero/xpcom/reader.js +++ b/chrome/content/zotero/xpcom/reader.js @@ -520,7 +520,6 @@ class ReaderInstance { }, this._iframeWindow, { cloneFunctions: true })); this._resolveInitPromise(); - // Set title once again, because `ReaderWindow` isn't loaded the first time this.updateTitle(); @@ -1017,19 +1016,28 @@ class ReaderTab extends ReaderInstance { this._onToggleSidebarCallback = options.onToggleSidebar; this._onChangeSidebarWidthCallback = options.onChangeSidebarWidth; this._window = Services.wm.getMostRecentWindow('navigator:browser'); - let { id, container } = this._window.Zotero_Tabs.add({ - id: options.tabID, - type: 'reader', - title: options.title || '', - index: options.index, - data: { - itemID: this._item.id - }, - select: !options.background, - preventJumpback: options.preventJumpback - }); - this.tabID = id; - this._tabContainer = container; + let existingTabID = this._window.Zotero_Tabs.getTabIDByItemID(this._item.id); + // If an unloaded tab for this item already exists, load the reader in it. + // Otherwise, create a new tab + if (existingTabID) { + this.tabID = existingTabID; + this._tabContainer = this._window.document.getElementById(existingTabID); + } + else { + let { id, container } = this._window.Zotero_Tabs.add({ + id: options.tabID, + type: 'reader', + title: options.title || '', + index: options.index, + data: { + itemID: this._item.id + }, + select: !options.background, + preventJumpback: options.preventJumpback + }); + this.tabID = id; + this._tabContainer = container; + } this._iframe = this._window.document.createXULElement('browser'); this._iframe.setAttribute('class', 'reader'); @@ -1737,6 +1745,9 @@ class Reader { async open(itemID, location, { title, tabIndex, tabID, openInBackground, openInWindow, allowDuplicate, secondViewState, preventJumpback } = {}) { let { libraryID } = Zotero.Items.getLibraryAndKeyFromID(itemID); let library = Zotero.Libraries.get(libraryID); + let win = Zotero.getMainWindow(); + // Change tab's type from "unloaded-reader" to "reader" + win.Zotero_Tabs.markAsLoaded(tabID); await library.waitForDataLoad('item'); let item = Zotero.Items.get(itemID); @@ -1747,7 +1758,6 @@ class Reader { this._loadSidebarState(); this.triggerAnnotationsImportCheck(itemID); let reader; - let win = Zotero.getMainWindow(); // If duplicating is not allowed, and no reader instance is loaded for itemID, // try to find an unloaded tab and select it. Zotero.Reader.open will then be called again if (!allowDuplicate && !this._readers.find(r => r.itemID === itemID)) {