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

extension: Re-enable SWF takeover using declarativeNetRequest #16694

Merged
merged 11 commits into from
Aug 7, 2024

Conversation

danielhjacobs
Copy link
Contributor

@danielhjacobs danielhjacobs commented Jun 12, 2024

This is based on w3c/webextensions#460. I added feature detection. This should be tested on Chrome Beta or above.

See screencast:

Screencast.from.2024-06-28.09-56-35.webm

@danielhjacobs danielhjacobs force-pushed the swf-takeover-dnr branch 3 times, most recently from 56c1c08 to ce55ea5 Compare June 14, 2024 16:23
@danielhjacobs danielhjacobs marked this pull request as ready for review June 17, 2024 15:50
@danielhjacobs danielhjacobs force-pushed the swf-takeover-dnr branch 7 times, most recently from 74c19bc to c967b88 Compare June 20, 2024 20:47
@danielhjacobs danielhjacobs force-pushed the swf-takeover-dnr branch 2 times, most recently from bf1ec66 to 0a20baf Compare June 21, 2024 19:53
@evilpie
Copy link
Collaborator

evilpie commented Jun 22, 2024

Can we at least wait until this is properly supported in Chrome so we don't have to do that annoying bug detection dance?

@danielhjacobs
Copy link
Contributor Author

danielhjacobs commented Jun 22, 2024

Can we at least wait until this is properly supported in Chrome so we don't have to do that annoying bug detection dance?

I don't disagree with waiting but we'll still need to decide what to do here. Supposedly Chromium does throw just like Firefox with a fully unsupported RuleCondition. The bug, as confirmed in https://issues.chromium.org/issues/347186592#comment9, is actually that features developed behind a flag are not fully hidden behind the flag in Chrome. Without this "annoying bug detection dance", with just a try and catch, we may end up in a situation where both older and newer versions of Chrome work properly with Ruffle but the versions that were implementing this feature behind a flag are broken. I'm not sure that's acceptable.

@evilpie
Copy link
Collaborator

evilpie commented Jun 22, 2024

the versions that were implementing this feature behind a flag are broken. I'm not sure that's acceptable.

I didn't quite realize this could happen. I sort of assumed this problem would still be limited to Canary.

Do we want/need to have a setting for disabling this specific feature?

@danielhjacobs
Copy link
Contributor Author

danielhjacobs commented Jun 22, 2024

Having a setting for disabling this feature may be nice in general since some people did find it frustrating when trying to just download SWFs. I added showSWFDownload to the player page for that reason.

However, I'm not sure that's enough. Try checking out ca7fc87 and using the extension in Chrome stable. https://google.com redirects to the player page. Needing to disable this feature to stop that is unintuitive.

@danielhjacobs danielhjacobs force-pushed the swf-takeover-dnr branch 3 times, most recently from 09bee68 to 4a2c6e7 Compare June 24, 2024 18:33
@danielhjacobs danielhjacobs force-pushed the swf-takeover-dnr branch 2 times, most recently from eb6c42e to 18d36b8 Compare July 24, 2024 15:57
@danielhjacobs danielhjacobs force-pushed the swf-takeover-dnr branch 3 times, most recently from 30cbb5c to df9a44f Compare August 5, 2024 22:31
@danielhjacobs danielhjacobs force-pushed the swf-takeover-dnr branch 2 times, most recently from 7f98af6 to 504520f Compare August 6, 2024 19:48
Copy link
Contributor

@Dinnerbone Dinnerbone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@danielhjacobs danielhjacobs enabled auto-merge (rebase) August 7, 2024 23:03
@danielhjacobs danielhjacobs merged commit 8481d99 into ruffle-rs:master Aug 7, 2024
15 checks passed
@danielhjacobs danielhjacobs deleted the swf-takeover-dnr branch August 8, 2024 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension Related to the Ruffle WebExtension newsworthy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants