mw.storage not working when browser blocks setting of data
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	WMDE-Fisch
	Oct 24 2016, 7:01 PM

Description

You can reproduce this by setting Block sites from setting any data in the Chrome/Chromium Content Settings.

Visiting en.wikipedia.org you will get:

load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=0f3xo9s:176 SecurityError: Failed to read the 'localStorage' property from 'Window': Access is denied for this document. DOMException: Failed to read the 'localStorage' property from 'Window': Access is denied for this document.
    at Error (native)
    at Object.init (https://en.wikipedia.org/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=0f3xo9s:171:803)
    at Object.work (https://en.wikipedia.org/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=0f3xo9s:166:656)
    at enqueue (https://en.wikipedia.org/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=0f3xo9s:163:476)
    at Object.using (https://en.wikipedia.org/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=0f3xo9s:169:277)
    at runScript (https://en.wikipedia.org/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=0f3xo9s:161:350)
    at checkCssHandles (https://en.wikipedia.org/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=0f3xo9s:162:124)
    at execute (https://en.wikipedia.org/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=0f3xo9s:162:826)
    at Object.implement (https://en.wikipedia.org/w/load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=0f3xo9s:168:843)
    at https://en.wikipedia.org/wiki/Main_Page:7:2796logError @ load.php?debug=false&lang=en&modules=jquery%2Cmediawiki&only=scripts&skin=vector&version=0f3xo9s:176.

Also login wont work getting you an similar error and the message

There seems to be a problem with your login session; this action has been canceled as a precaution against session hijacking. Go back to the previous page, reload that page and then try again.

Looking at the implementation I assume that setting localStorage: window.localStorage, as a field might be a problem if window.localStorage does not exist. Are there any plans to improve mw.storage? E.g. by using jquery.jStorage, that seems to give a much more robust implementation for storing local data.

Details

Related Changes in Gerrit:

	Subject	Repo	Branch	Lines +/-
	mediawiki.storage: Catch exceptions on window.localStorage property access	mediawiki/core	master	+8 -1

Customize query in gerrit

Related Objects

Mentioned In: T195647: Consider hiding (handled) warning about localStorage in mw.loader
Mentioned Here: T195647: Consider hiding (handled) warning about localStorage in mw.loader

Event Timeline

WMDE-Fisch created this task.Oct 24 2016, 7:01 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 24 2016, 7:01 PM

Paladox added a project: MediaWiki-ResourceLoader.Oct 24 2016, 8:56 PM

• Krinkle claimed this task.Oct 24 2016, 10:53 PM

• Krinkle triaged this task as Medium priority.

• Krinkle edited projects, added MediaWiki-General, Performance-Team; removed MediaWiki-ResourceLoader.

• Krinkle added a project: JavaScript.

ResourceLoader avoids this exception by using an in key lookup in startup.js for conditionals ('localStorage' in window). And for other access, always wrapped in try/catch.

The mw.storage module does the same thing, except when it obtains a reference to the localStorage object itself.

	mw.storage = {
		localStorage: window.localStorage,

		/**-*/
		get: function ( key ) {
			try {
				return mw.storage.localStorage.getItem( key );
			} catch ( e ) {}
			return false;
		},

Change 317712 had a related patch set uploaded (by Krinkle):
mediawiki.storage: Catch exceptions on window.localStorage property access

https://gerrit.wikimedia.org/r/317712

gerritbot added a project: Patch-For-Review.Oct 24 2016, 11:03 PM

Change 317712 merged by Gilles:
mediawiki.storage: Catch exceptions on window.localStorage property access

https://gerrit.wikimedia.org/r/317712

ReleaseTaggerBot added projects: MW-1.28-release-notes, MW-1.29-release (WMF-deploy-2016-11-01_(1.29.0-wmf.1)).Oct 25 2016, 7:00 PM

• Krinkle closed this task as Resolved.Oct 26 2016, 3:04 PM

• Petar.petkovic removed a project: Patch-For-Review.May 25 2018, 10:33 PM

This error is still happening. Block cookies in Chrome and you have this error. If you have whitelisted the wikis you regularly visit, then visiting a page on non-whitelisted wiki shows this error in console (it's easy to reproduce, so I don't put the entire log):
Failed to read the 'localStorage' property from 'Window': Access is denied for this document.

When disabling cookies in Chrome and viewing a page, I do not get any uncaught exception or error in the console from mw.storage.

However, I do see the following:

Exception in store-localstorage-init:
> DOMException: Failed to read the 'localStorage' property from 'Window': Access is denied for this document.

However, this is not an error and not from mw.storage. It is about use of localStorage by mw.loader, and is logged as a warning (not an error) because it was caught and handled accordingly.

Perhaps we should supress the display in the console of this particular warning, but in terms of behaviour and functionality, everything is working as intended, and no fix would change how it behaves.

The proper fix for exceptions from localStorage is a try/catch. The code in question from mw.loader, looks as follows:

mw.loader.store

try {
	raw = localStorage.getItem( mw.loader.store.getStoreKey() );
	// ..
) {
	mw.track( 'resourceloader.exception', { exception: e, source: 'store-localstorage-init' } );
}

So we are already catching it, but then intentionally logging it for debug purposes.

I'm re-closing this task because it was about mw.storage, not about mw.loader. I've filed T195647 for the mw.loader warning.

• Krinkle closed this task as Resolved.May 26 2018, 10:14 AM

• Krinkle mentioned this in T195647: Consider hiding (handled) warning about localStorage in mw.loader.

mw.storage not working when browser blocks setting of dataClosed, ResolvedPublicActions

Description

Details

Related Objects

Event Timeline

mw.storage not working when browser blocks setting of data
Closed, ResolvedPublic
Actions