mw.storage: Fix broken test (incompatible with Chrome 45)

The localStorage global is read-only which Chrome 45 enforces.

Simplify things by dropping mediaWiki.storage.isStorageEnabled, and
by wrapping each operation with a try / catch.

Bug: T113413
Change-Id: Iecbdeb050b9a69febd3388898d98293a84588a94

diff --git a/resources/src/mediawiki/mediawiki.storage.js b/resources/src/mediawiki/mediawiki.storage.js
index e10b561..3958392 100644 (file)
--- a/resources/src/mediawiki/mediawiki.storage.js
+++ b/resources/src/mediawiki/mediawiki.storage.js
@@ -1,69 +1,58 @@
 ( function ( mw ) {
        'use strict';
-       var storage;
 
        /**
         * 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
         */
-       storage = {
-               isLocalStorageSupported: false,
+       mw.storage = {
+
+               localStorage: window.localStorage,
+
                /**
                 * Retrieve value from device storage.
                 *
-                * @param {String} key of item to retrieve
-                * @returns {String|Boolean} false when localStorage not available, otherwise string
+                * @param {string} key Key of item to retrieve
+                * @return {string|boolean} False when localStorage not available, otherwise string
                 */
                get: function ( key ) {
-                       if ( this.isLocalStorageSupported ) {
-                               return localStorage.getItem( key );
-                       } else {
-                               return false;
-                       }
+                       try {
+                               return mw.storage.localStorage.getItem( key );
+                       } catch ( e ) {}
+                       return false;
                },
 
                /**
-                * Set a value in device storage.
-                *
-                * @param {String} key key name to store under.
-                * @param {String} value to be stored.
-                * @returns {Boolean} whether the save succeeded or not.
-                */
+                 * Set a value in device storage.
+                 *
+                 * @param {string} key Key name to store under
+                 * @param {string} value Value to be stored
+                 * @returns {boolean} Whether the save succeeded or not
+                 */
                set: function ( key, value ) {
                        try {
-                               localStorage.setItem( key, value );
+                               mw.storage.localStorage.setItem( key, value );
                                return true;
-                       } catch ( e ) {
-                               return false;
-                       }
+                       } catch ( e ) {}
+                       return false;
                },
 
                /**
-                * Remove a value from device storage.
-                *
-                * @param {String} key of item to remove.
-                * @returns {Boolean} whether the save succeeded or not.
-                */
+                 * Remove a value from device storage.
+                 *
+                 * @param {string} key Key of item to remove
+                 * @returns {boolean} Whether the save succeeded or not
+                 */
                remove: function ( key ) {
-                       if ( this.isLocalStorageSupported ) {
-                               localStorage.removeItem( key );
+                       try {
+                               mw.storage.localStorage.removeItem( key );
                                return true;
-                       } else {
-                               return false;
-                       }
+                       } catch ( e ) {}
+                       return false;
                }
        };
 
-       mw.storage = storage;
-       // See if local storage is supported
-       try {
-               localStorage.setItem( 'localStorageTest', 'localStorageTest' );
-               localStorage.removeItem( 'localStorageTest' );
-               storage.isLocalStorageSupported = true;
-       } catch ( e ) {
-               // Already set. No body needed.
-       }
-
 }( mediaWiki ) );

diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.storage.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.storage.test.js
index c25641d..6cef4a7 100644 (file)
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.storage.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.storage.test.js
@@ -1,53 +1,36 @@
 ( function ( mw ) {
-       QUnit.module( 'mediawiki.storage: normal case.', {
-               setup: function () {
-                       this.sandbox.stub( mw.storage, 'isLocalStorageSupported', true );
-                       this.spy = this.sandbox.spy( localStorage, 'setItem' );
-                       this.sandbox.stub( localStorage, 'getItem' )
-                               .withArgs( 'foo' ).returns( 'test' )
-                               .withArgs( 'bar' ).returns( null );
-               }
-       } );
+       QUnit.module( 'mediawiki.storage' );
+
+       QUnit.test( 'set/get with localStorage', 3, function ( assert ) {
+               this.sandbox.stub( mw.storage, 'localStorage', {
+                       setItem: this.sandbox.spy(),
+                       getItem: this.sandbox.stub()
+               } );
 
-       QUnit.test( 'set/get with localStorage', 4, function ( assert ) {
                mw.storage.set( 'foo', 'test' );
-               assert.strictEqual( this.spy.calledOnce, true, 'Check localStorage called.' );
-               assert.strictEqual( this.spy.calledWith( 'foo', 'test' ), true,
-                       'Check prefixed.' );
+               assert.ok( mw.storage.localStorage.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.module( 'mediawiki.storage: localStorage does not exist', {
-               setup: function () {
-                       this.sandbox.stub( mw.storage, 'isLocalStorageSupported', false );
-                       this.sandbox.stub( localStorage, 'setItem' ).throws();
-               }
-       } );
-
        QUnit.test( 'set/get without localStorage', 3, function ( assert ) {
-               assert.strictEqual( mw.storage.set( 'foo', 'test' ), false,
-                       'When localStorage not available save fails.' );
-
-               assert.strictEqual( mw.storage.remove( 'foo', 'test' ), false,
-                       'When localStorage not available remove fails.' );
+               this.sandbox.stub( mw.storage, 'localStorage', {
+                       getItem: this.sandbox.stub(),
+                       removeItem: this.sandbox.stub(),
+                       setItem: this.sandbox.stub()
+               } );
 
-               assert.strictEqual( mw.storage.get( 'foo' ), false,
-                               'Inability to retrieve values return false to differentiate from null (not set).' );
-       } );
+               mw.storage.localStorage.getItem.throws();
+               assert.strictEqual( mw.storage.get( 'foo' ), false );
 
-       QUnit.module( 'mediawiki.storage: localStorage exhausted', {
-               setup: function () {
-                       this.sandbox.stub( mw.storage, 'isLocalStorageSupported', true );
-                       this.sandbox.stub( localStorage, 'setItem' ).throws();
-                       this.sandbox.stub( localStorage, 'getItem' ).returns( null );
-               }
-       } );
+               mw.storage.localStorage.setItem.throws();
+               assert.strictEqual( mw.storage.set( 'foo', 'test' ), false );
 
-       QUnit.test( 'set/get without localStorage', 2, function ( assert ) {
-               assert.strictEqual( mw.storage.set( 'foo', 'test' ), false,
-                       'When localStorage not available inform user with false.' );
-               assert.strictEqual( mw.storage.get( 'foo' ), null, 'No value registered.' );
+               mw.storage.localStorage.removeItem.throws();
+               assert.strictEqual( mw.storage.remove( 'foo', 'test' ), false );
        } );
 
 }( mediaWiki ) );