play-to-xbmc-chrome
play-to-xbmc-chrome copied to clipboard
Addon settings saved using Window.localStorage instead of storage.local
The storage.local
API has some advantages over the Window.localStorage
:
Overview
This API has been optimized to meet the specific storage needs of extensions. It provides the same storage capabilities as the localStorage API with the following key differences:
- User data can be automatically synced with Chrome sync (using storage.sync).
- Your extension's content scripts can directly access user data without the need for a background page.
- A user's extension settings can be persisted even when using split incognito behavior.
- It's asynchronous with bulk read and write operations, and therefore faster than the blocking and serial localStorage API.
- User data can be stored as objects (the localStorage API stores data in strings).
- Enterprise policies configured by the administrator for the extension can be read (using storage.managed with a schema).
(https://developer.chrome.com/extensions/storage)
For Firefox the localStorage usage causes some issues (lost settings after upgrade):
Although this API is similar to Window.localStorage it is recommended that you don't use Window.localStorage in extension code. Firefox will clear data stored by extensions using the localStorage API in various scenarios where users clear their browsing history and data for privacy reasons, while data saved using the storage.local API will be correctly persisted in these scenarios. (https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/storage/local)
Would you be interested in changing the API used? Or maybe you already have some code ready? I would be working on changing it for Firefox so the settings are persistent during upgrades.
One important thins is that the storage.local
is asynchronous, so the change to this API won't be just a simple rename of the function calls.
Based on the information provided it does seem like the better option in this case, I wasn't sure if I implemented Window.localStorage because it was the only option at the time or I simply wasn't aware of storage.local. In any case, I am interested in switching over, but I don't have any code ready, so the earliest I would be working on this is probably the coming weekend. A key point in my implementation is to ensure the settings of Window.localStorage is migrated over, not sure if you've taken that into consideration.
How would you prefer this done? Do you want me to work on it and you merge it over to your fork, or do you want to work on in separately and send me a pull request? I do not mind it either way.
If you haven't worked on it already, then I'll code it and send a pull request. But I won't be able to work on the migration from the old settings. The settings are already gone during the upgrade (on Firefox), so I won't be able to test it.
Would it be OK?
@khloke I created pull request #129. I made quite a lot of changes there related to this issue
Short summary of the changes:
- changed all the places where the settings were used (asynchronous approach of the new API required more complicated changes)
- implemented code which should migrate all settings from localStorage to browser.storage.
- function doing upgrades used 4 digit number of some old version, now versions are 3 digit so I divide the versions > 999 by 10, to be able to compare properly
- added some more debugging messages
- settings_migration_test.js - file added to test the migration, if you want to test the migration include it in the manifest.json in the background array (it should not be added in the official addon)
If you find something is not working, require additional changes or have questions to the implementation let me know.