Skip to content

Commit

Permalink
Fix items list problems when adding item with a search entered
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dstillman committed Oct 26, 2017
1 parent 1a779bb commit 73d8842
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 1 deletion.
17 changes: 16 additions & 1 deletion chrome/content/zotero/xpcom/itemTreeView.js
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down
22 changes: 22 additions & 0 deletions test/tests/itemTreeViewTest.js
Expand Up @@ -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');

Expand Down

0 comments on commit 73d8842

Please sign in to comment.