Merge pull request #20122 from owncloud/files-consolidateiconupdate

Fix icon update to be more consistent

5 files changed, 86 insertions, 8 deletions

diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js
index c84d6c3c47d..99804b1b9be 100644
--- a/apps/files/js/filelist.js
+++ b/apps/files/js/filelist.js
@@ -767,7 +767,7 @@
 		 */
 		elementToFile: function($el){
 			$el = $($el);
-			return {
+			var data = {
 				id: parseInt($el.attr('data-id'), 10),
 				name: $el.attr('data-file'),
 				mimetype: $el.attr('data-mime'),
@@ -777,6 +777,15 @@
 				etag: $el.attr('data-etag'),
 				permissions: parseInt($el.attr('data-permissions'), 10)
 			};
+			var icon = $el.attr('data-icon');
+			if (icon) {
+				data.icon = icon;
+			}
+			var mountType = $el.attr('data-mounttype');
+			if (mountType) {
+				data.mountType = mountType;
+			}
+			return data;
 		},
 
 		/**
@@ -899,11 +908,12 @@
 				mtime = parseInt(fileData.mtime, 10),
 				mime = fileData.mimetype,
 				path = fileData.path,
+				dataIcon = null,
 				linkUrl;
 			options = options || {};
 
 			if (isNaN(mtime)) {
-				mtime = new Date().getTime()
+				mtime = new Date().getTime();
 			}
 
 			if (type === 'dir') {
@@ -911,6 +921,7 @@
 
 				if (fileData.mountType && fileData.mountType.indexOf('external') === 0) {
 					icon = OC.MimeType.getIconUrl('dir-external');
+					dataIcon = icon;
 				}
 			}
 
@@ -926,6 +937,11 @@
 				"data-permissions": fileData.permissions || this.getDirectoryPermissions()
 			});
 
+			if (dataIcon) {
+				// icon override
+				tr.attr('data-icon', dataIcon);
+			}
+
 			if (fileData.mountType) {
 				tr.attr('data-mounttype', fileData.mountType);
 			}
@@ -1177,7 +1193,7 @@
 			// display actions
 			this.fileActions.display(filenameTd, !options.silent, this);
 
-			if (fileData.isPreviewAvailable) {
+			if (fileData.isPreviewAvailable && mime !== 'httpd/unix-directory') {
 				var iconDiv = filenameTd.find('.thumbnail');
 				// lazy load / newly inserted td ?
 				// the typeof check ensures that the default value of animate is true
diff --git a/apps/files/js/mainfileinfodetailview.js b/apps/files/js/mainfileinfodetailview.js
index abf7da52ff4..69c796e492f 100644
--- a/apps/files/js/mainfileinfodetailview.js
+++ b/apps/files/js/mainfileinfodetailview.js
@@ -128,8 +128,8 @@
 					$iconDiv.addClass('icon-loading icon-32');
 					this.loadPreview(this.model.getFullPath(), this.model.get('mimetype'), this.model.get('etag'), $iconDiv, $container, this.model.isImage());
 				} else {
-					// TODO: special icons / shared / external
-					$iconDiv.css('background-image', 'url("' + OC.MimeType.getIconUrl('dir') + '")');
+					var iconUrl = this.model.get('icon') || OC.MimeType.getIconUrl('dir');
+					$iconDiv.css('background-image', 'url("' + iconUrl + '")');
 					OC.Util.scaleFixForIE8($iconDiv);
 				}
 				this.$el.find('[title]').tooltip({placement: 'bottom'});
diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js
index 994e1d32844..05e6fcc6122 100644
--- a/apps/files/tests/js/filelistSpec.js
+++ b/apps/files/tests/js/filelistSpec.js
@@ -1166,7 +1166,7 @@ describe('OCA.Files.FileList tests', function() {
 		it('renders provided icon for file when provided', function() {
 			var fileData = {
 				type: 'file',
-				name: 'test dir',
+				name: 'test file',
 				icon: OC.webroot + '/core/img/filetypes/application-pdf.svg',
 				mimetype: 'application/pdf'
 			};
@@ -1178,7 +1178,7 @@ describe('OCA.Files.FileList tests', function() {
 		it('renders preview when no icon was provided and preview is available', function() {
 			var fileData = {
 				type: 'file',
-				name: 'test dir',
+				name: 'test file',
 				isPreviewAvailable: true
 			};
 			var $tr = fileList.add(fileData);
@@ -1192,7 +1192,7 @@ describe('OCA.Files.FileList tests', function() {
 		it('renders default file type icon when no icon was provided and no preview is available', function() {
 			var fileData = {
 				type: 'file',
-				name: 'test dir',
+				name: 'test file',
 				isPreviewAvailable: false
 			};
 			var $tr = fileList.add(fileData);
@@ -1200,6 +1200,47 @@ describe('OCA.Files.FileList tests', function() {
 			expect(OC.TestUtil.getImageUrl($imgDiv)).toEqual(OC.webroot + '/core/img/filetypes/file.svg');
 			expect(previewLoadStub.notCalled).toEqual(true);
 		});
+		it('does not render preview for directories', function() {
+			var fileData = {
+				type: 'dir',
+				mimetype: 'httpd/unix-directory',
+				name: 'test dir',
+				isPreviewAvailable: true
+			};
+			var $tr = fileList.add(fileData);
+			var $td = $tr.find('td.filename');
+			expect(OC.TestUtil.getImageUrl($td.find('.thumbnail'))).toEqual(OC.webroot + '/core/img/filetypes/folder.svg');
+			expect(previewLoadStub.notCalled).toEqual(true);
+		});
+		it('render external storage icon for external storage root', function() {
+			var fileData = {
+				type: 'dir',
+				mimetype: 'httpd/unix-directory',
+				name: 'test dir',
+				isPreviewAvailable: true,
+				mountType: 'external-root'
+			};
+			var $tr = fileList.add(fileData);
+			var $td = $tr.find('td.filename');
+			expect(OC.TestUtil.getImageUrl($td.find('.thumbnail'))).toEqual(OC.webroot + '/core/img/filetypes/folder-external.svg');
+			expect(previewLoadStub.notCalled).toEqual(true);
+		});
+		it('render external storage icon for external storage subdir', function() {
+			var fileData = {
+				type: 'dir',
+				mimetype: 'httpd/unix-directory',
+				name: 'test dir',
+				isPreviewAvailable: true,
+				mountType: 'external'
+			};
+			var $tr = fileList.add(fileData);
+			var $td = $tr.find('td.filename');
+			expect(OC.TestUtil.getImageUrl($td.find('.thumbnail'))).toEqual(OC.webroot + '/core/img/filetypes/folder-external.svg');
+			expect(previewLoadStub.notCalled).toEqual(true);
+			// default icon override
+			expect($tr.attr('data-icon')).toEqual(OC.webroot + '/core/img/filetypes/folder-external.svg');
+		});
+
 	});
 	describe('viewer mode', function() {
 		it('enabling viewer mode hides files table and action buttons', function() {
diff --git a/apps/files/tests/js/mainfileinfodetailviewSpec.js b/apps/files/tests/js/mainfileinfodetailviewSpec.js
index f4403196f2e..460629806c8 100644
--- a/apps/files/tests/js/mainfileinfodetailviewSpec.js
+++ b/apps/files/tests/js/mainfileinfodetailviewSpec.js
@@ -112,6 +112,20 @@ describe('OCA.Files.MainFileInfoDetailView tests', function() {
 
 			lazyLoadPreviewStub.restore();
 		});
+		it('uses icon from model if present in model', function() {
+			var lazyLoadPreviewStub = sinon.stub(fileList, 'lazyLoadPreview');
+			testFileInfo.set('mimetype', 'httpd/unix-directory');
+			testFileInfo.set('icon', OC.MimeType.getIconUrl('dir-external'));
+			view.setFileInfo(testFileInfo);
+
+			expect(lazyLoadPreviewStub.notCalled).toEqual(true);
+
+			expect(view.$el.find('.thumbnail').hasClass('icon-loading')).toEqual(false);
+			expect(view.$el.find('.thumbnail').css('background-image'))
+				.toContain('filetypes/folder-external.svg');
+
+			lazyLoadPreviewStub.restore();
+		});
 		it('displays thumbnail', function() {
 			var lazyLoadPreviewStub = sinon.stub(fileList, 'lazyLoadPreview');
 
diff --git a/apps/files_sharing/js/share.js b/apps/files_sharing/js/share.js
index 30a803f3207..63225a0d8ec 100644
--- a/apps/files_sharing/js/share.js
+++ b/apps/files_sharing/js/share.js
@@ -130,6 +130,13 @@
 					// remove icon, if applicable
 					OC.Share.markFileAsShared($tr, false, false);
 				}
+				var newIcon = $tr.attr('data-icon');
+				// in case markFileAsShared decided to change the icon,
+				// we need to modify the model
+				// (FIXME: yes, this is hacky)
+				if (fileInfoModel.get('icon') !== newIcon) {
+					fileInfoModel.set('icon', newIcon);
+				}
 			});
 			fileList.registerTabView(shareTab);
 		},