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

Finally test managed storage #4

Open
rugk opened this issue Apr 2, 2019 · 5 comments
Open

Finally test managed storage #4

rugk opened this issue Apr 2, 2019 · 5 comments
Labels
good first issue Good for newcomers hacktoberfest Hacktoberfest 2020, https://hacktoberfest.digitalocean.com

Comments

@rugk
Copy link
Owner

rugk commented Apr 2, 2019

Tracked/tested in rugk/offline-qr-code#177

Guide is currently written for that other add-on, but can be adjusted for this one. (most is just the same)

@rugk rugk added the good first issue Good for newcomers label Apr 2, 2019
@rugk rugk added the hacktoberfest Hacktoberfest 2020, https://hacktoberfest.digitalocean.com label Sep 25, 2019
@rugk rugk removed the hacktoberfest Hacktoberfest 2020, https://hacktoberfest.digitalocean.com label Nov 7, 2019
@rugk rugk added the hacktoberfest Hacktoberfest 2020, https://hacktoberfest.digitalocean.com label Sep 29, 2020
@tdulcet
Copy link
Contributor

tdulcet commented Oct 3, 2020

Note that this code block does NOT work in Chrome: https://github.com/TinyWebEx/AutomaticSettings/blob/26c51c6ac40171731f11ec4855a93c591811e301/internal/LoadAndSave.js#L167-L178 and thus blocks #54. It causes all the settings to be identified as managed, which effectively disables the options page:
image

A simple solution is the replace that block with return Promise.reject();, which makes the options page work, but obviously disables the managed options feature.

@rugk
Copy link
Owner Author

rugk commented Oct 3, 2020

Uhhh, that is unfortunate. The problem seems to be Chrom/ium only supports the sync and local settings storage, i.e. no managed at all.

The question is just, why does browser.storage.managed then succeed in Chrome/ium? Should not it throw an error?
But well, the fix would then be a browser check and disable that feature for that browser.
Or better maybe "polyfill" the browser.storage.managed API, so it throws an NotImplemented exception or so…

@tdulcet
Copy link
Contributor

tdulcet commented Oct 4, 2020

Uhhh, that is unfortunate. The problem seems to be Chrom/ium only supports the sync and local settings storage, i.e. no managed at all.

No, Chrome does support managed storage (also see here). That was the first thing I checked when I noticed this issue... 🙂

BTW, there is a link to try it in Chrome at the bottom of this comment. Just rename the chromemanifest.json file to manifest.json.

@rugk
Copy link
Owner Author

rugk commented Oct 4, 2020

Hmm ok, so what's the issue in Chrome/ium then… maybe the fact that we use Promises is bad and it needs this polyfill?

@tdulcet
Copy link
Contributor

tdulcet commented Oct 4, 2020

I am not sure, something in that code block... I stopped debugging it after I found that workaround, which allowed me to finish testing this extension in Chrome. I did use the WebExtension polyfill, as nothing would have worked without that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers hacktoberfest Hacktoberfest 2020, https://hacktoberfest.digitalocean.com
Projects
None yet
Development

No branches or pull requests

2 participants