#24 Popup is broken when `dom.storage.enabled == false`, making popup unusable
Opened 2 years ago by swissknife. Modified 2 years ago


Browser: Firefox 97.0.1 (64-bit)
OS: macOS 12.2.1 21D62 x86_64
Addon: JShelter 0.7
Website: all websites

How to repro:

1 - Open firefox, go to about:config, toggle dom.storage.enabled to `false
2 - Go to a random webpage
3 - Click on the addon button in the bar

Expected results:

popup appears with buttons allowing to control what restriction level to have on the site, whether to enable network boundary, whether to enable fingerprinting prevention.

Actual results:

Only the hermit crab icon, the words jshelter, the button for "global settings" and a horizontal line are visible. See attached screenshot

Time started:

Say about a week ago

Additional info

I navigated to about:devtools-toolbox?id=jsr%40javascriptrestrictor&type=extension and turned on breakpoints->pause on exceptions in the debugger. Going through the repro steps shows me that an exception is thrown at popup.js:309: widget.checked = localStorage.getItem(key) === 'true';


I currently have the choice between reenabling storage and disabling the addon.

Maintainer note based on the discussion below: The extensions works fine, it is only the popup that is broken. Everything can be configured in global options anyway. Only the number of calls of each JSS wrapping groups is not listed in the UI.


I'm not sure if this traceback is related or not, here it is anyways.
04:28:54.017 TypeError: tab is undefined contentScriptLevelSetter moz-extension://3965d656-9a07-4ae7-89b9-7720035074cc/level_cache.js:94 notifyListeners moz-extension://3965d656-9a07-4ae7-89b9-7720035074cc/nscl/common/SyncMessage.js:169 <anonymous> moz-extension://3965d656-9a07-4ae7-89b9-7720035074cc/nscl/common/SyncMessage.js:40 processing message {"message":"get wrapping for URL","url":"about:blank"} from Object { contextId: 549756131986, id: "jsr@javascriptrestrictor", envType: "content_child", url: "about:blank" } SyncMessage.js:172:21 notifyListeners moz-extension://3965d656-9a07-4ae7-89b9-7720035074cc/nscl/common/SyncMessage.js:172 <anonymous> moz-extension://3965d656-9a07-4ae7-89b9-7720035074cc/nscl/common/SyncMessage.js:40


No, that is not a good idea as that can potentially break websites that use DOM (local) storage to store datthat they consider of more persistent nature and less suitable for cookies.

I guess that turning off DOM storage prevents the extension to save and load configuration. Sure, there is an option to load an empty configuration if nothing is loaded by updateLevels() https://pagure.io/JShelter/webextension/blob/main/f/common/levels.js#_720 like a modification of checkAndSaveConfig() (https://pagure.io/JShelter/webextension/blob/main/f/common/update.js#_359).

BUT, if we do this, there might be many errors caused by the broken writes to the sessions storage. I currently have no idea about their nature as the design expects that the configuration is always there. A small fix like above would raise issues like a change of JSS level does not take effect, FPD cannot be turned on, NBS cannot be turned off, global settings are not stored, the global configuration pages and pop up are broken.

As I realised that updateLevels() handles empty configuration quite well, I tried using the add on with dom.storage.enabled = false.

@swissknife If you open global settings and go to advanced settings, is your configuration like:

    "fpDetectionOn": false,
    "version": 6.1,
    "fpdWhitelist": {},
    "custom_levels": {},
    "domains": {},
    "requestShieldOn": true,
    "__default__": "2",
    "whitelistedHosts": {}

(The order of the JSON keys is not important.)

If you open the extension console like you did above, type levels, do you see:

Object { 0: {…}, 2: {…}, 3: {…}, Experiment: {…} }
​0: Object { builtin: true, level_id: "0", level_text: "Turn JavaScript Shield off", … }
​2: Object { builtin: true, level_id: "2", level_text: "Recommended", … }
​3: Object { builtin: true, level_id: "3", level_text: "Strict", … }
​Experiment: Object { builtin: true, level_id: "Experiment", level_text: "Experimental", … }
​<prototype>: Object { … }

If you see the levels, the extension works as expected, you are not able to change anything because you configured the browser not to store configuration.

Metadata Update from @polcak:
- Issue tagged with: question

2 years ago

Here is what I know:
- when I go to jshelter advanced settings, all my settings are there, so they are loaded correctly whether dom.storage.enabled is true or not.
- when I go to the extension console and type levels, all the levels are there. I can even add and edit a custom level from the global settings page.
- I can also add and remove items from network boundary shield list on the global settings page, and if I disable and reenable the addon, the new entry is still there, even with dom.storage.enabled set to false
- I have not touched either of keepStorageOnUninstall nor keepUuidOnUninstall (source: https://support.mozilla.org/en-US/questions/1270250), they have default values, so the extension's data should theoretically be reset when uninstalling and reinstalling the addon, and yet...
- my settings persist even if I uninstall and reinstall the addon

All this tells me that the extension can save and load data whether dom.storage.enabled is true or not, which suggests in turn that the extension isn't storing anything in the dom. Where is the data stored then? Beats me.

The configs seem to be working fine, this issue is really limited to the popup loading correctly as far as I can tell (and potentially to the data not being cleared on uninstall)

Thanks for the input, so that the extension storage is not affected by the settings. I will correct the title of the issue as the extension works perfectly fine, you can use the extension as is. There are only two downsides for you:

  • You can not use pop up for configuration, everything needs to be configured through global options,
  • you cannot see the number of calls in the popup.

@gioma1 I haven't investigated in the code but

popup.js:309: widget.checked = localStorage.getItem(key) === 'true';

Is there any reason why we use localStorage?

Metadata Update from @polcak:
- Issue assigned to gioma1

2 years ago

you can use the extension as is

Yes, at the price of opening 2 tabs and switching between 3 tabs every time I hit a problematic website. This is doable but kills the ease of use, so rather than doing this, I can only realistically choose between disabling the extension entirely and re-enabling dom.storage. That being said, this issue's title is definitely more accurate after your correction.

Thanks for creating and maintaining this extension! Also hi to @gioma1, I really like your noscript addon and I was a user for several years. Seeing you involved with JShelter is quite a nice surprise :-)

Login to comment on this ticket.