fregante / webext-permission-toggle

Browser-action context menu to request permission for the current tab.
https://fregante.github.io/webext-permission-toggle/
MIT License
70 stars 5 forks source link

Add a checkmark and the ability to toggle the permission #8

Closed fregante closed 5 years ago

fregante commented 5 years ago

Fixes https://github.com/bfred-it/webext-domain-permission-toggle/issues/4 Fixes https://github.com/sindresorhus/refined-github/issues/1509

Full rewrite sadly necessary to enable that little checkmark:

checkbox

I looked into a few simpler methods but they didn't work:

Also I couldn't find anything simpler than these to pick which domain to enable it on:

https://github.com/bfred-it/webext-domain-permission-toggle/blob/325b814979d0e803532ead364848f5ac8a823152/webext-domain-permission-toggle.js#L98-L103

https://github.com/bfred-it/webext-domain-permission-toggle/blob/325b814979d0e803532ead364848f5ac8a823152/webext-domain-permission-toggle.js#L45-L56

fregante commented 5 years ago

Things to look forward: https://blog.chromium.org/2018/10/trustworthy-chrome-extensions-by-default.html

In 2019 we will introduce the next extensions manifest version .... Some key goals of manifest v3 include: More narrowly-scoped and declarative APIs, to decrease the need for overly-broad access and enable more performant implementation by the browser, while preserving important functionality

sindresorhus commented 5 years ago

How can I test this without having access to GitHub Enterprise?

fregante commented 5 years ago
  1. Visit google.com
  2. “Enable Refined GitHub…”
  3. See obvious errors in console

The injection part hasn’t changed, but the permission management has

busches commented 5 years ago

Testing this via https://github.com/sindresorhus/refined-github/pull/1774

If I'm on GH, I see it checked, I remove it, RGH is disabled, but if I reenable it on GH, it doesn't turn on, but the checkmark says it's enabled.

Testing on GHE, for a domain that was previously added to permissions, first load RGH is working. Disable it, RGH is disabled. Also, I noticed if I disable it, it's coming back as checked on the next page load, but RGH is disabled. The extension tells me I need to refresh the page, even though I did. I used the extension to refresh, still shows as checked, but RGH is not working. It shows a weird state: image I'm now in a state when I uncheck the domain and it thinks it's loaded for the domain. If I uncheck it (even though it's already removed the permission), then recheck it, it prompts for permission. If I grant it, RGH is still not working.

I can see the permissions exist for GH and GHE but it's not working on either: image

fregante commented 5 years ago

That’s… not right. This module only handles the permissions; if you can see your domain as officially allowed in the chrome page then it really is, and the content scripts should be automatically injected by the other module (unrelated, untouched by this PR)

fregante commented 5 years ago

Can you check again? The github.com permission cannot be programmatically revoked and there’s nothing we can do to prevent its injection, so if you don’t see RGH there there’s something bigger at play (Chrome bug or you’re testing it wrong) 😅

Alternatively you might have disabled it from the native “Page access” menu, hence the red X on the icon.

busches commented 5 years ago

Retested. It's again working when I enable it, not sure what happened.

I can still "disable" it on GitHub, it doesn't actually disabled RGH, but the checkbox is missing.

With that said, when it's not enabled on the domain and we click the icon, we get "Refresh Required" message and I find it confusing. Refreshing the page doesn't matter if I haven't enabled it for the domain yet. Can we tie the "Ok, Refresh" button to also asking for the domain permission if it's not there?

fregante commented 5 years ago

The check is already there. In my case I never see the red X on the icon and that might be throwing off the system. At every step I verify that we have permanent permission to the domain. If not, the check will be hidden.

In my case, I receive an Alert saying that I can’t disable it on GitHub.com

What Chrome version are you on? Can you make sure no flags are enabled in chrome://flags?

busches commented 5 years ago

The issue is likely Chrome versions, I'm locked to 64 currently when using GHE. This requires a change to the manifest and to update the catch{} blocks in two places to get it to run. I have not tested it at home, since it would be the same testing you're doing. :)

fregante commented 5 years ago

Alright, there are indeed some yucky Chrome bugs:

Also after clicking that context menu item we will still have access to the current tab until we navigate away from the current domain (thanks to activeTab) — regardless of the permissions we actually have.

This might have caused some issues to your testing and it means I'll have to rewrite https://github.com/bfred-it/webext-dynamic-content-scripts to have the browser inject the scripts where they're permanently allowed, and not on activeTab

Closing for now until https://github.com/bfred-it/webext-dynamic-content-scripts/issues/4 is done.

fregante commented 5 years ago

Beautiful Chrome extensions bug:

screenshot 2019-02-16 at 00 03 48

chrome.permissions.getAll will report origins from content_scripts (manifest.json) but chrome.permissions.contains only looks in the permissions field


Edit: note to future self: this can probably be fixed by checking whether a domain is declared in the manifest, via chrome.runtime.getManifest().content_scripts

Also that webext-dynamic-content-scripts activeTab issue can be fixed by using the same mechanism to detect whether the current origin is allowed.

fregante commented 5 years ago

Will test version v1.0.0-2

fregante commented 5 years ago

This is now completely disabled on domains that can't be removed: