From 1d2e2bb1e7086a168f589864170c904ba2680c6b Mon Sep 17 00:00:00 2001 From: Bogdan Abaev Date: Fri, 7 Aug 2026 14:09:12 -0700 Subject: [PATCH 1/2] citation dlg: suggest page locator from opened doc Display a suggestion popup with the current page label after an open document item is added as a bubble. Unloaded tabs do not have an opened reader, so the suggestion does not apply to them. The suggestion popup appears below the bubble-input and can be accepted via click or arrow down + Enter. When the popup is visible, the first keypress dismisses the popup. The popup does not consume keypress, except for Escape, so one can continue typing. Fixes: #6001 --- .../zotero/integration/citationDialog.js | 34 ++++++ .../zotero/integration/citationDialog.xhtml | 3 + .../integration/citationDialog/helpers.mjs | 27 +++++ .../citationDialog/keyboardHandler.mjs | 50 +++++++++ .../citationDialog/popupHandler.mjs | 72 ++++++++++++ chrome/content/zotero/xpcom/reader.js | 13 +++ scss/components/_citationDialog.scss | 24 ++++ test/tests/citationDialogTest.js | 103 +++++++++++++++++- 8 files changed, 325 insertions(+), 1 deletion(-) diff --git a/chrome/content/zotero/integration/citationDialog.js b/chrome/content/zotero/integration/citationDialog.js index 9972e3b36a..52c0db9776 100644 --- a/chrome/content/zotero/integration/citationDialog.js +++ b/chrome/content/zotero/integration/citationDialog.js @@ -1274,6 +1274,8 @@ const IOManager = { doc.addEventListener("focus-item-tree", ({ detail }) => this.focusItemTree(detail)); // update bubbles after citation item is updated by itemDetails popup doc.addEventListener("item-details-updated", () => this.updateBubbleInput()); + // add the page suggested from an opened reader tab as a locator + doc.addEventListener("page-suggestion-accepted", ({ detail: { page } }) => this._applyPageSuggestion(page)); doc.addEventListener("DOMMenuBarActive", () => this._handleMenuBarAppearance()); @@ -1434,6 +1436,9 @@ const IOManager = { // Add entries into the citation with the current locator if specified let bubbleItems = items.map(item => BubbleItem.fromItem(item)); + // Page of a reader tab with an attachment of the added item to be + // suggested as a locator once the bubble is added + let pageSuggestion = null; if (locator) { for (let bubbleItem of bubbleItems) { bubbleItem.locator = locator.locator; @@ -1450,6 +1455,10 @@ const IOManager = { // that, the user presumably knows about the shortcut _id("bubble-input").showJustAddedPlaceholder = DIALOG_STATE.isCitingItems() && this._timesItemsAdded < 1; + // If the item is opened in a reader tab, suggest its current page as a locator + if (DIALOG_STATE.isCitingItems()) { + pageSuggestion = Helpers.getOpenTabPage(items[0]); + } } else { // A multi-item add doesn't enter locator-typing mode, so typed text @@ -1466,12 +1475,14 @@ const IOManager = { this.updateBubbleInput(); // Show guidance panel on the first run + let guidanceShown = false; if (DIALOG_STATE.isCitingItems() && !Zotero.Prefs.get("firstRunGuidanceShown.citationDialog")) { doc.querySelector(".bubble").id = "first-bubble"; // Center the panel on the first bubble let width = doc.querySelector(".bubble").getBoundingClientRect().width; doc.querySelector("guidance-panel").setAttribute("x", Math.round(width / 2)); IOManager.showFirstRunDialog(); + guidanceShown = true; } // Render the preview before refreshing the list so resizeWindow measures its real height; // otherwise the debounced render lands after the resize and overflows the window. @@ -1481,6 +1492,14 @@ const IOManager = { if (!noInputRefocus) { _id("bubble-input").refocusInput(); } + // Suggest the page of the opened document, unless the first run guidance panel + // is already displayed next to the same bubble + if (pageSuggestion && !guidanceShown) { + PopupsHandler.openPageSuggestion({ + page: pageSuggestion, + anchor: _id("bubble-input").getCurrentInput() + }); + } dialogNotPristine(); }, @@ -1910,6 +1929,19 @@ const IOManager = { _processNumericLocatorInputDebounced: Zotero.Utilities.debounce(() => IOManager._processNumericLocatorInput(), NUMERIC_LOCATOR_TIMEOUT), + // Add the page suggested from an opened reader tab as a locator of the just-added bubble + _applyPageSuggestion(page) { + if (!this._justAddedBubbles) return; + for (let bubbleItem of this._justAddedBubbles) { + bubbleItem.locator = page; + bubbleItem.label = "page"; + } + // Clearing just-added bubbles refreshes bubble-input + this._clearJustAddedBubbles(); + _id("bubble-input").refocusInput(); + dialogNotPristine(); + }, + // Clear the record of which bubbles were just added. If a locator is typed // and Enter is presses, just-added bubbles get that locator. _clearJustAddedBubbles(event) { @@ -1926,6 +1958,8 @@ const IOManager = { // clear just added bubbles and update bubble input to reflect that this._justAddedBubbles = null; _id("bubble-input").showJustAddedPlaceholder = false; + // the page suggestion has no bubble to apply the locator to anymore + PopupsHandler.closePageSuggestion(); this.updateBubbleInput(); }, diff --git a/chrome/content/zotero/integration/citationDialog.xhtml b/chrome/content/zotero/integration/citationDialog.xhtml index 6cd90058a6..5d5e5db0c1 100644 --- a/chrome/content/zotero/integration/citationDialog.xhtml +++ b/chrome/content/zotero/integration/citationDialog.xhtml @@ -197,6 +197,9 @@ + +
+ diff --git a/chrome/content/zotero/integration/citationDialog/helpers.mjs b/chrome/content/zotero/integration/citationDialog/helpers.mjs index 8c9a207ea8..1552a96e14 100644 --- a/chrome/content/zotero/integration/citationDialog/helpers.mjs +++ b/chrome/content/zotero/integration/citationDialog/helpers.mjs @@ -506,4 +506,31 @@ export class CitationDialogHelpers { resolve(); }, delay); } + + + // Get the page currently displayed by a reader tab with an attachment of the given + // top-level item. If the item is open in multiple tabs, the selected tab wins. + // Returns null if the item is not open in any reader tab or if the reader has no + // pages (e.g. a snapshot). + getOpenTabPage(item) { + if (!item) return null; + let win = Zotero.getMainWindow(); + if (!win) return null; + let tabIDs = item.getAttachments() + .map(attachmentID => win.Zotero_Tabs.getTabIDByItemID(attachmentID)) + .filter(Boolean); + // Look at the selected tab first + tabIDs.sort((a, b) => { + if (a === win.Zotero_Tabs.selectedID) return -1; + if (b === win.Zotero_Tabs.selectedID) return 1; + return 0; + }); + for (let tabID of tabIDs) { + let reader = Zotero.Reader.getByTabID(tabID); + if (!reader) continue; + let page = reader.getCurrentPage(); + if (page) return page; + } + return null; + } } diff --git a/chrome/content/zotero/integration/citationDialog/keyboardHandler.mjs b/chrome/content/zotero/integration/citationDialog/keyboardHandler.mjs index 0928ac5cd8..8a716a09ec 100644 --- a/chrome/content/zotero/integration/citationDialog/keyboardHandler.mjs +++ b/chrome/content/zotero/integration/citationDialog/keyboardHandler.mjs @@ -48,6 +48,10 @@ export class CitationDialogKeyboardHandler { // capturing keydown listener to handle keypresses regardless of if they are handled by // lower-level components captureKeydown(event) { + // Page suggestion popup takes priority over all other keyboard handling. + // It has to be handled during the capture phase, since bubble-input handles + // Enter on its own inputs before the event reaches the document. + if (this._handlePageSuggestionKeydown(event)) return; let cmdOrCtrl = Zotero.isMac ? event.metaKey : event.ctrlKey; // Cmd/Ctrl-Enter will always accept the dialog regardless of the target if (event.key == "Enter" && cmdOrCtrl) { @@ -83,6 +87,52 @@ export class CitationDialogKeyboardHandler { } } + // Handle keypresses while the page suggestion popup is opened. ArrowDown highlights + // the suggestion and Enter then adds it as a locator. Escape dismisses the popup + // without closing the dialog, so a second Escape is needed to close it. + // Any other keypress dismisses the popup but is otherwise handled as usual. + // @returns {Boolean} true if the keypress was handled and should not propagate + _handlePageSuggestionKeydown(event) { + let panel = this._id("page-suggestion"); + if (!["open", "showing"].includes(panel.state)) return false; + let option = this._id("page-suggestion-option"); + let highlighted = option.classList.contains("highlighted"); + let noModifiers = !['ctrlKey', 'metaKey', 'shiftKey', 'altKey'].some(key => event[key]); + let handled = true; + if (event.key == "ArrowDown" && !highlighted && noModifiers) { + this._highlightPageSuggestion(true); + } + else if (event.key == "ArrowUp" && highlighted && noModifiers) { + this._highlightPageSuggestion(false); + } + // Enter with a modifier is left alone, so that e.g. Cmd/Ctrl-Enter still accepts the dialog + else if (event.key == "Enter" && highlighted && noModifiers) { + option.click(); + } + else if (event.key == "Escape") { + panel.hidePopup(); + } + else { + handled = false; + // Modifiers on their own are not a keypress one can dismiss the popup with + if (!["Shift", "Control", "Alt", "Meta"].includes(event.key)) { + panel.hidePopup(); + } + } + if (handled) { + event.stopPropagation(); + event.preventDefault(); + } + return handled; + } + + _highlightPageSuggestion(highlighted) { + this.doc.dispatchEvent(new CustomEvent("page-suggestion-highlight", { + bubbles: true, + detail: { highlighted } + })); + } + _handleTopLevelKeydown(event) { let handled = false; let tgt = event.target; diff --git a/chrome/content/zotero/integration/citationDialog/popupHandler.mjs b/chrome/content/zotero/integration/citationDialog/popupHandler.mjs index 22e165f528..7995d02725 100644 --- a/chrome/content/zotero/integration/citationDialog/popupHandler.mjs +++ b/chrome/content/zotero/integration/citationDialog/popupHandler.mjs @@ -34,6 +34,7 @@ export class CitationDialogPopupsHandler { this.discardItemDetailsEdits = false; this.itemDetailsWhenOpened = {}; this.itemDetailsTimeOpened = null; + this.pageSuggestion = null; this.dialogState = dialogState; @@ -44,6 +45,8 @@ export class CitationDialogPopupsHandler { this.doc.addEventListener("popupshown", (event) => { // make sure overlay doesn't appear on tooltips and etc. if (event.target.tagName !== "xul:panel") return; + // page suggestion is never focused, so typing can continue uninterrupted + if (event.target.id == "page-suggestion") return; // if focus is not in the panel tab into it if (!event.target.contains(this.doc.activeElement)) { Services.focus.moveFocus(this.doc.defaultView, event.target, Services.focus.MOVEFOCUS_FORWARD, 0); @@ -85,6 +88,75 @@ export class CitationDialogPopupsHandler { if (this._getNode("#itemDetails").state !== "open") return; this.captureItemDetailsKeyDown(event); }, true); + + // Clicking the page suggestion adds it as a locator + this._getNode("#page-suggestion-option").addEventListener("click", () => this.acceptPageSuggestion()); + // Highlight the suggestion as one arrows into and out of it + this.doc.addEventListener("page-suggestion-highlight", ({ detail: { highlighted } }) => { + this.setPageSuggestionHighlighted(highlighted); + }); + // Any click outside of the page suggestion dismisses it. The click itself is not + // consumed, so it still does whatever it would have done otherwise. + this.doc.addEventListener("mousedown", (event) => { + if (!this.isPageSuggestionOpen()) return; + if (event.target.closest("#page-suggestion")) return; + this.closePageSuggestion(); + }, true); + // Discard the suggestion if the popup is hidden by any other means + this._getNode("#page-suggestion").addEventListener("popuphidden", () => { + this.pageSuggestion = null; + }); + } + + // Suggest the page currently opened in a reader tab as a locator for the + // just-added bubble. The popup is anchored to the input one is typing in. + openPageSuggestion({ page, anchor }) { + if (!page || !anchor) return; + let panel = this._getNode("#page-suggestion"); + // A popup that is still hiding cannot be opened again, so wait it out + if (panel.state == "hiding") { + panel.addEventListener("popuphidden", () => this.openPageSuggestion({ page, anchor }), { once: true }); + return; + } + this.pageSuggestion = { page }; + // Show the page as a localized short locator (e.g. "p.10") + this._getNode("#page-suggestion-option").textContent = Zotero.Cite.getLocatorString("page", "short").toLowerCase() + " " + page; + this.setPageSuggestionHighlighted(false); + if (panel.state == "closed") { + panel.openPopup(anchor, "after_start", 0, 4, false, false, null); + } + else { + panel.moveToAnchor(anchor, "after_start", 0, 4); + } + } + + closePageSuggestion() { + if (!this.isPageSuggestionOpen()) return; + this._getNode("#page-suggestion").hidePopup(); + this.pageSuggestion = null; + } + + // The popup counts as opened while it is still being shown, so that it can be + // dismissed by a keypress or a click that happens right after it appears + isPageSuggestionOpen() { + return ["open", "showing"].includes(this._getNode("#page-suggestion").state); + } + + setPageSuggestionHighlighted(highlighted) { + let option = this._getNode("#page-suggestion-option"); + option.classList.toggle("highlighted", highlighted); + option.setAttribute("aria-selected", highlighted); + } + + // Tell the citation dialog to add the suggested page as a locator + acceptPageSuggestion() { + if (!this.pageSuggestion) return; + let { page } = this.pageSuggestion; + this.closePageSuggestion(); + this.doc.dispatchEvent(new CustomEvent("page-suggestion-accepted", { + bubbles: true, + detail: { page } + })); } openItemDetails(bubbleItem, itemDescription) { diff --git a/chrome/content/zotero/xpcom/reader.js b/chrome/content/zotero/xpcom/reader.js index a8d027e7f2..ee3e93ce8a 100644 --- a/chrome/content/zotero/xpcom/reader.js +++ b/chrome/content/zotero/xpcom/reader.js @@ -115,6 +115,19 @@ class ReaderInstance { return state ? JSON.parse(JSON.stringify(state)) : undefined; } + /** + * Get the page currently displayed in the active view, as shown in the reader's toolbar. + * + * @returns {String|null} - Page, or null if the reader has no pages. + */ + getCurrentPage() { + let state = this._internalReader?._state; + if (!state) return null; + let stats = state.primary ? state.primaryViewStats : state.secondaryViewStats; + if (!stats || stats.pageIndex === undefined) return null; + return stats.pageLabel || String(stats.pageIndex + 1); + } + async migrateMendeleyColors(libraryID, annotations) { let colorMap = new Map(); colorMap.set('#fff5ad', '#ffd400'); diff --git a/scss/components/_citationDialog.scss b/scss/components/_citationDialog.scss index 9abef5df43..1733486a25 100644 --- a/scss/components/_citationDialog.scss +++ b/scss/components/_citationDialog.scss @@ -720,6 +720,7 @@ #popups { panel { padding: 0; + background: var(--material-background); .popup { padding: 14px 16px; } @@ -732,6 +733,29 @@ background-color: rgba(0, 0, 0, 0.1); z-index: 1000; } + #page-suggestion { + --panel-padding: 4px; + // Without this, the background and its corners are drawn natively, + // so the corner radius below has nothing to round + appearance: none; + border-radius: 5px; + @media (-moz-platform: macos) { + border: var(--material-border); + } + #page-suggestion-option { + height: 18px; + line-height: 16px; + cursor: default; + padding: 0 4px; + &:hover { + background-color: var(--accent-blue30); + } + // highlighted via arrow navigation, since the popup never takes focus + &.highlighted { + background-color: var(--accent-blue30); + } + } + } #settings-popup { top: 35px; right: 5px; diff --git a/test/tests/citationDialogTest.js b/test/tests/citationDialogTest.js index e7ae2f6f94..909336b319 100644 --- a/test/tests/citationDialogTest.js +++ b/test/tests/citationDialogTest.js @@ -16,7 +16,7 @@ describe("Citation Dialog", function () { preview: () => {}, allCitedDataLoadedPromise: Promise.resolve(), }; - let dialog, win, doc, IOManager, CitationDataManager, SearchHandler; + let dialog, win, doc, IOManager, CitationDataManager, SearchHandler, Helpers; before(async function () { // Zotero.Cite.getLocatorString() requires styles to be initialized @@ -30,6 +30,7 @@ describe("Citation Dialog", function () { IOManager = dialog.IOManager; CitationDataManager = dialog.CitationDataManager; SearchHandler = dialog.SearchHandler; + Helpers = dialog.Helpers; // wait for everything (e.g. itemTree/collectionTree) inside of the dialog to be loaded. while (!dialog.DIALOG_STATE.loaded) { await Zotero.Promise.delay(10); @@ -433,6 +434,106 @@ describe("Citation Dialog", function () { }); }); + describe("Page suggestion", function () { + let panel, option, getOpenTabPageStub; + + // Add an item to the citation, pretending that it is opened in a reader tab + // displaying the given page, and wait for the suggestion popup to appear + async function addItemWithOpenTabPage(page = "12") { + let item = await createDataObject('item'); + getOpenTabPageStub = sinon.stub(Helpers, "getOpenTabPage").returns(page); + await IOManager.addItemsToCitation([item]); + await waitForCallback(() => panel.state == "open"); + return item; + } + + function keydown(key) { + let input = doc.getElementById("bubble-input").getCurrentInput(); + input.dispatchEvent(new KeyboardEvent('keydown', { key, bubbles: true })); + } + + before(function () { + panel = doc.getElementById("page-suggestion"); + option = doc.getElementById("page-suggestion-option"); + }); + + beforeEach(function () { + CitationDataManager.items = []; + IOManager.updateBubbleInput(); + // The guidance panel is displayed next to the same bubble, so the suggestion + // is not shown until it has been seen + Zotero.Prefs.set("firstRunGuidanceShown.citationDialog", true); + }); + + afterEach(async function () { + getOpenTabPageStub?.restore(); + getOpenTabPageStub = null; + // Wait for the popup to be fully hidden, since a popup that is still + // hiding cannot be opened again by the next test + if (panel.state != "closed") { + panel.hidePopup(); + await waitForCallback(() => panel.state == "closed"); + } + }); + + it("should suggest the current page of an item opened in a reader tab", async function () { + let item = await createDataObject('item'); + let attachment = await importPDFAttachment(item); + await Zotero.Reader.open(attachment.id); + let reader = Zotero.Reader.getByTabID(win.Zotero_Tabs.selectedID); + await reader._waitForReader(); + // View stats, which the current page comes from, are set after the view renders + await waitForCallback(() => reader.getCurrentPage()); + assert.equal(reader.getCurrentPage(), "1"); + + // Opening a tab refocuses the dialog, which kicks off a search + while (SearchHandler.searching) { + await Zotero.Promise.delay(10); + } + assert.equal(Helpers.getOpenTabPage(item), "1"); + + await IOManager.addItemsToCitation([item]); + await waitForCallback(() => panel.state == "open"); + assert.equal(option.textContent, Zotero.Cite.getLocatorString("page", "short").toLowerCase() + " 1"); + + // Cleanup + panel.hidePopup(); + win.Zotero_Tabs.close(win.Zotero_Tabs.selectedID); + SearchHandler.clearNonLibraryItemsCache(); + }); + + it("should add the suggested page as a locator on ArrowDown and Enter", async function () { + await addItemWithOpenTabPage("12"); + + keydown("ArrowDown"); + assert.isTrue(option.classList.contains("highlighted")); + + keydown("Enter"); + assert.equal(CitationDataManager.items[0].locator, "12"); + assert.equal(CitationDataManager.items[0].label, "page"); + assert.equal(panel.state, "closed"); + // The bubble no longer awaits a typed locator + assert.notOk(IOManager._justAddedBubbles); + }); + + it("should add the suggested page as a locator on click", async function () { + await addItemWithOpenTabPage("iv"); + + option.click(); + assert.equal(CitationDataManager.items[0].locator, "iv"); + assert.equal(CitationDataManager.items[0].label, "page"); + assert.equal(panel.state, "closed"); + }); + + + it("should dismiss the suggestion when the bubble is no longer just-added", async function () { + await addItemWithOpenTabPage("12"); + + IOManager._clearJustAddedBubbles(); + assert.equal(panel.state, "closed"); + }); + }); + describe("UI", function () { beforeEach(function () { CitationDataManager.items = []; From c16db4e4aa929ad0351854ec1f33b075292e7716 Mon Sep 17 00:00:00 2001 From: Bogdan Abaev Date: Sat, 8 Aug 2026 13:23:26 -0700 Subject: [PATCH 2/2] suggest last opened page of unloaded tabs When a tab is unloaded, save the pageLabel as a field in reader state file, which can then be read by the citation dialog to make the suggestion. --- .../zotero/integration/citationDialog.js | 26 ++++++------ .../integration/citationDialog/helpers.mjs | 23 +++++----- chrome/content/zotero/xpcom/reader.js | 40 ++++++++++++++++-- test/tests/citationDialogTest.js | 42 ++++++++++++++++++- 4 files changed, 105 insertions(+), 26 deletions(-) diff --git a/chrome/content/zotero/integration/citationDialog.js b/chrome/content/zotero/integration/citationDialog.js index 52c0db9776..8b7f38c6fd 100644 --- a/chrome/content/zotero/integration/citationDialog.js +++ b/chrome/content/zotero/integration/citationDialog.js @@ -1436,9 +1436,9 @@ const IOManager = { // Add entries into the citation with the current locator if specified let bubbleItems = items.map(item => BubbleItem.fromItem(item)); - // Page of a reader tab with an attachment of the added item to be - // suggested as a locator once the bubble is added - let pageSuggestion = null; + // Whether to suggest the page of a reader tab with an attachment of the + // added item as a locator once the bubble is added + let suggestPage = false; if (locator) { for (let bubbleItem of bubbleItems) { bubbleItem.locator = locator.locator; @@ -1455,10 +1455,8 @@ const IOManager = { // that, the user presumably knows about the shortcut _id("bubble-input").showJustAddedPlaceholder = DIALOG_STATE.isCitingItems() && this._timesItemsAdded < 1; - // If the item is opened in a reader tab, suggest its current page as a locator - if (DIALOG_STATE.isCitingItems()) { - pageSuggestion = Helpers.getOpenTabPage(items[0]); - } + // If the item is opened in a reader tab, its page is suggested as a locator + suggestPage = DIALOG_STATE.isCitingItems(); } else { // A multi-item add doesn't enter locator-typing mode, so typed text @@ -1494,11 +1492,15 @@ const IOManager = { } // Suggest the page of the opened document, unless the first run guidance panel // is already displayed next to the same bubble - if (pageSuggestion && !guidanceShown) { - PopupsHandler.openPageSuggestion({ - page: pageSuggestion, - anchor: _id("bubble-input").getCurrentInput() - }); + if (suggestPage && !guidanceShown) { + let page = await Helpers.getOpenTabPage(items[0]); + // The bubble the locator would be added to may be gone by now + if (page && this._justAddedBubbles) { + PopupsHandler.openPageSuggestion({ + page, + anchor: _id("bubble-input").getCurrentInput() + }); + } } dialogNotPristine(); }, diff --git a/chrome/content/zotero/integration/citationDialog/helpers.mjs b/chrome/content/zotero/integration/citationDialog/helpers.mjs index 1552a96e14..6ea73d293e 100644 --- a/chrome/content/zotero/integration/citationDialog/helpers.mjs +++ b/chrome/content/zotero/integration/citationDialog/helpers.mjs @@ -510,25 +510,28 @@ export class CitationDialogHelpers { // Get the page currently displayed by a reader tab with an attachment of the given // top-level item. If the item is open in multiple tabs, the selected tab wins. + // An unloaded tab has no reader, so the page saved by the reader before the tab was + // unloaded is used for it. // Returns null if the item is not open in any reader tab or if the reader has no // pages (e.g. a snapshot). - getOpenTabPage(item) { + async getOpenTabPage(item) { if (!item) return null; let win = Zotero.getMainWindow(); if (!win) return null; - let tabIDs = item.getAttachments() - .map(attachmentID => win.Zotero_Tabs.getTabIDByItemID(attachmentID)) - .filter(Boolean); + let tabs = item.getAttachments() + .map(attachmentID => ({ attachmentID, tabID: win.Zotero_Tabs.getTabIDByItemID(attachmentID) })) + .filter(({ tabID }) => tabID); // Look at the selected tab first - tabIDs.sort((a, b) => { - if (a === win.Zotero_Tabs.selectedID) return -1; - if (b === win.Zotero_Tabs.selectedID) return 1; + tabs.sort((a, b) => { + if (a.tabID === win.Zotero_Tabs.selectedID) return -1; + if (b.tabID === win.Zotero_Tabs.selectedID) return 1; return 0; }); - for (let tabID of tabIDs) { + for (let { attachmentID, tabID } of tabs) { let reader = Zotero.Reader.getByTabID(tabID); - if (!reader) continue; - let page = reader.getCurrentPage(); + let page = reader + ? reader.getCurrentPage() + : await Zotero.Reader.getSavedPageLabel(attachmentID); if (page) return page; } return null; diff --git a/chrome/content/zotero/xpcom/reader.js b/chrome/content/zotero/xpcom/reader.js index ee3e93ce8a..90bffd55d6 100644 --- a/chrome/content/zotero/xpcom/reader.js +++ b/chrome/content/zotero/xpcom/reader.js @@ -40,6 +40,8 @@ const ARRAYBUFFER_MAX_LENGTH = Services.appinfo.is64Bit ? Math.pow(2, 33) : Math.pow(2, 32) - 1; +const READER_STATE_FILE_NAME = '.zotero-reader-state'; + const READ_ALOUD_ENABLED_VOICES_PATH = PathUtils.join(Zotero.Profile.dir, 'readAloudEnabledVoices.json'); const READ_ALOUD_VOICE_DEFAULTS_PATH = PathUtils.join(Zotero.Profile.dir, 'readAloudVoiceDefaults.json'); @@ -48,7 +50,7 @@ let readAloudCachePruned = false; class ReaderInstance { constructor(options) { - this.stateFileName = '.zotero-reader-state'; + this.stateFileName = READER_STATE_FILE_NAME; this.annotationItemIDs = []; this._item = options.item; this._instanceID = Zotero.Utilities.randomString(); @@ -125,7 +127,13 @@ class ReaderInstance { if (!state) return null; let stats = state.primary ? state.primaryViewStats : state.secondaryViewStats; if (!stats || stats.pageIndex === undefined) return null; - return stats.pageLabel || String(stats.pageIndex + 1); + if (this._type === 'pdf') { + return stats.pageLabel || String(stats.pageIndex + 1); + } + if (this._type === 'epub' && stats.usePhysicalPageNumbers) { + return stats.pageLabel || null; + } + return null; } async migrateMendeleyColors(libraryID, annotations) { @@ -1046,6 +1054,10 @@ class ReaderInstance { } } + // Save the page displayed at the time of this state, so that it is available + // while the reader is not loaded (e.g. for an unloaded tab) + state.pageLabel = this.getCurrentPage(); + let file = Zotero.Attachments.getStorageDirectory(item); if (!(await OS.File.exists(file.path))) { await Zotero.Attachments.createDirectoryForItem(item); @@ -2864,7 +2876,29 @@ class Reader { getByTabID(tabID) { return this._readers.find(r => (r instanceof ReaderTab) && r.tabID === tabID); } - + + /** + * Get the page that was displayed when the reader for the given attachment last saved + * its state by reading the state file. + * @param {Number} itemID - Attachment item ID + * @returns {Promise} - Page, or null if none was saved + */ + async getSavedPageLabel(itemID) { + try { + let item = Zotero.Items.get(itemID); + if (!item?.isFileAttachment()) return null; + let directory = Zotero.Attachments.getStorageDirectory(item); + let path = PathUtils.join(directory.path, READER_STATE_FILE_NAME); + if (!(await IOUtils.exists(path))) return null; + let state = await IOUtils.readJSON(path); + return state.pageLabel || null; + } + catch (e) { + Zotero.logError(e); + return null; + } + } + getWindowStates() { return this._readers .filter(r => r instanceof ReaderWindow) diff --git a/test/tests/citationDialogTest.js b/test/tests/citationDialogTest.js index 909336b319..daf5f08fba 100644 --- a/test/tests/citationDialogTest.js +++ b/test/tests/citationDialogTest.js @@ -490,7 +490,7 @@ describe("Citation Dialog", function () { while (SearchHandler.searching) { await Zotero.Promise.delay(10); } - assert.equal(Helpers.getOpenTabPage(item), "1"); + assert.equal(await Helpers.getOpenTabPage(item), "1"); await IOManager.addItemsToCitation([item]); await waitForCallback(() => panel.state == "open"); @@ -502,6 +502,46 @@ describe("Citation Dialog", function () { SearchHandler.clearNonLibraryItemsCache(); }); + it("should suggest the last saved page of an item opened in an unloaded tab", async function () { + let item = await createDataObject('item'); + let attachment = await importPDFAttachment(item); + await Zotero.Reader.open(attachment.id); + let tabID = win.Zotero_Tabs.selectedID; + let reader = Zotero.Reader.getByTabID(tabID); + await reader._waitForReader(); + await waitForCallback(() => reader.getCurrentPage()); + // The reader saves its state on a debounce, and there is nothing to flush + // until it has done so at least once + await waitForCallback(() => Number.isInteger(attachment.getAttachmentLastPageIndex())); + + // Only an unselected tab can be unloaded. Unloading closes the reader, + // which flushes its state -- including the page -- to disk. + win.Zotero_Tabs.select("zotero-pane"); + win.Zotero_Tabs.unload(tabID); + // The reader is discarded once the tab close notification is processed + await waitForCallback(() => !Zotero.Reader.getByTabID(tabID)); + // The state is written asynchronously + let savedPage; + for (let i = 0; i < 100 && !savedPage; i++) { + savedPage = await Zotero.Reader.getSavedPageLabel(attachment.id); + await Zotero.Promise.delay(10); + } + assert.equal(savedPage, "1"); + assert.equal(await Helpers.getOpenTabPage(item), "1"); + + while (SearchHandler.searching) { + await Zotero.Promise.delay(10); + } + await IOManager.addItemsToCitation([item]); + await waitForCallback(() => panel.state == "open"); + assert.equal(option.textContent, Zotero.Cite.getLocatorString("page", "short").toLowerCase() + " 1"); + + // Cleanup + panel.hidePopup(); + win.Zotero_Tabs.close(tabID); + SearchHandler.clearNonLibraryItemsCache(); + }); + it("should add the suggested page as a locator on ArrowDown and Enter", async function () { await addItemWithOpenTabPage("12");