Auto-add fileid in URL for currently displayed folder

6 files changed, 111 insertions, 15 deletions

diff --git a/apps/files/js/app.js b/apps/files/js/app.js
index eac080a009d..60b5f1816ab 100644
--- a/apps/files/js/app.js
+++ b/apps/files/js/app.js
@@ -174,6 +174,7 @@
 
 			// detect when app changed their current directory
 			$('#app-content').delegate('>div', 'changeDirectory', _.bind(this._onDirectoryChanged, this));
+			$('#app-content').delegate('>div', 'afterChangeDirectory', _.bind(this._onAfterDirectoryChanged, this));
 			$('#app-content').delegate('>div', 'changeViewerMode', _.bind(this._onChangeViewerMode, this));
 
 			$('#app-navigation').on('itemChanged', _.bind(this._onNavigationChanged, this));
@@ -224,7 +225,16 @@
 		 */
 		_onDirectoryChanged: function(e) {
 			if (e.dir) {
-				this._changeUrl(this.navigation.getActiveItem(), e.dir);
+				this._changeUrl(this.navigation.getActiveItem(), e.dir, e.fileId);
+			}
+		},
+
+		/**
+		 * Event handler for when an app notified that its directory changed
+		 */
+		_onAfterDirectoryChanged: function(e) {
+			if (e.dir && e.fileId) {
+				this._changeUrl(this.navigation.getActiveItem(), e.dir, e.fileId);
 			}
 		},
 
@@ -263,12 +273,20 @@
 		/**
 		 * Change the URL to point to the given dir and view
 		 */
-		_changeUrl: function(view, dir) {
+		_changeUrl: function(view, dir, fileId) {
 			var params = {dir: dir};
 			if (view !== 'files') {
 				params.view = view;
+			} else if (fileId) {
+				params.fileid = fileId;
+			}
+			var currentParams = OC.Util.History.parseUrlQuery();
+			if (currentParams.dir === params.dir && currentParams.view === params.view && currentParams.fileid !== params.fileid) {
+				// if only fileid changed or was added, replace instead of push
+				OC.Util.History.replaceState(params);
+			} else {
+				OC.Util.History.pushState(params);
 			}
-			OC.Util.History.pushState(params);
 		}
 	};
 })();
