[Safari] Settings erased on web history clear, fix inside!

[ghost]

The version info doesn't really matter, but I'll include it anyway: OS X 10.10.3, Safari 8.0.5, uBlock 0.9.4.0. But this is the behavior in ALL old versions of Safari too, so version doesn't matter.

The issue: uBlock is saving its extension settings in HTML5 LocalStorage. In info.plist you've allocated a local database of 10 mb. That is a common mistake among extension coders coming from other browsers (Firefox, Chrome).

When you click on Safari - Clear History and Website Data, "all history" and restart the browser, uBlock will be back to default settings. It loses all custom filters, dynamic filters, options, etc. Literally everything is lost. This is because "Clear history and website data" in Safari clears HTML5 LocalStorage (and databases).

Why does it clear it? Because "LocalStorage" is intended for websites, not extensions. Extensions can write/read it, but it's not intended as any permanent storage. LocalStorage is only supposed to be used for temporary/cache data, such as loaded filter-lists. But for actual settings, Apple has a different and more robust method for storing permanent Extension settings that persist through cache clears: https://developer.apple.com/library/safari/documentation/Tools/Conceptual/SafariExtensionGuide/ExtensionSettings/ExtensionSettings.html

The solution:
1. Add every "permanent" setting to Settings.plist, and write/read to that instead of using LocalStorage. Permanent settings would include important things like checkbox-states, enabled/added 3rd party lists, custom filters and dynamic filters.
2. You write/read permanent settings via "safari.extension.settings.keyname" as if it was just a regular variable.
3. You can still use LocalStorage for filterlist-caches and stuff like that. Heavy data belongs in LocalStorage, which is its purpose (a nice, dirty cache for big, temporary data).
4. That's it.

The solution (alternative):
1. Add a "Hidden" setting in Extension Builder, and call it something like "ublock_settings_backup".
2. Store everything in LocalStorage as you're doing right now.
3. Periodically (or when the settings change, if you can find a single point that the code always calls to save settings), serialize all LocalStorage data as a json object and store it in the hidden setting. I haven't tried this method myself. It should work but will obviously be slow if there's lots of data to serialize, and there might be a size limit to the variable value...
4. Have a flag called "firstrun" or something, which gets set to false after startup-initialization is complete. During startup, check if firstrun is true, which means the extension was either just installed, or the user has cleared their Safari cache and lost the LocalStorage. To check if backed up settings exist, read the "ublock_settings_backup" value and unserialize it, and write it to LocalStorage. Presto!

I would suggest method #1 since it's the proper way. Hopefully uBlock has a modular "save/load settings" function and doesn't have hardcoded calls to localstorage all over the place. If there's a modular location, you can simply hook the function for Safari and make it use Extension Settings instead of local storage.

[chrisaljoudi]

@ublockuser Hi!

Thanks for the detailed report and your ideas. I understand the issue, and thought I'd point out a few things:

• It's not LocalStorage being used, but rather IndexedDB/Web SQL (client-side database). What you said remains true.
• The quota in Info.plist applies only to client-side databases, neither to LocalStorage nor to safari.extension.settings values. (As a note: the quota in this case is 100MB, not 10MB, to be able to fit the filter list caches).

You're absolutely right, though: we should store the preferences under the extension's settings, not in the database that has the caches and stuff.

The problem is, uBlock was written for Chrome first and it was written to use the same storage for both the caches and the settings. So, the fix is a matter of changing that.

I'll get to work on this in any case — just thought I'd share some things in case anyone's curious.

[ghost]

@chrisaljoudi Thanks for the extra information.

Given the bad situation for Safari users, as well as future-proofing this for other browsers, I'd probably create a platform-agnostic "read/write setting" interface as well as a "query cache" interface, and then changing all old, hardcoded references to using the wrappers instead.

Something like:
uBlock.Settings.set('keyname', newValue); //write
uBlock.Settings.get('keyname'); //read
uBlock.Cache.query(...);

When the code refers to such functions everywhere, it's "only" a matter of making the individual platforms set up the underlying Settings/Cache objects appropriate for the browser. Most would use Database/Localstorage for both; Safari would use Extension Settings for Settings and Database for Cache.

I hope that there aren't a lot of scattered read/writes to complicate such a rewrite. Although a grep would find them all, I suppose...

I also hope it'll be possible to do platform-specific overwriting of the handler before uBlock's core tries to use it.

And the last complicating issue I can think of is when the values are arrays... I suppose the set/get handlers would have to check the datatype of the incoming value and serialize it if the underlying storage needs it...

Sounds like using a database for storage made things very easy for things like multi-value settings, so I wish you lots of luck with this rewrite.

[chrisaljoudi]

Alright; 288ea7b fixes this. I spent the last few days making sure everything still works, but further testing is quite desirable.

[ghost]

@chrisaljoudi I have read through the commit and am impressed at the amount of work it has required and that you accomplished it! Seems like the original code was a bit spaghetti as far as preference handling, with many different names for the preferences objects (µb.assets, vAPI.storage, µb.keyvalSetOne and µBlock.keyvalSetOne). Wow...

I see that you've modified the browser-specific vapi-background.js for Safari so that its "set/get setting" interfaces with Safari's extension settings object. Awesome.

I want to test this for you and do many browser cache clears while toggling checkboxes on/off and trying dynamic filters and custom filters, to ensure that every setting is now preserved properly, but I don't know how to run unsigned/unpackaged extensions so I can't test the latest changes. Do you know if there's a way other than me installing Xcode and building the extension myself?