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

Remove usage of content scripts #61

Open
dellagustin opened this issue Feb 2, 2020 · 3 comments
Open

Remove usage of content scripts #61

dellagustin opened this issue Feb 2, 2020 · 3 comments
Labels
Projects

Comments

@dellagustin
Copy link
Contributor

Reasoning

Although not explicitly mentioned in the Chrome API documentation:

The usage of Content Scripts as used in podStation:

"content_scripts": [
	{
		"matches": ["http://*/*", "https://*/*"],
		"js": [
			"lib/jquery.min.js",
			"feedFinder.js"
		]
	}
],

Does cause the installation warning: Read and change all your data on the websites you visit

Why do we use content scripts?

At the moment, we inject the script https://github.com/podStation/podStation/blob/master/extension/feedFinder.js into every visited webpage to detect rss feeds.

When a feed is found, the extension button on the toolbar will change and give you the option to add the podcast from a popup window:
image

About the feed finder feature

When I introduced this feature, it sounded like a cool idea.
It is used, at least according to analytics:
image

More than 1000 times over the course of 6 months.

I particularly do not find it very useful, as many podcast webpages will not correctly expose the podcast feed in the webpage metadata, but I do not want to de-comission the feature.

Alternative implementation

My idea on how to keep this feature while avoiding injecting a script in every webpage the user visits is to use chrome.declarativeContent, maybe in combination with the activeTab permission.

@dellagustin dellagustin added this to Planned in podStation via automation Feb 2, 2020
@dellagustin
Copy link
Contributor Author

dellagustin commented Feb 2, 2020

Research Result

The plot thickens...

It happens that the Declarative Content API is very very limited.
I could possibly implement this feature but we would lose some stuff.
Currently when a pattern is matched on a website, we can do two things at best:

  • Show the Page Action (not useful, as it can't be used together with Browser Action)
  • Set the Icon

I can't have the same behavior, nor show a pop up.
The declarative API would need to enhanced for that.

The alternative would be, we change the icon to something else, and when clicked, it would have run the content script (using the "activeTab" permission), and then show a page with the feeds it found (no pop up anymore).

The amount of change makes me really think about letting this feature go...

References

@mikhoul
Copy link

mikhoul commented Dec 7, 2020

Why don't you put the " feed finder feature" optional in the settings ?

This way users that don't want a script to be injected inside every page would keep the option disabled.

You could also put a button accessible from the toolbar to scan manually the current page for "feeds".

Regards :octocat:

@dellagustin
Copy link
Contributor Author

Hi @mikhoul , sorry for the late reply.
Thank you for the comment.

Why don't you put the " feed finder feature" optional in the settings ?

That is an option, I just don't know if it is actually worth it. It would be turned off by default, and I guess most users would never even know it exist.

You could also put a button accessible from the toolbar to scan manually the current page for "feeds".

I am not sure that is actually possible, last time I checked an extension could only have a single button. I think it would be possible to scan the current tab when the button is pressed and show the pop up only then, but having a specific badge when the page contains a feed would not be possible anymore (by the way, please vote for https://bugs.chromium.org/p/chromium/issues/detail?id=893375, that would make it possible to use the declarative content API and show the badge without actually adding a script to every page).

@dellagustin dellagustin moved this from Planned to Accepted in podStation Oct 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
podStation
  
Accepted
Development

No branches or pull requests

2 participants