Properly upload local changes after automatic conflict resolution

If an object changed on both sides and the changes were either
non-conflicting or identical but there were other local changes, the
local object was incorrectly being marked as synced, causing it not to
be uploaded until it was next modified locally.

chrome/content/zotero/xpcom/sync/syncLocal.js

@@ -886,9 +886,11 @@ Zotero.Sync.Data.Local = {
 									Zotero.debug("No remote changes to apply to local "
 										+ objectType + " " + obj.libraryKey);
 									saveOptions.skipData = true;
-									// If local object is different but we ignored the changes
+									// If either there were additional local changes after cancelling
-									// (e.g., ISBN hyphenation), save as unsynced. Since we're skipping
+									// out equivalent changes on both sides or the local object was
-									// data, the local version won't be overwritten.
+									// different but we ignored the changes (e.g., ISBN hyphenation),
+									// keep as unsynced. In the latter case, since we're skipping
+									// data, the local fields won't be overwritten.
 									if (result.localChanged) {
 										saveOptions.saveAsUnsynced = true;
 									}
@@ -940,6 +942,10 @@ Zotero.Sync.Data.Local = {
 									jsonDataLocal[x] = jsonData[x];
 								})
 								jsonObject.data = jsonDataLocal;
+								// If there were additional local changes, keep as unsynced
+								if (result.localChanged) {
+									saveOptions.saveAsUnsynced = true;
+								}
 							}
 						}
 						// Object doesn't exist locally
@@ -1535,6 +1541,7 @@ Zotero.Sync.Data.Local = {
 					let creators2 = c2.value;
 					if (creators1.length == creators2.length
 							&& creators1.every((c, index) => Zotero.Creators.equals(c, creators2[index]))) {
+						changeset1.splice(i--, 1);
 						changeset2.splice(j--, 1);
 						continue;
 					}
@@ -1551,6 +1558,7 @@ Zotero.Sync.Data.Local = {
 						|| (c1.op == 'member-remove' && c2.op == 'member-remove')
 						|| (c1.op == 'property-member-add' && c2.op == 'property-member-add')
 						|| (c1.op == 'property-member-remove' && c2.op == 'property-member-remove')) {
+					changeset1.splice(i--, 1);
 					changeset2.splice(j--, 1);
 					continue;
 				}
@@ -1558,6 +1566,7 @@ Zotero.Sync.Data.Local = {
 				// If both sides have values, see if they're the same, and if so remove the
 				// second one
 				if (c1.op != 'delete' && c2.op != 'delete' && c1.value === c2.value) {
+					changeset1.splice(i--, 1);
 					changeset2.splice(j--, 1);
 					continue;
 				}
@@ -1570,6 +1579,7 @@ Zotero.Sync.Data.Local = {
 				}
 				
 				// Conflict
+				changeset1.splice(i--, 1);
 				changeset2.splice(j--, 1);
 				conflicts.push([c1, c2]);
 			}
@@ -1577,7 +1587,10 @@ Zotero.Sync.Data.Local = {
 		
 		return {
 			changes: changeset2,
-			conflicts
+			conflicts,
+			// If there were unique local changes, we need to know that so the item can be kept as
+			// unsynced
+			localChanged: !!changeset1.length
 		};
 	},
 	

test/tests/syncLocalTest.js

@@ -526,7 +526,43 @@ describe("Zotero.Sync.Data.Local", function() {
 			assert.equal(obj.version, 10);
 			assert.equal(obj.getField('title'), changedTitle);
 			assert.equal(obj.getField('place'), changedPlace);
-		})
+			// Item should be marked as unsynced so the local changes are uploaded
+			assert.isFalse(obj.synced);
+		});
+		
+		it("should keep local item changes while ignoring matching remote changes", async function () {
+			var libraryID = Zotero.Libraries.userLibraryID;
+			
+			var type = 'item';
+			let obj = await createDataObject(type, { version: 5 });
+			let data = obj.toJSON();
+			await Zotero.Sync.Data.Local.saveCacheObjects(type, libraryID, [data]);
+			
+			// Change local title and place
+			await modifyDataObject(obj)
+			var changedTitle = obj.getField('title');
+			var changedPlace = 'New York';
+			obj.setField('place', changedPlace);
+			await obj.saveTx();
+			
+			// Create remote version without title but with changed place
+			data.key = obj.key;
+			data.version = 10;
+			data.place = changedPlace;
+			let json = {
+				key: obj.key,
+				version: 10,
+				data: data
+			};
+			await Zotero.Sync.Data.Local.processObjectsFromJSON(
+				type, libraryID, [json], { stopOnError: true }
+			);
+			assert.equal(obj.version, 10);
+			assert.equal(obj.getField('title'), changedTitle);
+			assert.equal(obj.getField('place'), changedPlace);
+			// Item should be marked as unsynced so the local changes are uploaded
+			assert.isFalse(obj.synced);
+		});
 		
 		it("should save item with overriding local conflict as unsynced", async function () {
 			var libraryID = Zotero.Libraries.userLibraryID;