Page MenuHomePhabricator

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

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.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 24 2016, 7:01 PM
Krinkle claimed this task.Oct 24 2016, 10:53 PM
Krinkle triaged this task as Normal priority.
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

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

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

Krinkle closed this task as Resolved.Oct 26 2016, 3:04 PM
Petar.petkovic reopened this task as Open.May 25 2018, 11:18 PM
Petar.petkovic added a subscriber: Petar.petkovic.

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.

Krinkle added a comment.EditedMay 26 2018, 10:14 AM

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.