jenkins-bot has submitted this change and it was merged.
Change subject: mw.storage: Fix broken test (incompatible with Chrome 45)
......................................................................
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
(cherry picked from commit 430a0d3984fe188531175a295a24906d472abe42)
---
M resources/src/mediawiki/mediawiki.storage.js
M tests/qunit/suites/resources/mediawiki/mediawiki.storage.test.js
2 files changed, 51 insertions(+), 79 deletions(-)
Approvals:
Ori.livneh: Looks good to me, approved
Jforrester: Looks good to me, approved
jenkins-bot: Verified
diff --git a/resources/src/mediawiki/mediawiki.storage.js
b/resources/src/mediawiki/mediawiki.storage.js
index e10b561..3958392 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 c25641d..6cef4a7 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', 4, function ( assert ) {
+ QUnit.test( 'set/get with localStorage', 3, function ( assert ) {
+ this.sandbox.stub( mw.storage, 'localStorage', {
+ setItem: this.sandbox.spy(),
+ getItem: this.sandbox.stub()
+ } );
+
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.' );
+ this.sandbox.stub( mw.storage, 'localStorage', {
+ getItem: this.sandbox.stub(),
+ removeItem: this.sandbox.stub(),
+ setItem: this.sandbox.stub()
+ } );
- assert.strictEqual( mw.storage.remove( 'foo', 'test' ), false,
- 'When localStorage not available remove fails.' );
+ mw.storage.localStorage.getItem.throws();
+ assert.strictEqual( mw.storage.get( 'foo' ), false );
- assert.strictEqual( mw.storage.get( 'foo' ), false,
- 'Inability to retrieve values return false to
differentiate from null (not set).' );
- } );
+ mw.storage.localStorage.setItem.throws();
+ assert.strictEqual( mw.storage.set( 'foo', 'test' ), 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 );
- }
- } );
-
- 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 ) );
--
To view, visit https://gerrit.wikimedia.org/r/242416
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iecbdeb050b9a69febd3388898d98293a84588a94
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: REL1_26
Gerrit-Owner: Jforrester <[email protected]>
Gerrit-Reviewer: Edokter <[email protected]>
Gerrit-Reviewer: Jack Phoenix <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: Ori.livneh <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
