Always include 'contentType'/'charset'/'filename' in attachment JSON

And omit in ZFS file sync requests

The API previously didn't allow these properties to be set for group items,
because they were set atomically during the file upload process, but 1) that's
not really necessary (makes a little sense for 'filename', but not really a big
deal if an old file is renamed on another computer before the new file is
synced down) and 2) skipping them results in the properties getting erased
after items are uploaded and the empty values returned by the server overwrite
the local values.

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

@@ -4114,22 +4114,17 @@ Zotero.Item.prototype.toJSON = function (options = {}) {
 			let linkMode = this.attachmentLinkMode;
 			obj.linkMode = Zotero.Attachments.linkModeToName(linkMode);
 			
-			let imported = this.isImportedAttachment();
+			obj.contentType = this.attachmentContentType;
-			let skipFileProps = options.skipImportedFileProperties;
+			obj.charset = this.attachmentCharset;
-			
-			if (!imported || !skipFileProps) {
-				obj.contentType = this.attachmentContentType;
-				obj.charset = this.attachmentCharset;
-			}
 			
 			if (linkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE) {
 				obj.path = this.attachmentPath;
 			}
-			else if (linkMode != Zotero.Attachments.LINK_MODE_LINKED_URL && !skipFileProps) {
+			else if (linkMode != Zotero.Attachments.LINK_MODE_LINKED_URL) {
 				obj.filename = this.attachmentFilename;
 			}
 			
-			if (imported && !skipFileProps) {
+			if (this.isImportedAttachment() && !options.skipStorageProperties) {
 				if (options.syncedStorageProperties) {
 					obj.mtime = this.attachmentSyncedModificationTime;
 					obj.md5 = this.attachmentSyncedHash;

chrome/content/zotero/xpcom/storage/zfs.js

@@ -335,14 +335,6 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = {
 			filename,
 			size: file.fileSize
 		};
-		var charset = item.attachmentCharset;
-		var contentType = item.attachmentContentType;
-		if (charset) {
-			json.charset = charset;
-		}
-		if (contentType) {
-			json.contentType = contentType;
-		}
 		if (zip) {
 			json.zip = true;
 		}
@@ -372,14 +364,6 @@ Zotero.Sync.Storage.Mode.ZFS.prototype = {
 			filename,
 			filesize: (yield OS.File.stat(uploadPath)).size
 		};
-		var charset = item.attachmentCharset;
-		var contentType = item.attachmentContentType;
-		if (charset) {
-			params.charset = charset;
-		}
-		if (contentType) {
-			params.contentType = contentType;
-		}
 		if (zip) {
 			params.zipMD5 = yield Zotero.Utilities.Internal.md5Async(uploadPath);
 			params.zipFilename = OS.Path.basename(uploadPath);

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

@@ -878,8 +878,8 @@ Zotero.Sync.Data.Engine.prototype._uploadObjects = Zotero.Promise.coroutine(func
 					objectType,
 					o.id,
 					{
-						// Only include file properties ('contentType', etc.) for WebDAV files
+						// Only storage properties ('mtime', 'md5') for WebDAV files
-						skipImportedFileProperties:
+						skipStorageProperties:
 							objectType == 'item'
 								? Zotero.Sync.Storage.Local.getModeForLibrary(this.library.libraryID)
 									!= 'webdav'
@@ -1073,8 +1073,8 @@ Zotero.Sync.Data.Engine.prototype._getJSONForObject = function (objectType, id,
 			includeKey: true,
 			includeVersion: true, // DEBUG: remove?
 			includeDate: true,
-			// Whether to skip 'contentType', 'filename', 'mtime', and 'md5'
+			// Whether to skip 'mtime' and 'md5'
-			skipImportedFileProperties: options.skipImportedFileProperties,
+			skipStorageProperties: options.skipStorageProperties,
 			// Use last-synced mtime/md5 instead of current values from the file itself
 			syncedStorageProperties: true,
 			patchBase: cacheObj ? cacheObj.data : false

test/tests/itemTest.js

@@ -1051,6 +1051,22 @@ describe("Zotero.Item", function () {
 				assert.equal(json.md5, (yield item.attachmentHash));
 			})
 			
+			it("should omit storage values with .skipStorageProperties", function* () {
+				var file = getTestDataDirectory();
+				file.append('test.png');
+				var item = yield Zotero.Attachments.importFromFile({ file });
+				
+				item.attachmentSyncedModificationTime = new Date().getTime();
+				item.attachmentSyncedHash = 'b32e33f529942d73bea4ed112310f804';
+				yield item.saveTx({ skipAll: true });
+				
+				var json = item.toJSON({
+					skipStorageProperties: true
+				});
+				assert.isUndefined(json.mtime);
+				assert.isUndefined(json.md5);
+			});
+			
 			it("should output synced storage values with .syncedStorageProperties", function* () {
 				var item = new Zotero.Item('attachment');
 				item.attachmentLinkMode = 'imported_file';

test/tests/syncEngineTest.js

@@ -787,7 +787,7 @@ describe("Zotero.Sync.Data.Engine", function () {
 		});
 		
 		
-		it("shouldn't include file properties for attachments in ZFS libraries", function* () {
+		it("shouldn't include storage properties for attachments in ZFS libraries", function* () {
 			({ engine, client, caller } = yield setup());
 			
 			var libraryID = Zotero.Libraries.userLibraryID;
@@ -812,9 +812,9 @@ describe("Zotero.Sync.Data.Engine", function () {
 						let itemJSON = json[0];
 						assert.equal(itemJSON.key, item.key);
 						assert.equal(itemJSON.version, 0);
-						assert.notProperty(itemJSON, "contentType");
+						assert.property(itemJSON, "contentType");
-						assert.notProperty(itemJSON, "charset");
+						assert.property(itemJSON, "charset");
-						assert.notProperty(itemJSON, "filename");
+						assert.property(itemJSON, "filename");
 						assert.notProperty(itemJSON, "mtime");
 						assert.notProperty(itemJSON, "md5");
 						req.respond(
@@ -840,7 +840,7 @@ describe("Zotero.Sync.Data.Engine", function () {
 		});
 		
 		
-		it("should include file properties for attachments in WebDAV libraries", function* () {
+		it("should include storage properties for attachments in WebDAV libraries", function* () {
 			({ engine, client, caller } = yield setup());
 			
 			var libraryID = Zotero.Libraries.userLibraryID;

test/tests/zfsTest.js

@@ -323,7 +323,6 @@ describe("Zotero.Sync.Storage.Mode.ZFS", function () {
 					assert.equal(params.mtime, mtime1);
 					assert.equal(params.filename, filename1);
 					assert.equal(params.filesize, size1);
-					assert.equal(params.contentType, contentType1);
 					
 					req.respond(
 						200,
@@ -375,8 +374,6 @@ describe("Zotero.Sync.Storage.Mode.ZFS", function () {
 					assert.equal(params.filename, filename2);
 					assert.equal(params.zipFilename, item2.key + ".zip");
 					assert.isTrue(parseInt(params.filesize) == params.filesize);
-					assert.equal(params.contentType, contentType2);
-					assert.equal(params.charset, charset2);
 					
 					req.respond(
 						200,