Use mw.storage.get/set/remove directly

Bug: T77349
Change-Id: I5b670ed617c8359f28029cf117cfda26b635e871
This commit is contained in:
Simon Legner 2024-08-11 09:56:43 +02:00
parent 8c4186962e
commit d698b8b006
2 changed files with 7 additions and 123 deletions

View file

@ -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' );
}
/**

View file

@ -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() };