diff --git a/apps/files/js/fileactions.js b/apps/files/js/fileactions.js
index 69e32d500c4..c3d4fba9ef5 100644
--- a/apps/files/js/fileactions.js
+++ b/apps/files/js/fileactions.js
@@ -619,7 +619,7 @@
 
 			this.register('dir', 'Open', OC.PERMISSION_READ, '', function (filename, context) {
 				var dir = context.$file.attr('data-path') || context.fileList.getCurrentDirectory();
-				context.fileList.changeDirectory(OC.joinPaths(dir, filename));
+				context.fileList.changeDirectory(OC.joinPaths(dir, filename), true, false, parseInt(context.$file.attr('data-id'), 10));
 			});
 
 			this.registerAction({
diff --git a/apps/files/js/filelist.js b/apps/files/js/filelist.js
index 16ca5e91ed2..9395112bce3 100644
--- a/apps/files/js/filelist.js
+++ b/apps/files/js/filelist.js
@@ -1362,19 +1362,20 @@
 			return parseInt(this.$el.find('#permissions').val(), 10);
 		},
 		/**
-		 * @brief Changes the current directory and reload the file list.
-		 * @param targetDir target directory (non URL encoded)
-		 * @param changeUrl false if the URL must not be changed (defaults to true)
-		 * @param {boolean} force set to true to force changing directory
+		 * Changes the current directory and reload the file list.
+		 * @param {string} targetDir target directory (non URL encoded)
+		 * @param {boolean} [changeUrl=true] if the URL must not be changed (defaults to true)
+		 * @param {boolean} [force=false] set to true to force changing directory
+		 * @param {string} [fileId] optional file id, if known, to be appended in the URL
 		 */
-		changeDirectory: function(targetDir, changeUrl, force) {
+		changeDirectory: function(targetDir, changeUrl, force, fileId) {
 			var self = this;
 			var currentDir = this.getCurrentDirectory();
 			targetDir = targetDir || '/';
 			if (!force && currentDir === targetDir) {
 				return;
 			}
-			this._setCurrentDir(targetDir, changeUrl);
+			this._setCurrentDir(targetDir, changeUrl, fileId);
 			this.reload().then(function(success){
 				if (!success) {
 					self.changeDirectory(currentDir, true);
@@ -1389,8 +1390,9 @@
 		 * Sets the current directory name and updates the breadcrumb.
 		 * @param targetDir directory to display
 		 * @param changeUrl true to also update the URL, false otherwise (default)
+		 * @param {string} [fileId] file id
 		 */
-		_setCurrentDir: function(targetDir, changeUrl) {
+		_setCurrentDir: function(targetDir, changeUrl, fileId) {
 			targetDir = targetDir.replace(/\\/g, '/');
 			var previousDir = this.getCurrentDirectory(),
 				baseDir = OC.basename(targetDir);
@@ -1408,10 +1410,14 @@
 			this.$el.find('#dir').val(targetDir);
 
 			if (changeUrl !== false) {
-				this.$el.trigger(jQuery.Event('changeDirectory', {
+				var params = {
 					dir: targetDir,
 					previousDir: previousDir
-				}));
+				};
+				if (fileId) {
+					params.fileId = fileId;
+				}
+				this.$el.trigger(jQuery.Event('changeDirectory', params));
 			}
 			this.breadcrumb.setDirectory(this.getCurrentDirectory());
 		},
@@ -1557,6 +1563,18 @@
 
 			result.sort(this._sortComparator);
 			this.setFiles(result);
+
+			if (this.dirInfo) {
+				var newFileId = this.dirInfo.id;
+				// update fileid in URL
+				var params = {
+					dir: this.getCurrentDirectory()
+				};
+				if (newFileId) {
+					params.fileId = newFileId;
+				}
+				this.$el.trigger(jQuery.Event('afterChangeDirectory', params));
+			}
 			return true;
 		},
 
diff --git a/apps/files/tests/js/appSpec.js b/apps/files/tests/js/appSpec.js
index dce513339f0..3070d687fc5 100644
--- a/apps/files/tests/js/appSpec.js
+++ b/apps/files/tests/js/appSpec.js
@@ -22,6 +22,7 @@
 describe('OCA.Files.App tests', function() {
 	var App = OCA.Files.App;
 	var pushStateStub;
+	var replaceStateStub;
 	var parseUrlQueryStub;
 	var oldLegacyFileActions;
 
@@ -48,6 +49,7 @@ describe('OCA.Files.App tests', function() {
 		OCA.Files.fileActions = new OCA.Files.FileActions();
 
 		pushStateStub = sinon.stub(OC.Util.History, 'pushState');
+		replaceStateStub = sinon.stub(OC.Util.History, 'replaceState');
 		parseUrlQueryStub = sinon.stub(OC.Util.History, 'parseUrlQuery');
 		parseUrlQueryStub.returns({});
 
@@ -59,6 +61,7 @@ describe('OCA.Files.App tests', function() {
 		window.FileActions = oldLegacyFileActions;
 
 		pushStateStub.restore();
+		replaceStateStub.restore();
 		parseUrlQueryStub.restore();
 	});
 
@@ -132,6 +135,23 @@ describe('OCA.Files.App tests', function() {
 			expect(pushStateStub.getCall(0).args[0].dir).toEqual('subdir');
 			expect(pushStateStub.getCall(0).args[0].view).toEqual('other');
 		});
+		it('replaces the state to the URL when fileid is known', function() {
+			$('#app-content-files').trigger(new $.Event('changeDirectory', {dir: 'subdir'}));
+			expect(pushStateStub.calledOnce).toEqual(true);
+			expect(pushStateStub.getCall(0).args[0].dir).toEqual('subdir');
+			expect(pushStateStub.getCall(0).args[0].view).not.toBeDefined();
+			expect(replaceStateStub.notCalled).toEqual(true);
+
+			parseUrlQueryStub.returns({dir: 'subdir'});
+
+			$('#app-content-files').trigger(new $.Event('afterChangeDirectory', {dir: 'subdir', fileId: 123}));
+
+			expect(pushStateStub.calledOnce).toEqual(true);
+			expect(replaceStateStub.calledOnce).toEqual(true);
+			expect(replaceStateStub.getCall(0).args[0].dir).toEqual('subdir');
+			expect(replaceStateStub.getCall(0).args[0].view).not.toBeDefined();
+			expect(replaceStateStub.getCall(0).args[0].fileid).toEqual(123);
+		});
 		describe('onpopstate', function() {
 			it('sends "urlChanged" event to current app', function() {
 				var handler = sinon.stub();
diff --git a/apps/files/tests/js/filelistSpec.js b/apps/files/tests/js/filelistSpec.js
index cc3bcd74b46..7e6408128bb 100644
--- a/apps/files/tests/js/filelistSpec.js
+++ b/apps/files/tests/js/filelistSpec.js
@@ -1358,6 +1358,15 @@ describe('OCA.Files.FileList tests', function() {
 			expect(handler.calledOnce).toEqual(true);
 			expect(handler.getCall(0).args[0].dir).toEqual('/somedir');
 		});
+		it('triggers "afterChangeDirectory" event with fileid after changing directory', function() {
+			var handler = sinon.stub();
+			$('#app-content-files').on('afterChangeDirectory', handler);
+			fileList.changeDirectory('/somedir');
+			deferredList.resolve(200, [testRoot].concat(testFiles));
+			expect(handler.calledOnce).toEqual(true);
+			expect(handler.getCall(0).args[0].dir).toEqual('/somedir');
+			expect(handler.getCall(0).args[0].fileId).toEqual(99);
+		});
 		it('changes the directory when receiving "urlChanged" event', function() {
 			$('#app-content-files').trigger(new $.Event('urlChanged', {view: 'files', dir: '/somedir'}));
 			expect(fileList.getCurrentDirectory()).toEqual('/somedir');
diff --git a/core/js/js.js b/core/js/js.js
index 69ebabdb419..edee72ca3ca 100644
--- a/core/js/js.js
+++ b/core/js/js.js
@@ -2050,8 +2050,9 @@ OC.Util.History = {
 	 *
 	 * @param params to append to the URL, can be either a string
 	 * or a map
+	 * @param {boolean} [replace=false] whether to replace instead of pushing
 	 */
-	pushState: function(params) {
+	_pushState: function(params, replace) {
 		var strParams;
 		if (typeof(params) === 'string') {
 			strParams = params;
@@ -2061,7 +2062,11 @@ OC.Util.History = {
 		}
 		if (window.history.pushState) {
 			var url = location.pathname + '?' + strParams;
-			window.history.pushState(params, '', url);
+			if (replace) {
+				window.history.replaceState(params, '', url);
+			} else {
+				window.history.pushState(params, '', url);
+			}
 		}
 		// use URL hash for IE8
 		else {
@@ -2073,6 +2078,32 @@ OC.Util.History = {
 	},
 
 	/**
+	 * Push the current URL parameters to the history stack
+	 * and change the visible URL.
+	 * Note: this includes a workaround for IE8/IE9 that uses
+	 * the hash part instead of the search part.
+	 *
+	 * @param params to append to the URL, can be either a string
+	 * or a map
+	 */
+	pushState: function(params) {
+		return this._pushState(params, false);
+	},
+
+	/**
+	 * Push the current URL parameters to the history stack
+	 * and change the visible URL.
+	 * Note: this includes a workaround for IE8/IE9 that uses
+	 * the hash part instead of the search part.
+	 *
+	 * @param params to append to the URL, can be either a string
+	 * or a map
+	 */
+	replaceState: function(params) {
+		return this._pushState(params, true);
+	},
+
+	/**
 	 * Add a popstate handler
 	 *
 	 * @param handler function