Skip to content

Commit

Permalink
Compatibility fixes for Bluebird >=3.4.2
Browse files Browse the repository at this point in the history
Before 3.4.2, `yield` in a Bluebird `coroutine()` released Zalgo if an
already-resolved promise (e.g., from `Promise.resolve()`) was yielded,
continuing immediately instead of on the next turn of the event loop. We
apparently relied on that broken behavior in a few places, particularly from
`Zotero.DB.waitForTransaction()`. All the transaction-queueing stuff probably
needs to be reevaluated in general, but for now, mirror the previous possibly
ill-advised behavior by checking explicitly for a transaction before yielding
on `waitForTransaction()` (as well as the result of a 'load' event from
`libraryTreeView`).
  • Loading branch information
dstillman committed Jan 14, 2017
1 parent 7e30afb commit 3d569f1
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 13 deletions.
2 changes: 1 addition & 1 deletion chrome/content/zotero/browser.js
Expand Up @@ -605,7 +605,7 @@ var Zotero_Browser = new function() {
return;
}

if (!Zotero.isConnector) {
if (!Zotero.isConnector && Zotero.DB.inTransaction()) {
yield Zotero.DB.waitForTransaction();
}

Expand Down
4 changes: 3 additions & 1 deletion chrome/content/zotero/xpcom/db.js
Expand Up @@ -953,7 +953,9 @@ Zotero.DBConnection.prototype.backupDatabase = Zotero.Promise.coroutine(function

// Start a promise that will be resolved when the backup is finished
var resolveBackupPromise;
yield this.waitForTransaction();
if (this.inTransaction()) {
yield this.waitForTransaction();
}
this._backupPromise = new Zotero.Promise(function () {
resolveBackupPromise = arguments[0];
});
Expand Down
5 changes: 3 additions & 2 deletions chrome/content/zotero/xpcom/schema.js
Expand Up @@ -1010,8 +1010,9 @@ Zotero.Schema = new function(){
return;
}

// If transaction already in progress, delay by ten minutes
yield Zotero.DB.waitForTransaction();
if (Zotero.DB.inTransaction()) {
yield Zotero.DB.waitForTransaction();
}

// Get the last timestamp we got from the server
var lastUpdated = yield this.getDBVersion('repository');
Expand Down
32 changes: 23 additions & 9 deletions chrome/content/zotero/zoteroPane.js
Expand Up @@ -947,7 +947,9 @@ var ZoteroPane = new function()


this.newSearch = Zotero.Promise.coroutine(function* () {
yield Zotero.DB.waitForTransaction();
if (Zotero.DB.inTransaction()) {
yield Zotero.DB.waitForTransaction();
}

var s = new Zotero.Search();
s.libraryID = this.getSelectedLibraryID();
Expand Down Expand Up @@ -1011,7 +1013,9 @@ var ZoteroPane = new function()


this.openLookupWindow = Zotero.Promise.coroutine(function* () {
yield Zotero.DB.waitForTransaction();
if (Zotero.DB.inTransaction()) {
yield Zotero.DB.waitForTransaction();
}

if (!this.canEdit()) {
this.displayCannotEditLibraryMessage();
Expand Down Expand Up @@ -1379,8 +1383,10 @@ var ZoteroPane = new function()
* be a better test for whether the item pane changed)
*/
this.itemSelected = function (event) {
return Zotero.spawn(function* () {
yield Zotero.DB.waitForTransaction();
return Zotero.Promise.coroutine(function* () {
if (Zotero.DB.inTransaction()) {
yield Zotero.DB.waitForTransaction();
}

// Don't select item until items list has loaded
//
Expand All @@ -1389,7 +1395,9 @@ var ZoteroPane = new function()
this.itemsView.addEventListener('load', function () {
deferred.resolve();
});
yield deferred.promise;
if (deferred.promise.isPending()) {
yield deferred.promise;
}

if (!this.itemsView || !this.itemsView.selection) {
Zotero.debug("Items view not available in itemSelected", 2);
Expand Down Expand Up @@ -1592,7 +1600,7 @@ var ZoteroPane = new function()
}

return true;
}, this)
}.bind(this))()
.finally(function () {
return this.itemsView.onSelect();
}.bind(this));
Expand Down Expand Up @@ -3686,7 +3694,9 @@ var ZoteroPane = new function()
//
// Duplicate newItem() checks here
//
yield Zotero.DB.waitForTransaction();
if (Zotero.DB.inTransaction()) {
yield Zotero.DB.waitForTransaction();
}

// Currently selected row
if (row === undefined && this.collectionsView && this.collectionsView.selection) {
Expand Down Expand Up @@ -3806,7 +3816,9 @@ var ZoteroPane = new function()
//
// Duplicate newItem() checks here
//
yield Zotero.DB.waitForTransaction();
if (Zotero.DB.inTransaction()) {
yield Zotero.DB.waitForTransaction();
}

// Currently selected row
if (row === undefined) {
Expand Down Expand Up @@ -3894,7 +3906,9 @@ var ZoteroPane = new function()
* |link| -- create web link instead of snapshot
*/
this.addAttachmentFromPage = Zotero.Promise.coroutine(function* (link, itemID) {
yield Zotero.DB.waitForTransaction();
if (Zotero.DB.inTransaction()) {
yield Zotero.DB.waitForTransaction();
}

if (typeof itemID != 'number') {
throw new Error("itemID must be an integer");
Expand Down

0 comments on commit 3d569f1

Please sign in to comment.