samrum / vite-plugin-web-extension

A vite plugin for generating cross browser platform, ES module based web extensions.
MIT License
334 stars 32 forks source link

Content scripts not loading in Chrome 130 following `use_dynamic_url` launch #144

Closed oliverdunk closed 1 month ago

oliverdunk commented 1 month ago

Hi, I work on the Chrome Extensions team and I wanted to reach out ahead of the launch of the use_dynamic_url feature in Chrome 130. It looks like this is set by vite-plugin-web-extension.

The use_dynamic_url property was mentioned in our documentation when Manifest V3 was first announced but didn't have support in Chrome, and we gracefully ignored the field. Regardless, we shouldn't have documented it, so sorry about that. We are now launching support but there is a known issue in this first implementation where importing a web accessible resource in a content script violates the CSP policy: https://issues.chromium.org/363027634

We're looking into addressing this, but it might not be fixed before Chrome 130 goes to stable, and seems to prevent any content scripts from loading when using this plugin. It's currently live in Canary, so you should be able to see the issue there.

It would be great if you could include some guidance around this for developers, or possibly change the default for useDynamicUrlWebAccessibleResources. We want to try and mitigate the impact as much as possible.

Feel free to reach out to me (oliverdunk@google.com) if you have any questions :)

SpaceK33z commented 1 month ago

Also want to add here that this will completely break ANY extension that is using this package combined with a content script (or just use_dynamic_url in another way) when Chrome 130 becomes stable, so this is a big deal. I really can't comprehend this decision from Chrome's team.

(I'm using this package and observed it breaking in the current Chrome Canary)

oliverdunk commented 1 month ago

Hi @SpaceK33z, could you elaborate on the situations you're concerned about? This should only impact use_dynamic_url used in combination with an import statement and not any other usage.

Our main reason for deciding to go ahead here is that prior to Chrome 130, the property was not supported - so any extensions with that property in the manifest were not actually benefitting from it. Therefore, impacted extensions can set the key to false to get the same behavior they had in Chrome 129. Meanwhile, extensions that aren't impacted by this specific bug can make use of the feature which is definitely a positive in terms of reducing fingerprinting across the web.

There are certainly tradeoffs and we're open to feedback here but generally launching things is a good step as it can help us identify any other issues and make sure we get to a good place sooner :)

SpaceK33z commented 1 month ago

Sure; I'm concerned about that exact situation, with use_dynamic_url in combination with import statement.

It's very common to load more JS like this, it's become a very basic use case and lots of bundlers do this automatically.

I understand that this property was first not supported, and that that was a bug in itself, but what I don't understand is that now it's clear that extensions are relying on this, you're still going ahead with this, purposefully breaking extensions (with seemingly just assuming that it has a low impact), even though this change could also just as well be delayed until there's a fix for this. Since it wasn't supported anyway for a long time, why rush to support it now in a way that's broken?

oliverdunk commented 1 month ago

We definitely aren't making quick assumptions about the impact - we have a lot of tooling to see what extensions across the Chrome Web Store are doing and looked at things like usage of this library as part of that.

The other side of this is that there are extensions that would like to adopt this, but are waiting for it to launch, and extensions which already enable it in the manifest and wouldn't break with these changes. So there are definitely positives to shipping it. The impact of this issue also came to light after Chrome 130 had made it to beta - we can still make changes in beta, of course, but there's a higher bar.

samrum commented 1 month ago

Hey, thanks for reaching out. It seems like adding that useDynamicUrlWebAccessibleResources option and defaulting it to true was some overly optimistic thinking on my part, so this is unfortunate.

Assuming the issue won't be fixed before the release of Chrome 130, I can push a plugin release to default the option to false and add a note to the readme mentioning the issue and cautioning against its use.

Is there anything we could do to notify users of the plugin to update to get the new default before their extensions break, though?

xPaw commented 3 weeks ago

@oliverdunk Since I found this issue from searching, it's unacceptable that such a change shipped in stable Chrome without actually being fixed.

I don't use vite, but it affected my extension because I do similar things: create a dynamic script tag and add it to the page from a content script: https://github.com/SteamDatabase/BrowserExtension/blob/master/scripts/store/invalidate_cache.js

So I had to disable use_dynamic_url and submit a new version for review in the store... Just a quick search on github shows that affects many other extensions.

oliverdunk commented 3 weeks ago

I'm a little late, but thanks so much for the fix @samrum. On our side, I've reached out to a number of extensions impacted by this, and I'm going to continue discussing with the team to see if this is something we can support. I'll make sure to reach out with any updates.

@xPaw, I'm really sorry for the trouble you've had with this (I've also seen some of the other places you've mentioned this). At this point I think the best path forward is to see if we can get a fix into one of the next versions of Chrome. I'm still hopeful we'll be able to do that for anyone that hasn't been able to update and so you can use use_dynamic_url in the future.

oliverdunk commented 2 weeks ago

Hi all, just checking in to say that we've landed a fix for this which is available in today's Chrome Canary. Thanks for all of the feedback - the impact here was greater than we hoped and it allowed us to prioritize working on the change. @xPaw, this doesn't yet address the issue you mentioned but we're still planning to take a look at that.

14790897 commented 2 weeks ago

Please prioritize and expedite the fix @oliverdunk