From d0a1ac967763c114ad924542e5f3d2581a8c4f21 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Mon, 27 Feb 2017 04:51:04 -0500 Subject: [PATCH] Fix potential incorrect placement of new subcollections For one particular complicated collection structure, new collections could be placed in the wrong place until a restart. --- .../zotero/xpcom/collectionTreeView.js | 10 ++++++++ test/tests/collectionTreeViewTest.js | 23 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js index df186955f9..1f2bda6e8f 100644 --- a/chrome/content/zotero/xpcom/collectionTreeView.js +++ b/chrome/content/zotero/xpcom/collectionTreeView.js @@ -588,6 +588,16 @@ Zotero.CollectionTreeView.prototype._addSortedRow = Zotero.Promise.coroutine(fun } treeRow = this.getRow(i); rowLevel = this.getLevel(i); + // If going from lower level to a row higher than the target level, we found + // our place: + // + // - 1 + // - 3 + // - 4 + // - 2 <<<< 5, a sibling of 3, goes above here + if (rowLevel < level) { + break loop; + } } if (Zotero.localeCompare(treeRow.ref.name, collection.name) > 0) { diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js index a9c550fd64..25a2a27295 100644 --- a/test/tests/collectionTreeViewTest.js +++ b/test/tests/collectionTreeViewTest.js @@ -326,6 +326,29 @@ describe("Zotero.CollectionTreeView", function() { }) + it("should add collection after parent's subcollection and before non-sibling", function* () { + var c0 = yield createDataObject('collection', { name: "Test" }); + var rootRow = cv.getRowIndexByID(c0.treeViewID); + + var c1 = yield createDataObject('collection', { name: "1", parentID: c0.id }); + var c2 = yield createDataObject('collection', { name: "2", parentID: c0.id }); + var c3 = yield createDataObject('collection', { name: "3", parentID: c1.id }); + var c4 = yield createDataObject('collection', { name: "4", parentID: c3.id }); + var c5 = yield createDataObject('collection', { name: "5", parentID: c1.id }); + + assert.equal(cv.getRowIndexByID(c1.treeViewID), rootRow + 1); + + assert.isAbove(cv.getRowIndexByID(c1.treeViewID), cv.getRowIndexByID(c0.treeViewID)); + assert.isAbove(cv.getRowIndexByID(c2.treeViewID), cv.getRowIndexByID(c0.treeViewID)); + + assert.isAbove(cv.getRowIndexByID(c3.treeViewID), cv.getRowIndexByID(c1.treeViewID)); + assert.isAbove(cv.getRowIndexByID(c5.treeViewID), cv.getRowIndexByID(c1.treeViewID)); + assert.isBelow(cv.getRowIndexByID(c5.treeViewID), cv.getRowIndexByID(c2.treeViewID)); + + assert.equal(cv.getRowIndexByID(c4.treeViewID), cv.getRowIndexByID(c3.treeViewID) + 1); + }); + + it("should add multiple collections", function* () { var col1, col2; yield Zotero.DB.executeTransaction(function* () {