wildskyf / two-col-google-search-for-firefox

https://addons.mozilla.org/en-US/firefox/addon/2-col-google-results/
3 stars 1 forks source link

Limit manifest to Google domains. #18

Open roeme opened 5 years ago

roeme commented 5 years ago

Instead of running for all websites, limit to google domains. This will speed up FF and make for a more trustworthy prompt upon installation, improving overall UX.

wildskyf commented 4 years ago

@roeme Sorry for my late response and thanks for your PR, but you missed some other domains, here is a list for you. and we support https://*.startpage.com/do/search too, please add it.

also, if you want to list it all, you might also need to remove the include_globs. ref: https://github.com/wildskyf/two-col-google-search-for-firefox/pull/18/files#diff-4b1eb3dc48c4e16d49db5b42298fe654R218-R222

roeme commented 4 years ago

@roeme Sorry for my late response and thanks for your PR, but you missed some other domains, here is a list for you.

No worries - but I think you forgot to link the list?

and we support https://*.startpage.com/do/search too, please add it.

done!

also, if you want to list it all, you might also need to remove the include_globs. ref: https://github.com/wildskyf/two-col-google-search-for-firefox/pull/18/files#diff-4b1eb3dc48c4e16d49db5b42298fe654R218-R222

If we were to remove include_glob, then we would need to double the size of content_script[matches], because the latter uses Match patterns, which need more verbosity than globs in order to limit the script loading. I think mixing include_glob and matches strikes a good balance.

wildskyf commented 4 years ago

Yes... I forgot to give you the link: https://github.com/wildskyf/personal-blocklist/blob/master/public/manifest.json#L40

Imo, you listed the whole list for content script to load, which is stricter that the global match. Leave it in manifest.json is a double limit, so I think it could be omitted.