Merge "mediawiki.storage: Provide a wrapper for sessionStorage too"

3 files changed, 111 insertions, 64 deletions

diff --git a/maintenance/jsduck/categories.json b/maintenance/jsduck/categories.json
index aad85da4a44d..9fe5009acdeb 100644
--- a/maintenance/jsduck/categories.json
+++ b/maintenance/jsduck/categories.json
@@ -27,6 +27,7 @@
 					"mw.notification",
 					"mw.Notification_",
 					"mw.storage",
+					"mw.storage.session",
 					"mw.user",
 					"mw.util",
 					"mw.plugin.*",
diff --git a/resources/src/mediawiki/mediawiki.storage.js b/resources/src/mediawiki/mediawiki.storage.js
index a9d17ff70dc1..20f8efb659ad 100644
--- a/resources/src/mediawiki/mediawiki.storage.js
+++ b/resources/src/mediawiki/mediawiki.storage.js
@@ -1,65 +1,91 @@
 ( function ( mw ) {
 	'use strict';
 
-	/**
-	 * Library for storing device specific information. It should be used for storing simple
-	 * strings and is not suitable for storing large chunks of data.
-	 *
-	 * @class mw.storage
-	 * @singleton
-	 */
-	mw.storage = {
-
-		localStorage: ( function () {
-			// Catch exceptions to avoid fatal in Chrome's "Block data storage" mode
-			// which throws when accessing the localStorage property itself, as opposed
-			// to the standard behaviour of throwing on getItem/setItem. (T148998)
+	// Catch exceptions to avoid fatal in Chrome's "Block data storage" mode
+	// which throws when accessing the localStorage property itself, as opposed
+	// to the standard behaviour of throwing on getItem/setItem. (T148998)
+	var
+		localStorage = ( function () {
 			try {
 				return window.localStorage;
 			} catch ( e ) {}
 		}() ),
-
-		/**
-		 * Retrieve value from device storage.
-		 *
-		 * @param {string} key Key of item to retrieve
-		 * @return {string|boolean} False when localStorage not available, otherwise string
-		 */
-		get: function ( key ) {
+		sessionStorage = ( function () {
 			try {
-				return mw.storage.localStorage.getItem( key );
+				return window.sessionStorage;
 			} catch ( e ) {}
-			return false;
-		},
+		}() );
 
-		/**
-		  * Set a value in device storage.
-		  *
-		  * @param {string} key Key name to store under
-		  * @param {string} value Value to be stored
-		  * @return {boolean} Whether the save succeeded or not
-		  */
-		set: function ( key, value ) {
-			try {
-				mw.storage.localStorage.setItem( key, value );
-				return true;
-			} catch ( e ) {}
-			return false;
-		},
+	/**
+	 * A wrapper for an HTML5 Storage interface (`localStorage` or `sessionStorage`)
+	 * that is safe to call on all browsers.
+	 *
+	 * @class mw.SafeStorage
+	 * @private
+	 */
 
-		/**
-		  * Remove a value from device storage.
-		  *
-		  * @param {string} key Key of item to remove
-		  * @return {boolean} Whether the save succeeded or not
-		  */
-		remove: function ( key ) {
-			try {
-				mw.storage.localStorage.removeItem( key );
-				return true;
-			} catch ( e ) {}
-			return false;
-		}
+	/**
+	 * @ignore
+	 * @param {Object|undefined} store The Storage instance to wrap around
+	 */
+	function SafeStorage( store ) {
+		this.store = store;
+	}
+
+	/**
+	 * Retrieve value from device storage.
+	 *
+	 * @param {string} key Key of item to retrieve
+	 * @return {string|boolean} False when localStorage not available, otherwise string
+	 */
+	SafeStorage.prototype.get = function ( key ) {
+		try {
+			return this.store.getItem( key );
+		} catch ( e ) {}
+		return false;
+	};
+
+	/**
+	  * Set a value in device storage.
+	  *
+	  * @param {string} key Key name to store under
+	  * @param {string} value Value to be stored
+	  * @return {boolean} Whether the save succeeded or not
+	  */
+	SafeStorage.prototype.set = function ( key, value ) {
+		try {
+			this.store.setItem( key, value );
+			return true;
+		} catch ( e ) {}
+		return false;
+	};
+
+	/**
+	  * Remove a value from device storage.
+	  *
+	  * @param {string} key Key of item to remove
+	  * @return {boolean} Whether the save succeeded or not
+	  */
+	SafeStorage.prototype.remove = function ( key ) {
+		try {
+			this.store.removeItem( key );
+			return true;
+		} catch ( e ) {}
+		return false;
 	};
 
+	/**
+	 * @class
+	 * @singleton
+	 * @extends mw.SafeStorage
+	 */
+	mw.storage = new SafeStorage( localStorage );
+
+	/**
+	 * @class
+	 * @singleton
+	 * @extends mw.SafeStorage
+	 */
+	mw.storage.session = new SafeStorage( sessionStorage );
+
 }( mediaWiki ) );
diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.storage.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.storage.test.js
index 6cef4a7c8184..436cb2ed7555 100644
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.storage.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.storage.test.js
@@ -1,36 +1,56 @@
 ( function ( mw ) {
 	QUnit.module( 'mediawiki.storage' );
 
-	QUnit.test( 'set/get with localStorage', 3, function ( assert ) {
-		this.sandbox.stub( mw.storage, 'localStorage', {
+	QUnit.test( 'set/get with storage support', function ( assert ) {
+		var stub = {
 			setItem: this.sandbox.spy(),
 			getItem: this.sandbox.stub()
-		} );
+		};
+		stub.getItem.withArgs( 'foo' ).returns( 'test' );
+		stub.getItem.returns( null );
+		this.sandbox.stub( mw.storage, 'store', stub );
 
 		mw.storage.set( 'foo', 'test' );
-		assert.ok( mw.storage.localStorage.setItem.calledOnce );
+		assert.ok( stub.setItem.calledOnce );
 
-		mw.storage.localStorage.getItem.withArgs( 'foo' ).returns( 'test' );
-		mw.storage.localStorage.getItem.returns( null );
 		assert.strictEqual( mw.storage.get( 'foo' ), 'test', 'Check value gets stored.' );
 		assert.strictEqual( mw.storage.get( 'bar' ), null, 'Unset values are null.' );
 	} );
 
-	QUnit.test( 'set/get without localStorage', 3, function ( assert ) {
-		this.sandbox.stub( mw.storage, 'localStorage', {
+	QUnit.test( 'set/get with storage methods disabled', function ( assert ) {
+		// This covers browsers where storage is disabled
+		// (quota full, or security/privacy settings).
+		// On most browsers, these interface will be accessible with
+		// their methods throwing.
+		var stub = {
 			getItem: this.sandbox.stub(),
 			removeItem: this.sandbox.stub(),
 			setItem: this.sandbox.stub()
-		} );
+		};
+		stub.getItem.throws();
+		stub.setItem.throws();
+		stub.removeItem.throws();
+		this.sandbox.stub( mw.storage, 'store', stub );
 
-		mw.storage.localStorage.getItem.throws();
 		assert.strictEqual( mw.storage.get( 'foo' ), false );
-
-		mw.storage.localStorage.setItem.throws();
 		assert.strictEqual( mw.storage.set( 'foo', 'test' ), false );
+		assert.strictEqual( mw.storage.remove( 'foo', 'test' ), false );
+	} );
+
+	QUnit.test( 'set/get with storage object disabled', function ( assert ) {
+		// On other browsers, these entire object is disabled.
+		// `'localStorage' in window` would be true (and pass feature test)
+		// but trying to read the object as window.localStorage would throw
+		// an exception. Such case would instantiate SafeStorage with
+		// undefined after the internal try/catch.
+		var old = mw.storage.store;
+		mw.storage.store = undefined;
 
-		mw.storage.localStorage.removeItem.throws();
+		assert.strictEqual( mw.storage.get( 'foo' ), false );
+		assert.strictEqual( mw.storage.set( 'foo', 'test' ), false );
 		assert.strictEqual( mw.storage.remove( 'foo', 'test' ), false );
+
+		mw.storage.store = old;
 	} );
 
 }( mediaWiki ) );