From 02a36eab9b97298bd70e872e2d557e2b131f6480 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Tue, 5 May 2015 02:35:04 -0400 Subject: [PATCH] Fix various issues with rapid UI/data changes due to asyncification --- .../content/zotero/bindings/tagselector.xml | 26 ++++++------ chrome/content/zotero/overlay.js | 7 +++- .../zotero/xpcom/collectionTreeView.js | 8 ++-- chrome/content/zotero/xpcom/data/tags.js | 4 +- chrome/content/zotero/xpcom/itemTreeView.js | 42 ++++++++++++------- chrome/content/zotero/zoteroPane.js | 12 ++++-- test/tests/collectionTreeViewTest.js | 2 + test/tests/dbTest.js | 10 ++++- 8 files changed, 69 insertions(+), 42 deletions(-) diff --git a/chrome/content/zotero/bindings/tagselector.xml b/chrome/content/zotero/bindings/tagselector.xml index 2bee846fc..567f93f2d 100644 --- a/chrome/content/zotero/bindings/tagselector.xml +++ b/chrome/content/zotero/bindings/tagselector.xml @@ -600,20 +600,18 @@ diff --git a/chrome/content/zotero/overlay.js b/chrome/content/zotero/overlay.js index ebe11ce98..8de42e036 100644 --- a/chrome/content/zotero/overlay.js +++ b/chrome/content/zotero/overlay.js @@ -93,7 +93,12 @@ var ZoteroOverlay = new function() prefBranch.clearUserPref('statusBarIcon'); // Add toolbar icon - Services.scriptloader.loadSubScript("chrome://zotero/content/icon.js", {}, "UTF-8"); + try { + Services.scriptloader.loadSubScript("chrome://zotero/content/icon.js", {}, "UTF-8"); + } + catch (e) { + Zotero.logError(e); + } // TODO: Add only when progress window is open document.getElementById('appcontent').addEventListener('mousemove', Zotero.ProgressWindowSet.updateTimers, false); diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js index 8b2ba4724..c2f22a4b9 100644 --- a/chrome/content/zotero/xpcom/collectionTreeView.js +++ b/chrome/content/zotero/xpcom/collectionTreeView.js @@ -2392,7 +2392,7 @@ Zotero.CollectionTreeRow.prototype.getItems = Zotero.Promise.coroutine(function* if (!ids.length) { return [] } - return Zotero.Items.get(ids); + return Zotero.Items.getAsync(ids); }); Zotero.CollectionTreeRow.prototype.getSearchResults = Zotero.Promise.coroutine(function* (asTempTable) { @@ -2493,7 +2493,7 @@ Zotero.CollectionTreeRow.prototype.getSearchObject = Zotero.Promise.coroutine(fu * * @return {Promise} */ -Zotero.CollectionTreeRow.prototype.getChildTags = Zotero.Promise.method(function () { +Zotero.CollectionTreeRow.prototype.getChildTags = Zotero.Promise.coroutine(function* () { switch (this.type) { // TODO: implement? case 'share': @@ -2502,8 +2502,8 @@ Zotero.CollectionTreeRow.prototype.getChildTags = Zotero.Promise.method(function case 'bucket': return false; } - - return Zotero.Tags.getAllWithinSearchResults(this.getSearchResults(true)); + var results = yield this.getSearchResults(true); + return Zotero.Tags.getAllWithinSearchResults(results); }); diff --git a/chrome/content/zotero/xpcom/data/tags.js b/chrome/content/zotero/xpcom/data/tags.js index b889f938c..46e74c042 100644 --- a/chrome/content/zotero/xpcom/data/tags.js +++ b/chrome/content/zotero/xpcom/data/tags.js @@ -155,13 +155,11 @@ Zotero.Tags = new function() { /** * Get all tags within the items of a temporary table of search results * - * @param {String|Promise} tmpTable Temporary table with items to use + * @param {String} tmpTable Temporary table with items to use * @param {Array} [types] Array of tag types to fetch * @return {Promise} Promise for object with tag data in API JSON format, keyed by tagID */ this.getAllWithinSearchResults = Zotero.Promise.coroutine(function* (tmpTable, types) { - tmpTable = yield Zotero.Promise.resolve(tmpTable); - var sql = "SELECT DISTINCT name AS tag, type FROM itemTags " + "JOIN tags USING (tagID) WHERE itemID IN " + "(SELECT itemID FROM " + tmpTable + ") "; diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js index de4abbe37..e7de1bdcb 100644 --- a/chrome/content/zotero/xpcom/itemTreeView.js +++ b/chrome/content/zotero/xpcom/itemTreeView.js @@ -64,10 +64,10 @@ Zotero.ItemTreeView.prototype.type = 'item'; /** * Called by the tree itself */ -Zotero.ItemTreeView.prototype.setTree = Zotero.Promise.coroutine(function* (treebox) +Zotero.ItemTreeView.prototype.setTree = Zotero.serial(Zotero.Promise.coroutine(function* (treebox) { try { - //Zotero.debug("Calling setTree()"); + Zotero.debug("Setting item tree"); var start = Date.now(); // Try to set the window document if not yet set if (treebox && !this._ownerDocument) { @@ -85,7 +85,13 @@ Zotero.ItemTreeView.prototype.setTree = Zotero.Promise.coroutine(function* (tree } if (!treebox) { - Components.utils.reportError("Passed treebox empty in setTree()"); + Zotero.debug("Treebox not passed in setTree()", 2); + return; + } + + if (!this._ownerDocument) { + Zotero.debug("No owner document in setTree()", 2); + return; } this._treebox = treebox; @@ -103,8 +109,6 @@ Zotero.ItemTreeView.prototype.setTree = Zotero.Promise.coroutine(function* (tree return; } - yield Zotero.DB.waitForTransaction(); - yield this.refresh(); // Add a keypress listener for expand/collapse @@ -235,10 +239,8 @@ Zotero.ItemTreeView.prototype.setTree = Zotero.Promise.coroutine(function* (tree tree._handleEnter = function () {}; yield this.sort(); - yield this.expandMatchParents(); - if (this._ownerDocument.defaultView.ZoteroPane_Local) { this._ownerDocument.defaultView.ZoteroPane_Local.clearItemsPaneMessage(); } @@ -256,13 +258,14 @@ Zotero.ItemTreeView.prototype.setTree = Zotero.Promise.coroutine(function* (tree yield this._runListeners('load'); } catch (e) { + Zotero.debug(e, 1); Components.utils.reportError(e); if (this.onError) { this.onError(e); } throw e; - }; -}); + } +})); /** @@ -331,27 +334,32 @@ Zotero.ItemTreeView.prototype.refresh = Zotero.serial(Zotero.Promise.coroutine(f var added = 0; for (let i=0, len=newItems.length; i < len; i++) { + let item = newItems[i]; + // Only add regular items if sourcesOnly is set - if (this._sourcesOnly && !newItems[i].isRegularItem()) { + if (this._sourcesOnly && !item.isRegularItem()) { continue; } // Don't add child items directly (instead mark their parents for // inclusion below) - let parentItemID = newItems[i].parentItemID; + let parentItemID = item.parentItemID; if (parentItemID) { newSearchParentIDs[parentItemID] = true; } // Add top-level items else { + yield item.loadItemData(); + yield item.loadCollections(); + this._addRow( newRows, - new Zotero.ItemTreeRow(newItems[i], 0, false), + new Zotero.ItemTreeRow(item, 0, false), added + 1 ); added++; } - newSearchItemIDs[newItems[i].id] = true; + newSearchItemIDs[item.id] = true; } // Add parents of matches if not matches themselves @@ -925,9 +933,13 @@ Zotero.ItemTreeView.prototype.notify = Zotero.Promise.coroutine(function* (actio } //this._treebox.endUpdateBatch(); - var promise = this._getItemSelectedPromise(); + if (madeChanges) { + var promise = this._getItemSelectedPromise(); + } this.selection.selectEventsSuppressed = false; - yield promise; + if (madeChanges) { + yield promise; + } }); /* diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 905c4d8d7..5108fd9d5 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -1111,9 +1111,10 @@ var ZoteroPane = new function() this.onCollectionSelected = Zotero.Promise.coroutine(function* () { - yield Zotero.DB.waitForTransaction(); - var collectionTreeRow = this.getCollectionTreeRow(); + if (!collectionTreeRow) { + return; + } if (this.itemsView && this.itemsView.collectionTreeRow == collectionTreeRow) { Zotero.debug("Collection selection hasn't changed"); @@ -1131,7 +1132,12 @@ var ZoteroPane = new function() // Clear quick search and tag selector when switching views document.getElementById('zotero-tb-search').value = ""; - yield document.getElementById('zotero-tag-selector').clearAll(); + + // XBL functions might not yet be available + var tagSelector = document.getElementById('zotero-tag-selector'); + if (tagSelector.clearAll) { + tagSelector.clearAll(); + } // Not necessary with seltype="cell", which calls nsITreeView::isSelectable() /*if (collectionTreeRow.isSeparator()) { diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js index 6ff37d7a6..1a32f1986 100644 --- a/test/tests/collectionTreeViewTest.js +++ b/test/tests/collectionTreeViewTest.js @@ -1,3 +1,5 @@ +"use strict"; + describe("Zotero.CollectionTreeView", function() { var win, collectionsView; diff --git a/test/tests/dbTest.js b/test/tests/dbTest.js index 084e319ee..b12ca1364 100644 --- a/test/tests/dbTest.js +++ b/test/tests/dbTest.js @@ -1,11 +1,17 @@ describe("Zotero.DB", function() { var tmpTable = "tmpDBTest"; + before(function* () { + this.timeout(5000); + Zotero.debug("Waiting for DB activity to settle"); + yield Zotero.DB.waitForTransaction(); + yield Zotero.Promise.delay(1000); + }); beforeEach(function* () { - Zotero.DB.queryAsync("DROP TABLE IF EXISTS " + tmpTable); + yield Zotero.DB.queryAsync("DROP TABLE IF EXISTS " + tmpTable); }); after(function* () { - Zotero.DB.queryAsync("DROP TABLE IF EXISTS " + tmpTable); + yield Zotero.DB.queryAsync("DROP TABLE IF EXISTS " + tmpTable); }); describe("#executeTransaction()", function () {