From 430a0d3984fe188531175a295a24906d472abe42 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Tue, 22 Sep 2015 21:44:52 +0100 Subject: [PATCH] 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 --- resources/src/mediawiki/mediawiki.storage.js | 69 ++++++++----------- .../mediawiki/mediawiki.storage.test.js | 61 ++++++---------- 2 files changed, 51 insertions(+), 79 deletions(-) diff --git a/resources/src/mediawiki/mediawiki.storage.js b/resources/src/mediawiki/mediawiki.storage.js index e10b561fb8..39583926bb 100644 --- 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 c25641db48..6cef4a7c81 100644 --- 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 ) ); -- 2.20.1