browser-ext-github-monaco icon indicating copy to clipboard operation
browser-ext-github-monaco copied to clipboard

feat: add support for GitHub Enterprise instances

Open outadoc opened this issue 4 years ago • 6 comments

This PR aims to add GHE support to this cool extension 🎉

It turns out other extensions like Refined Github use very cool packages to do this: https://github.com/fregante/webext-domain-permission-toggle and https://github.com/fregante/webext-dynamic-content-scripts.

They add a browser action that the user can right-click on to enable running content scripts on a specific domain, and automatically request the right permissions for it.

Screen Shot 2021-03-05 at 14 23 23

This is otherwise easily doable in Firefox, but Chrome lacks support for the required contentScripts.register API, so this polyfill is especially nice.

I've tested it a bit on the latest versions of Firefox and Chrome with no visible issues, feel free to test some more.

outadoc avatar Mar 05 '21 13:03 outadoc

Thanks a lot, I will have a closer look soon!

Looks good in general. However, I think it might be a good idea to lock the version of the two new dependencies. I doubt they ever have to be updated automatically.

Also, can you (and maybe also someone else) have a quick glimpse at the source code of these new dependencies that npm installed as well as their dependencies? I would like to avoid any security holes under any circumstances ;)

hediet avatar Mar 05 '21 13:03 hediet

Thanks! I've locked the versions and taken a look at the dependencies' code. They seem legit and I didn't notice anything fishy, but I'm not fluent in JS/TS, so take that with a grain of salt 🙂

outadoc avatar Mar 05 '21 16:03 outadoc

Sorry for the delay, I had to learn for exams. Are you sure this is not required to make it work?

It is very unfortunate that I cannot test this feature, as I don't have access to a github enterprise instance.

hediet avatar Mar 20 '21 13:03 hediet

Hi, no problem. 🙂

I believe it is; by default, when you programmatically whitelist a new allowed domain, your content scripts won't be run when you visit it. You need to explicitly register them for the new domain, which is what this package does (while providing a polyfill for Chrome).

If you need me to test a change, I can probably find some time to do so 👍

outadoc avatar Mar 20 '21 17:03 outadoc

Given that a significant amount of users (~15%) uninstalled this extension when this extension requested for more permissions in the last update, I think it is better to create a separate extension that enables GitHub enterprise support.

Thanks for you effort!

hediet avatar Apr 01 '21 16:04 hediet

when this extension requested for more permissions in the last update

Note that this setup does not require further permissions; "contextMenus" "activeTab" do not trigger a popup.

You can follow the guide at https://github.com/fregante/webext-dynamic-content-scripts/blob/main/how-to-add-github-enterprise-support-to-web-extensions.md

fregante avatar Mar 09 '23 17:03 fregante