www

commit d0a1ac967763c114ad924542e5f3d2581a8c4f21
parent 18877a97483794db1ed7c675236653860c4d3610
Author: Dan Stillman <dstillman@zotero.org>
Date:   Mon, 27 Feb 2017 04:51:04 -0500

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.

Diffstat:
M
chrome/content/zotero/xpcom/collectionTreeView.js
 | 
10
++++++++++
M
test/tests/collectionTreeViewTest.js
 | 
23
+++++++++++++++++++++++

2 files changed, 33 insertions(+), 0 deletions(-)
diff --git 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
@@ -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* () {