Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remember all settings #2

Closed
Gitoffthelawn opened this issue May 20, 2017 · 13 comments
Closed

Remember all settings #2

Gitoffthelawn opened this issue May 20, 2017 · 13 comments

Comments

@Gitoffthelawn
Copy link

Hi Andy, can you improve it to remember all settings, even those labelled as "dangerous zone"?

@Thorin-Oakenpants
Copy link

Definitely needs to remember user's last choices (for all checkboxes)

@andy-portmen
Copy link
Owner

done in 0.1.3

@Gitoffthelawn
Copy link
Author

Gitoffthelawn commented Jul 25, 2017

Thanks Andy!

(BTW, I think the release notes use the word "not" instead of "now", giving them the opposite meaning.)

@andy-portmen
Copy link
Owner

Right! it is fixed. Thanks

@Thorin-Oakenpants
Copy link

Thorin-Oakenpants commented Jul 25, 2017

FF54.0.1 and eCleaner 0.1.3 (Windows 64bit but that shouldn't matter)

  • settings are not remembered
  • "done" notification still does not happen - it just sits for hours on Please wait...
  • These may be linked (?why?). Where are selections stored?

PS: hiding chrome/opera only check options and reduced width and fixed scrollbars = all perfect, thanks

Edit: I will test with a new nilla profile. My current one has a lot of tweaks, but nothing I can think of that would cause this - i.e the code never gets to the end (not that I've looked but I assume error handling is well, umm, handled).

@andy-portmen
Copy link
Owner

This extension uses localStorage for its preferences. Do you happen to have an add-on that blocks localStorage access?

@Thorin-Oakenpants
Copy link

I haven't forgotten about this - just time constraints :) I do not block local storage exactly - I block all cookies by default and only allow a few as first party only, and a few more as session only. Cookie Controller (but I will be moving to Cookie Auto-Delete = WebExt) controls dom storage as well. Not sure if that's it entirely (I can test when get time to see what happens with CC disabled, and with various FF cookie settings) .Or did you mean it's using indexedDB?

I also have these two set as false (man, there are so many storage areas these days)

/* 
* [1] https://developer.mozilla.org/en-US/docs/Web/API/StorageManager
* [2] https://developer.mozilla.org/en-US/docs/Web/API/Storage_AP ***/
user_pref("dom.storageManager.enabled", false); // (FF51+)
user_pref("browser.storageManager.enabled", false); // (FF53+)

Did you mean local dom storage, storage API/storage manage API, indexedDB ?

@Thorin-Oakenpants
Copy link

@Gitoffthelawn : what config for storage do you have that makes it work?

@Thorin-Oakenpants
Copy link

Thorin-Oakenpants commented Jul 29, 2017

OK .. the culprit is Firefox must allow cookies - with all cookies blocked by default, dom storage access is denied, which is why the success message never shows and why the settings never stick.

@andy-portmen
Copy link
Owner

I guess you need to use an extension to block cookies instead of using about:config

@Thorin-Oakenpants
Copy link

Thorin-Oakenpants commented Jul 29, 2017

That's what I'm doing. Default deny all and whitelist using Cookie Controller. I will be doing the same with Cookie Auto-Delete.

IMO, (I am not sure about Chrome), in Firefox you could use a preference instead. Otherwise this extension breaks (silently) for all users who have cookies disabled (and use site exceptions). That's a LOT of users. For FF, instead of writing to dom, write the value to "extensions.ecleaner.settings" - that way it won't break for anyone. (I am not a web extension developer, but tons of extensions do it this way). Using storage for an WE that cleans storage seems strange and problematic to me.

Edit: Or at the very least provide error handling if settings cannot be read/written (provide a warning). And even if you can't read/write settings, they should not stop the extension from doing the next step of actual cleaning (as per it never does anything in my case - never ends, always waiting).

@Gitoffthelawn
Copy link
Author

Gitoffthelawn commented Jul 29, 2017

The wise @Thorin-Oakenpants wrote:

Using storage for an WE that cleans storage seems strange and problematic to me.

+1

@andy-portmen
Copy link
Owner

This is fixed on 0.1.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants