Fix attachments & annotation box refresh bugs and add tests (#4031)

Fixes #3993
Fixes #3995
Closes #4082
This commit is contained in:
windingwind 2024-04-22 09:15:22 +08:00 committed by Dan Stillman
parent 07e8f0a01d
commit 91c0c28b5d
10 changed files with 806 additions and 49 deletions

View file

@ -1,9 +1,38 @@
describe("Item pane", function () {
var win, doc, itemsView;
var win, doc, ZoteroPane, Zotero_Tabs, ZoteroContextPane, itemsView;
async function waitForPreviewBoxRender(box) {
let success = await waitForCallback(
() => box._asyncRenderItemID && !box._asyncRendering);
if (!success) {
throw new Error("Wait for box render time out");
}
await box._preview._renderPromise;
return success;
}
async function waitForPreviewBoxReader(box, itemID) {
await waitForPreviewBoxRender(box);
let success = await waitForCallback(
() => box._preview._reader?.itemID == itemID, 100, 3000);
if (!success) {
throw new Error("Wait for box preview reader time out");
}
await box._preview._reader._initPromise;
return success;
}
function isPreviewDisplayed(box) {
return !!(box._preview.hasPreview
&& win.getComputedStyle(box._preview).display !== "none");
}
before(function* () {
win = yield loadZoteroPane();
doc = win.document;
ZoteroPane = win.ZoteroPane;
Zotero_Tabs = win.Zotero_Tabs;
ZoteroContextPane = win.ZoteroContextPane;
itemsView = win.ZoteroPane.itemsView;
});
after(function () {
@ -168,7 +197,7 @@ describe("Item pane", function () {
assert.equal(label.value, 'Test');
yield Zotero.Items.erase(id);
})
});
it("should swap creator names", async function () {
@ -344,7 +373,479 @@ describe("Item pane", function () {
'1'
);
});
})
});
describe("Attachments pane", function () {
let paneID = "attachments";
beforeEach(function () {
Zotero.Prefs.set("panes.attachments.open", true);
Zotero.Prefs.set("showAttachmentPreview", true);
Zotero_Tabs.select("zotero-pane");
});
afterEach(function () {
Zotero_Tabs.select("zotero-pane");
});
it("should show attachments pane in library for regular item", async function () {
// Regular item: show
let attachmentsBox = ZoteroPane.itemPane._itemDetails.getPane(paneID);
let item = new Zotero.Item('book');
await item.saveTx();
await ZoteroPane.selectItem(item.id);
assert.isFalse(attachmentsBox.hidden);
// Child attachment: hide
let file = getTestDataDirectory();
file.append('test.pdf');
let attachment = await Zotero.Attachments.importFromFile({
file,
parentItemID: item.id
});
await ZoteroPane.selectItem(attachment.id);
assert.isTrue(attachmentsBox.hidden);
// Standalone attachment: hide
let attachment1 = await importFileAttachment('test.pdf');
await ZoteroPane.selectItem(attachment1.id);
assert.isTrue(attachmentsBox.hidden);
});
it("should not show attachments pane preview in reader best-matched attachment item", async function () {
let item = new Zotero.Item('book');
let file = getTestDataDirectory();
file.append('test.pdf');
await item.saveTx();
let attachment = await Zotero.Attachments.importFromFile({
file,
parentItemID: item.id
});
await ZoteroPane.viewItems([attachment]);
let tabID = Zotero_Tabs.selectedID;
let itemDetails = ZoteroContextPane.context._getItemContext(tabID);
let attachmentsBox = itemDetails.getPane(paneID);
assert.isFalse(attachmentsBox.hidden);
await waitForScrollToPane(itemDetails, paneID);
assert.isFalse(isPreviewDisplayed(attachmentsBox));
});
it("should not show attachments pane in reader standalone attachment item", async function () {
let attachment = await importFileAttachment('test.pdf');
await ZoteroPane.viewItems([attachment]);
let tabID = Zotero_Tabs.selectedID;
let itemDetails = ZoteroContextPane.context._getItemContext(tabID);
let attachmentsBox = itemDetails.getPane(paneID);
assert.isTrue(attachmentsBox.hidden);
});
it("should show attachments pane preview in reader non-best-matched attachment item", async function () {
let item = new Zotero.Item('book');
let file = getTestDataDirectory();
file.append('test.pdf');
await item.saveTx();
await Zotero.Attachments.importFromFile({
file,
parentItemID: item.id
});
await Zotero.Attachments.importFromFile({
file,
parentItemID: item.id
});
let bestAttachments = await item.getBestAttachments();
await ZoteroPane.viewItems([bestAttachments[1]]);
// Ensure context pane is open
ZoteroContextPane.splitter.setAttribute("state", "open");
await waitForFrame();
let tabID = Zotero_Tabs.selectedID;
let itemDetails = ZoteroContextPane.context._getItemContext(tabID);
let attachmentsBox = itemDetails.getPane(paneID);
assert.isFalse(attachmentsBox.hidden);
await waitForScrollToPane(itemDetails, paneID);
await waitForPreviewBoxRender(attachmentsBox);
assert.isTrue(isPreviewDisplayed(attachmentsBox));
});
it("should not render attachments pane preview when show preview is disabled", async function () {
Zotero.Prefs.set("showAttachmentPreview", false);
let itemDetails = ZoteroPane.itemPane._itemDetails;
let attachmentsBox = itemDetails.getPane(paneID);
let item = new Zotero.Item('book');
await item.saveTx();
await ZoteroPane.selectItem(item.id);
assert.isFalse(attachmentsBox.hidden);
await waitForScrollToPane(itemDetails, paneID);
assert.isFalse(isPreviewDisplayed(attachmentsBox));
});
it("should only render after attachments pane becomes visible", async function () {
// Resize to very small height to ensure the attachment box is not in view
let height = doc.documentElement.clientHeight;
win.resizeTo(null, 100);
let itemDetails = ZoteroPane.itemPane._itemDetails;
let attachmentsBox = itemDetails.getPane(paneID);
let preview = attachmentsBox._preview;
// Force discard previous preview
await preview.discard(true);
let item = new Zotero.Item('book');
await item.saveTx();
let file = getTestDataDirectory();
file.append('test.pdf');
await Zotero.Attachments.importFromFile({
file,
parentItemID: item.id
});
await ZoteroPane.selectItem(item.id);
assert.isFalse(itemDetails.isPaneVisible(paneID));
// 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));
await waitForScrollToPane(itemDetails, paneID);
await waitForPreviewBoxRender(attachmentsBox);
assert.isTrue(itemDetails.isPaneVisible(paneID));
assert.equal(attachmentsBox._syncRenderItemID, item.id);
assert.equal(attachmentsBox._asyncRenderItemID, item.id);
assert.isTrue(isPreviewDisplayed(attachmentsBox));
assert.isTrue(preview.hasPreview);
win.resizeTo(null, height);
});
it("should update attachments pane when attachments changed", async function () {
// https://forums.zotero.org/discussion/113632/zotero-7-beta-pdf-attachment-preview-and-annotations-not-refreshed-after-adding-annotations
let itemDetails = ZoteroPane.itemPane._itemDetails;
let attachmentsBox = itemDetails.getPane(paneID);
let preview = attachmentsBox._preview;
// Force discard previous preview
await preview.discard(true);
// Pin the pane to ensure it's rendered
itemDetails.pinnedPane = paneID;
let item = new Zotero.Item('book');
await item.saveTx();
await ZoteroPane.selectItem(item.id);
assert.isTrue(await waitForPreviewBoxRender(attachmentsBox));
// No preview
assert.isFalse(isPreviewDisplayed(attachmentsBox));
// No row
assert.equal(attachmentsBox.querySelectorAll("attachment-row").length, 0);
// Add an attachment
let file = getTestDataDirectory();
file.append('test.png');
let _attachment1 = await Zotero.Attachments.importFromFile({
file,
parentItemID: item.id
});
await ZoteroPane.selectItem(item.id);
await itemDetails._renderPromise;
await waitForPreviewBoxRender(attachmentsBox);
// Image preview for item with image attachment
assert.isTrue(isPreviewDisplayed(attachmentsBox));
assert.equal(preview.previewType, "image");
// 1 row
assert.equal(attachmentsBox.querySelectorAll("attachment-row").length, 1);
// Add an PDF attachment, which will be best match and update the preview
file = getTestDataDirectory();
file.append('test.pdf');
let attachment2 = await Zotero.Attachments.importFromFile({
file,
parentItemID: item.id
});
await waitForPreviewBoxReader(attachmentsBox, attachment2.id);
await Zotero.Promise.delay(100);
// PDF preview
assert.isTrue(isPreviewDisplayed(attachmentsBox));
assert.equal(preview.previewType, "pdf");
// 2 rows
assert.equal(attachmentsBox.querySelectorAll("attachment-row").length, 2);
// Created annotations should be update in preview and attachment row
let annotation = await createAnnotation('highlight', attachment2);
await Zotero.Promise.delay(100);
// Annotation updated in preview reader
let readerAnnotation
= preview._reader._internalReader._annotationManager._annotations.find(
a => a.libraryID === annotation.libraryID && a.id === annotation.key
);
assert.exists(readerAnnotation);
assert.equal(attachmentsBox.querySelectorAll("attachment-row").length, 2);
let attachmentRow = attachmentsBox.querySelector(`attachment-row[attachment-id="${attachment2.id}"]`);
assert.isFalse(attachmentRow._annotationButton.hidden);
// 1 annotation
assert.equal(attachmentRow._annotationButton.querySelector('.label').textContent, "1");
// Deleted annotations should be removed from preview and attachment row
await annotation.eraseTx();
await Zotero.Promise.delay(100);
// Annotation removed from preview reader
readerAnnotation
= preview._reader._internalReader._annotationManager._annotations.find(
a => a.libraryID === annotation.libraryID && a.id === annotation.key
);
assert.notExists(readerAnnotation);
// Row might be recreated
attachmentRow = attachmentsBox.querySelector(`attachment-row[attachment-id="${attachment2.id}"]`);
assert.isTrue(attachmentRow._annotationButton.hidden);
// 0 annotation
assert.equal(attachmentRow._annotationButton.querySelector('.label').textContent, "0");
// Delete attachment
await attachment2.eraseTx();
await Zotero.Promise.delay(100);
// Image preview for item with image attachment
assert.isTrue(isPreviewDisplayed(attachmentsBox));
assert.equal(preview.previewType, "image");
// 1 row
assert.equal(attachmentsBox.querySelectorAll("attachment-row").length, 1);
// The corresponding row should be removed
attachmentRow = attachmentsBox.querySelector(`attachment-row[attachment-id="${attachment2.id}"]`);
assert.notExists(attachmentRow);
// Unpin
itemDetails.pinnedPane = "";
});
it("should keep attachments pane preview status after switching tab", async function () {
// https://forums.zotero.org/discussion/113658/zotero-7-beta-preview-appearing-in-the-item-pane-of-the-pdf-tab
let item = new Zotero.Item('book');
let file = getTestDataDirectory();
file.append('test.pdf');
await item.saveTx();
let attachment = await Zotero.Attachments.importFromFile({
file,
parentItemID: item.id
});
// Open reader
await ZoteroPane.viewItems([attachment]);
let tabID = Zotero_Tabs.selectedID;
await Zotero.Reader.getByTabID(tabID)._waitForReader();
// Ensure context pane is open
ZoteroContextPane.splitter.setAttribute("state", "open");
await waitForFrame();
let itemDetails = ZoteroContextPane.context._getItemContext(tabID);
let attachmentsBox = itemDetails.getPane(paneID);
assert.isFalse(attachmentsBox.hidden);
await waitForScrollToPane(itemDetails, paneID);
assert.isFalse(isPreviewDisplayed(attachmentsBox));
// Select library tab
Zotero_Tabs.select("zotero-pane");
let libraryItemDetails = ZoteroPane.itemPane._itemDetails;
let libraryAttachmentsBox = libraryItemDetails.getPane(paneID);
await ZoteroPane.selectItem(item.id);
await waitForScrollToPane(libraryItemDetails, paneID);
// Collapse section
libraryAttachmentsBox.querySelector('collapsible-section > .head').click();
await Zotero.Promise.delay(50);
// Open section
libraryAttachmentsBox.querySelector('collapsible-section > .head').click();
await Zotero.Promise.delay(50);
// Select reader tab
Zotero_Tabs.select(tabID);
// Make sure the preview status is not changed in reader
assert.isFalse(isPreviewDisplayed(attachmentsBox));
});
/**
* This test is essential to ensure the proper functioning of the sync/async rendering,
* scrolling handler, and pinning mechanism of ItemDetails.
* AttachmentsBox serves as a good example since it involves both sync and async rendering.
* If this test fails, it is not recommended to add timeouts as a quick fix.
*/
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;
// Pin the pane to avoid always scrolling to the section
itemDetails.pinnedPane = paneID;
// item with attachment (1 annotation)
let item1 = new Zotero.Item('book');
await item1.saveTx();
let file = getTestDataDirectory();
file.append('test.pdf');
let attachment1 = await Zotero.Attachments.importFromFile({
file,
parentItemID: item1.id
});
let annotation = await createAnnotation('highlight', attachment1);
await itemDetails._renderPromise;
await waitForPreviewBoxReader(attachmentsBox, attachment1.id);
assert.isFalse(attachmentsBox.hidden);
let readerAnnotation
= preview._reader._internalReader._annotationManager._annotations.find(
a => a.libraryID === annotation.libraryID && a.id === annotation.key
);
assert.exists(readerAnnotation);
assert.equal(attachmentsBox.querySelectorAll("attachment-row").length, 1);
let attachmentRow = attachmentsBox.querySelector(`attachment-row[attachment-id="${attachment1.id}"]`);
assert.isFalse(attachmentRow._annotationButton.hidden);
// 1 annotation
assert.equal(attachmentRow._annotationButton.querySelector('.label').textContent, "1");
// item with attachment (no annotation)
let item2 = new Zotero.Item('book');
await item2.saveTx();
file = getTestDataDirectory();
file.append('wonderland_short.pdf');
let attachment2 = await Zotero.Attachments.importFromFile({
file,
parentItemID: item2.id
});
// Select item with attachment (no annotation)
await itemDetails._renderPromise;
await waitForPreviewBoxReader(attachmentsBox, attachment2.id);
assert.isFalse(attachmentsBox.hidden);
readerAnnotation
= preview._reader._internalReader._annotationManager._annotations.find(
a => a.libraryID === annotation.libraryID && a.id === annotation.key
);
assert.notExists(readerAnnotation);
assert.equal(attachmentsBox.querySelectorAll("attachment-row").length, 1);
attachmentRow = attachmentsBox.querySelector(`attachment-row[attachment-id="${attachment2.id}"]`);
assert.isTrue(attachmentRow._annotationButton.hidden);
// 0 annotation
assert.equal(attachmentRow._annotationButton.querySelector('.label').textContent, "0");
let item3 = new Zotero.Item('book');
await item3.saveTx();
// Select item without attachment
await itemDetails._renderPromise;
assert.isFalse(attachmentsBox.hidden);
assert.equal(attachmentsBox.querySelectorAll("attachment-row").length, 0);
// Again, select item with attachment (1 annotation)
await ZoteroPane.selectItem(item1.id);
await itemDetails._renderPromise;
await waitForPreviewBoxReader(attachmentsBox, attachment1.id);
assert.isFalse(attachmentsBox.hidden);
readerAnnotation
= preview._reader._internalReader._annotationManager._annotations.find(
a => a.libraryID === annotation.libraryID && a.id === annotation.key
);
assert.exists(readerAnnotation);
assert.equal(attachmentsBox.querySelectorAll("attachment-row").length, 1);
attachmentRow = attachmentsBox.querySelector(`attachment-row[attachment-id="${attachment1.id}"]`);
assert.isFalse(attachmentRow._annotationButton.hidden);
// 1 annotation
assert.equal(attachmentRow._annotationButton.querySelector('.label').textContent, "1");
// Unpin
itemDetails.pinnedPane = "";
});
it("should open attachment on clicking attachment row", async function () {
let itemDetails = ZoteroPane.itemPane._itemDetails;
let attachmentsBox = itemDetails.getPane(paneID);
let item = new Zotero.Item('book');
await item.saveTx();
let file = getTestDataDirectory();
file.append('test.pdf');
let attachment = await Zotero.Attachments.importFromFile({
file,
parentItemID: item.id
});
await ZoteroPane.selectItem(item.id);
await waitForScrollToPane(itemDetails, paneID);
await waitForPreviewBoxRender(attachmentsBox);
let attachmentRow = attachmentsBox.querySelector(`attachment-row[attachment-id="${attachment.id}"]`);
attachmentRow._attachmentButton.click();
await Zotero.Promise.delay(100);
let reader = await Zotero.Reader.getByTabID(Zotero_Tabs.selectedID);
// Should open attachment
assert.equal(reader.itemID, attachment.id);
});
it("should select attachment on clicking annotation button of attachment row", async function () {
let itemDetails = ZoteroPane.itemPane._itemDetails;
let attachmentsBox = itemDetails.getPane(paneID);
let item = new Zotero.Item('book');
await item.saveTx();
let file = getTestDataDirectory();
file.append('test.pdf');
let attachment = await Zotero.Attachments.importFromFile({
file,
parentItemID: item.id
});
let _annotation = await createAnnotation('highlight', attachment);
await ZoteroPane.selectItem(item.id);
await waitForScrollToPane(itemDetails, paneID);
await waitForPreviewBoxRender(attachmentsBox);
let attachmentRow = attachmentsBox.querySelector(`attachment-row[attachment-id="${attachment.id}"]`);
attachmentRow._annotationButton.click();
await Zotero.Promise.delay(100);
// Should select attachment
assert.equal(ZoteroPane.getSelectedItems(true)[0], attachment.id);
});
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 item = new Zotero.Item('book');
await item.saveTx();
let file = getTestDataDirectory();
file.append('test.pdf');
let attachment = await Zotero.Attachments.importFromFile({
file,
parentItemID: item.id
});
await ZoteroPane.selectItem(item.id);
await waitForScrollToPane(itemDetails, paneID);
await waitForPreviewBoxRender(attachmentsBox);
let event = new MouseEvent('dblclick', {
bubbles: true,
cancelable: true,
view: window
});
preview.dispatchEvent(event);
await Zotero.Promise.delay(100);
let reader = await Zotero.Reader.getByTabID(Zotero_Tabs.selectedID);
// Should open attachment
assert.equal(reader.itemID, attachment.id);
});
});
describe("Notes pane", function () {
@ -461,6 +962,18 @@ describe("Item pane", function () {
describe("Attachment pane", function () {
let paneID = "attachment-info";
beforeEach(function () {
Zotero.Prefs.set("panes.attachment-info.open", true);
Zotero.Prefs.set("showAttachmentPreview", true);
Zotero_Tabs.select("zotero-pane");
});
afterEach(function () {
Zotero_Tabs.select("zotero-pane");
});
it("should refresh on file rename", async function () {
let file = getTestDataDirectory();
file.append('test.png');
@ -499,6 +1012,95 @@ describe("Item pane", function () {
await promise;
assert.equal(label.value, newTitle);
});
it("should show attachment pane in library for attachment item", async function () {
// Regular item: hide
let itemDetails = ZoteroPane.itemPane._itemDetails;
let box = itemDetails.getPane(paneID);
let item = new Zotero.Item('book');
await item.saveTx();
await ZoteroPane.selectItem(item.id);
await waitForScrollToPane(itemDetails, paneID);
assert.isTrue(box.hidden);
// Child attachment: show
let file = getTestDataDirectory();
file.append('test.pdf');
let attachment = await Zotero.Attachments.importFromFile({
file,
parentItemID: item.id
});
await ZoteroPane.selectItem(attachment.id);
await waitForScrollToPane(itemDetails, paneID);
await waitForPreviewBoxReader(box, attachment.id);
assert.isFalse(box.hidden);
await Zotero.Promise.delay(100);
assert.isTrue(isPreviewDisplayed(box));
// Standalone attachment: show
let attachment1 = await importFileAttachment('test.pdf');
await ZoteroPane.selectItem(attachment1.id);
await waitForScrollToPane(itemDetails, paneID);
await waitForPreviewBoxReader(box, attachment1.id);
assert.isFalse(box.hidden);
await Zotero.Promise.delay(100);
assert.isTrue(isPreviewDisplayed(box));
});
it("should show attachment pane without preview in reader for standalone attachment item", async function () {
// Attachment item with parent item: hide
let item = new Zotero.Item('book');
let file = getTestDataDirectory();
file.append('test.pdf');
await item.saveTx();
let attachment = await Zotero.Attachments.importFromFile({
file,
parentItemID: item.id
});
await ZoteroPane.viewItems([attachment]);
let tabID = Zotero_Tabs.selectedID;
let itemDetails = ZoteroContextPane.context._getItemContext(tabID);
let box = itemDetails.getPane(paneID);
assert.isTrue(box.hidden);
// Standalone attachment item: show
attachment = await importFileAttachment('test.pdf');
await ZoteroPane.viewItems([attachment]);
tabID = Zotero_Tabs.selectedID;
itemDetails = ZoteroContextPane.context._getItemContext(tabID);
box = itemDetails.getPane(paneID);
assert.isFalse(box.hidden);
await waitForScrollToPane(itemDetails, paneID);
// No preview
assert.isFalse(isPreviewDisplayed(box));
});
it("should only show attachment note container when exists", async function () {
let itemDetails = ZoteroPane.itemPane._itemDetails;
let box = itemDetails.getPane(paneID);
let noteContainer = box._id("note-container");
let noteEditor = box._id('attachment-note-editor');
// Hide note container by default
let attachment = await importFileAttachment('test.pdf');
await ZoteroPane.selectItem(attachment.id);
await itemDetails._renderPromise;
await waitForScrollToPane(itemDetails, paneID);
await waitForPreviewBoxRender(box);
assert.isTrue(noteContainer.hidden);
// Add attachment note
let itemModifyPromise = waitForItemEvent("modify");
attachment.setNote("<h1>TEST</h1>");
await attachment.saveTx();
await itemModifyPromise;
await waitForPreviewBoxRender(box);
// Should show note container
assert.isFalse(noteContainer.hidden);
// Should be readonly
assert.equal(noteEditor.mode, "view");
});
});
@ -602,4 +1204,4 @@ describe("Item pane", function () {
);
});
});
})
});