Page MenuHomePhabricator

MultimediaViewer should use mediawiki.storage
Closed, ResolvedPublic

Description

Currently it interacts directly with localStorage.
Given localStorage is shared by many other extensions this has potential to be troublesome and should be managed in the standard way.

Event Timeline

IIRC we avoided mediawiki.storage because it falls back to cookies and we did not want to do that. I don't remember why not, though.

It doesn't use cookies. It's just a dump localStorage wrapper but a standard interface various extensions are using (takes care of prefixing etc)

https://github.com/wikimedia/mediawiki/blob/master/resources/src/mediawiki/mediawiki.storage.js

I was confusing it with jStorage. I guess mediawiki.storage didn't exist yet when MV was written.

There are two problems that should be fixed with mediawiki.storage first, though: accessing window.localStorage can throw an exception, and there is no way to differentiate between successfully retrieving false and failing to retrieve the value.

jhobs triaged this task as Medium priority.Jun 15 2016, 3:16 PM
jhobs moved this task from Incoming to Triaged but Future on the Readers-Web-Backlog board.
jhobs added a project: Technical-Debt.

There are two problems that should be fixed with mediawiki.storage first, though: accessing window.localStorage can throw an exception, and there is no way to differentiate between successfully retrieving false and failing to retrieve the value.

This is not a problem. It is designed so that you can only store strings (which via JSON.stringify can allow you store more complex data structures). In which case you can return '0' or 'false' to represent this value.

Change 356805 had a related patch set uploaded (by Matthias Mullie; owner: Matthias Mullie):
[mediawiki/extensions/MultimediaViewer@master] Replace all direct localStorage interaction by mw.storage

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

Change 356805 merged by jenkins-bot:
[mediawiki/extensions/MultimediaViewer@master] Replace all direct localStorage interaction by mw.storage

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