diff --git a/resources/mmv.bootstrap/mmv.Config.js b/resources/mmv.bootstrap/mmv.Config.js index 520b3c1dd..cbbe8d4c0 100644 --- a/resources/mmv.bootstrap/mmv.Config.js +++ b/resources/mmv.bootstrap/mmv.Config.js @@ -35,60 +35,6 @@ class Config { this.viewerConfig = viewerConfig; } - /** - * Get value from local storage or fail gracefully. - * - * @param {string} key - * @param {any} [fallback] value to return when key is not set or localStorage is not supported - * @return {string|null} stored value or fallback or null if neither exists - */ - getFromLocalStorage( key, fallback ) { - const value = mw.storage.get( key ); - - // localStorage will only store strings; if values `null`, `false` or - // `0` are set, they'll come out as `"null"`, `"false"` or `"0"`, so we - // can be certain that an actual null is a failure to locate the item, - // and false is an issue with localStorage itself - if ( value !== null && value !== false ) { - return value; - } - - if ( value === null ) { - mw.log( `Failed to fetch item ${ key } from localStorage` ); - } - - return fallback !== undefined ? fallback : null; - } - - /** - * Set item in local storage or fail gracefully. - * - * @param {string} key - * @param {any} value - * @return {boolean} whether storing the item was successful - */ - setInLocalStorage( key, value ) { - return mw.storage.set( key, value ); - } - - /** - * Remove item from local storage or fail gracefully. - * - * @param {string} key - * @return {boolean} whether storing the item was successful - */ - removeFromLocalStorage( key ) { - mw.storage.remove( key ); - - // mw.storage.remove catches all exceptions and returns false if any - // occur, so we can't distinguish between actual issues, and - // localStorage not being supported - however, localStorage.removeItem - // is not documented to throw any errors, so nothing to worry about; - // when localStorage is not supported, we'll consider removal successful - // (it can't have been there in the first place) - return true; - } - /** * (Semi-)permanently stores the setting whether MediaViewer should handle thumbnail clicks. * - for logged-in users, we use preferences @@ -106,9 +52,9 @@ class Config { if ( !mw.user.isNamed() ) { if ( !enabled ) { - success = this.setInLocalStorage( 'wgMediaViewerOnClick', '0' ); // localStorage stringifies everything, best use strings in the first place + success = mw.storage.set( 'wgMediaViewerOnClick', '0' ); // localStorage stringifies everything, best use strings in the first place } else { - success = this.removeFromLocalStorage( 'wgMediaViewerOnClick' ); + success = mw.storage.remove( 'wgMediaViewerOnClick' ); } if ( success ) { deferred = $.Deferred().resolve(); @@ -146,7 +92,7 @@ class Config { * @return {boolean} */ shouldShowStatusInfo() { - return !isMediaViewerEnabledOnClick( mw.config, mw.user, mw.storage ) && this.getFromLocalStorage( 'mmv-showStatusInfo' ) === '1'; + return !isMediaViewerEnabledOnClick( mw.config, mw.user, mw.storage ) && mw.storage.get( 'mmv-showStatusInfo' ) === '1'; } /** @@ -156,9 +102,9 @@ class Config { * @private */ maybeEnableStatusInfo() { - const currentShowStatusInfo = this.getFromLocalStorage( 'mmv-showStatusInfo' ); + const currentShowStatusInfo = mw.storage.get( 'mmv-showStatusInfo' ); if ( currentShowStatusInfo === null ) { - this.setInLocalStorage( 'mmv-showStatusInfo', '1' ); + mw.storage.set( 'mmv-showStatusInfo', '1' ); } } @@ -166,7 +112,7 @@ class Config { * Called when status info is displayed. Future shouldShowStatusInfo() calls will return false. */ disableStatusInfo() { - this.setInLocalStorage( 'mmv-showStatusInfo', '0' ); + mw.storage.set( 'mmv-showStatusInfo', '0' ); } /** diff --git a/tests/qunit/mmv/mmv.Config.test.js b/tests/qunit/mmv/mmv.Config.test.js index 45dd43836..4b792c2d7 100644 --- a/tests/qunit/mmv/mmv.Config.test.js +++ b/tests/qunit/mmv/mmv.Config.test.js @@ -17,7 +17,7 @@ const { isMediaViewerEnabledOnClick } = require( 'mmv.head' ); const { Config } = require( 'mmv.bootstrap' ); -const { createLocalStorage, getDisabledLocalStorage, getFakeLocalStorage, getUnsupportedLocalStorage } = require( './mmv.testhelpers.js' ); +const { createLocalStorage, getFakeLocalStorage } = require( './mmv.testhelpers.js' ); const config0 = mw.config; const storage = mw.storage; const user = mw.user; @@ -38,68 +38,6 @@ const saveOption = mw.Api.prototype.saveOption; assert.true( config instanceof Config ); } ); - QUnit.test( 'getFromLocalStorage', function ( assert ) { - let config; - - mw.storage = getUnsupportedLocalStorage(); // no browser support - config = new Config( {} ); - assert.strictEqual( config.getFromLocalStorage( 'foo' ), null, 'Returns null when not supported' ); - assert.strictEqual( config.getFromLocalStorage( 'foo', 'bar' ), 'bar', 'Returns fallback when not supported' ); - - mw.storage = getDisabledLocalStorage(); // browser supports it but disabled - config = new Config( {} ); - assert.strictEqual( config.getFromLocalStorage( 'foo' ), null, 'Returns null when disabled' ); - assert.strictEqual( config.getFromLocalStorage( 'foo', 'bar' ), 'bar', 'Returns fallback when disabled' ); - - mw.storage = createLocalStorage( { getItem: this.sandbox.stub() } ); - config = new Config( {} ); - - mw.storage.store.getItem.withArgs( 'foo' ).returns( null ); - assert.strictEqual( config.getFromLocalStorage( 'foo' ), null, 'Returns null when key not set' ); - assert.strictEqual( config.getFromLocalStorage( 'foo', 'bar' ), 'bar', 'Returns fallback when key not set' ); - - mw.storage.store.getItem.reset(); - mw.storage.store.getItem.withArgs( 'foo' ).returns( 'boom' ); - assert.strictEqual( config.getFromLocalStorage( 'foo' ), 'boom', 'Returns correct value' ); - assert.strictEqual( config.getFromLocalStorage( 'foo', 'bar' ), 'boom', 'Returns correct value ignoring fallback' ); - } ); - - QUnit.test( 'setInLocalStorage', function ( assert ) { - let config; - - mw.storage = getUnsupportedLocalStorage(); // no browser support - config = new Config( {} ); - assert.strictEqual( config.setInLocalStorage( 'foo', 'bar' ), false, 'Returns false when not supported' ); - - mw.storage = getDisabledLocalStorage(); // browser supports it but disabled - config = new Config( {} ); - assert.strictEqual( config.setInLocalStorage( 'foo', 'bar' ), false, 'Returns false when disabled' ); - - mw.storage = createLocalStorage( { setItem: this.sandbox.stub(), removeItem: this.sandbox.stub() } ); - config = new Config( {} ); - - assert.strictEqual( config.setInLocalStorage( 'foo', 'bar' ), true, 'Returns true when works' ); - - mw.storage.store.setItem.throwsException( 'localStorage full!' ); - assert.strictEqual( config.setInLocalStorage( 'foo', 'bar' ), false, 'Returns false on error' ); - } ); - - QUnit.test( 'Localstorage remove', function ( assert ) { - let config; - - mw.storage = getUnsupportedLocalStorage(); // no browser support - config = new Config( {} ); - assert.strictEqual( config.removeFromLocalStorage( 'foo' ), true, 'Returns true when not supported' ); - - mw.storage = getDisabledLocalStorage(); // browser supports it but disabled - config = new Config( {} ); - assert.strictEqual( config.removeFromLocalStorage( 'foo' ), true, 'Returns true when disabled' ); - - mw.storage = createLocalStorage( { removeItem: this.sandbox.stub() } ); - config = new Config( {} ); - assert.strictEqual( config.removeFromLocalStorage( 'foo' ), true, 'Returns true when works' ); - } ); - QUnit.test( 'isMediaViewerEnabledOnClick', function ( assert ) { mw.storage = createLocalStorage( { getItem: this.sandbox.stub() } ); mw.config = { get: this.sandbox.stub() };