From 73d88421bb67e52428dabbe0e9506c595f8f6753 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Tue, 24 Oct 2017 04:54:36 -0400 Subject: [PATCH] Fix items list problems when adding item with a search entered When an item is created, an active quick search is cleared, but that's now an async operation. We weren't waiting for that, which meant that new items weren't selected and depending on a race condition could even show the welcome pane despite there being items in the library. --- chrome/content/zotero/xpcom/itemTreeView.js | 17 +++++++++++++++- test/tests/itemTreeViewTest.js | 22 +++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js index 16d6f9cbdf..9272319fc7 100644 --- a/chrome/content/zotero/xpcom/itemTreeView.js +++ b/chrome/content/zotero/xpcom/itemTreeView.js @@ -811,8 +811,14 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio } if (!allDeleted) { - // DEBUG: Search is async, so this might not work properly quicksearch.doCommand(); + // See _refreshPromise note below + if (this._refreshPromise) { + try { + yield this._refreshPromise; + } + catch (e) {} + } madeChanges = true; sort = true; } @@ -873,6 +879,15 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio } } quicksearch.doCommand(); + // We have to wait for the search in order to select new items properly, but doCommand() + // doesn't provide the return value from the oncommand handler, so we can't wait for an + // asynchronous handler. But really they just end up calling refresh(), so we wait for that. + if (this._refreshPromise) { + try { + yield this._refreshPromise; + } + catch (e) {} + } madeChanges = true; sort = true; } diff --git a/test/tests/itemTreeViewTest.js b/test/tests/itemTreeViewTest.js index cd80a78eaa..3a4ee67a25 100644 --- a/test/tests/itemTreeViewTest.js +++ b/test/tests/itemTreeViewTest.js @@ -137,6 +137,28 @@ describe("Zotero.ItemTreeView", function() { assert.equal(selected[0], existingItemID); }); + it("should clear search and select new item if non-matching quick search is active", async function () { + await createDataObject('item'); + + var quicksearch = win.document.getElementById('zotero-tb-search'); + quicksearch.value = Zotero.randomString(); + quicksearch.doCommand(); + await itemsView._refreshPromise; + + assert.equal(itemsView.rowCount, 0); + + // Create item + var item = await createDataObject('item'); + + assert.isAbove(itemsView.rowCount, 0); + assert.equal(quicksearch.value, ''); + + // New item should be selected + var selected = itemsView.getSelectedItems(); + assert.lengthOf(selected, 1); + assert.equal(selected[0].id, item.id); + }); + it("shouldn't clear quicksearch if skipSelect is passed", function* () { var searchString = Zotero.Items.get(existingItemID).getField('title');