Prevent setting parent item to non-regular item

Except for embedded-image attachments under notes and annotations under
attachments

chrome/content/zotero/xpcom/data/item.js

@@ -1420,11 +1420,23 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 		? (this.ObjectsClass.getIDFromLibraryAndKey(this.libraryID, parentItemKey) || null)
 		: null;
 	if (this._changed.parentKey) {
-		if (isNew) {
+		if (parentItemKey && parentItemKey == this.key) {
-			if (parentItemKey == this.key) {
+			throw new Error("Item cannot be set as parent of itself");
-				throw new Error("Item cannot be set as parent of itself");
+		}
+		
+		// Make sure parent is a regular item
+		if (parentItemID) {
+			let parentItem = yield Zotero.Items.getAsync(parentItemID);
+			if (!parentItem.isRegularItem()
+					// Allow embedded-image attachments under notes
+					&& !(this.isEmbeddedImageAttachment() && parentItem.isNote())
+					// Allow annotations under attachments
+					&& !(this.isAnnotation() && parentItem.isFileAttachment())) {
+				throw new Error(`Parent item ${parentItem.libraryKey} must be a regular item`);
 			}
-			
+		}
+		
+		if (isNew) {
 			if (!parentItemID) {
 				// TODO: clear caches?
 				let msg = "Parent item " + this.libraryID + "/" + parentItemKey + " not found";
@@ -1451,10 +1463,6 @@ Zotero.Item.prototype._saveData = Zotero.Promise.coroutine(function* (env) {
 		}
 		else {
 			if (parentItemKey) {
-				if (parentItemKey == this.key) {
-					throw new Error("Item cannot be set as parent of itself");
-				}
-				
 				if (!parentItemID) {
 					// TODO: clear caches
 					let msg = "Parent item " + this.libraryID + "/" + parentItemKey + " not found";

test/tests/itemTest.js

@@ -1544,6 +1544,41 @@ describe("Zotero.Item", function () {
 			assert.equal(e.message, "Item type must be set before saving");
 		})
 		
+		describe("saving a child item", function () {
+			it("should throw an error if a new note is the child of another note", async function () {
+				var note1 = await createDataObject('item', { itemType: 'note' });
+				var note2 = createUnsavedDataObject('item', { itemType: 'note', parentID: note1.id });
+				var e = await getPromiseError(note2.saveTx());
+				assert.ok(e);
+				assert.include(e.message, "must be a regular item");
+			});
+			
+			it("should throw an error if a new imported_file attachment is the child of a note", async function () {
+				var note = await createDataObject('item', { itemType: 'note' });
+				var e = await getPromiseError(importFileAttachment('test.png', { parentItemID: note.id }));
+				assert.ok(e);
+				assert.include(e.message, "must be a regular item");
+			});
+			
+			it("should throw an error if a new note is the child of another attachment", async function () {
+				var attachment = await importFileAttachment('test.png');
+				var note = createUnsavedDataObject('item', { itemType: 'note', parentID: attachment.id });
+				var e = await getPromiseError(note.saveTx());
+				assert.ok(e);
+				assert.include(e.message, "must be a regular item");
+			});
+			
+			it("should throw an error if an existing note is set as a child of another note", async function () {
+				var note1 = await createDataObject('item', { itemType: 'note' });
+				var note2 = createUnsavedDataObject('item', { itemType: 'note' });
+				await note2.saveTx();
+				note2.parentID = note1.id;
+				var e = await getPromiseError(note2.saveTx());
+				assert.ok(e);
+				assert.include(e.message, "must be a regular item");
+			});
+		});
+		
 		it("should reload child items for parent items", function* () {
 			var item = yield createDataObject('item');
 			var attachment = yield importFileAttachment('test.png', { parentItemID: item.id });