Skip to content

web: If fetch fails because HttpNotOk, always display error-swf-fetch #20792

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

danielhjacobs
Copy link
Contributor

Unfortunately, it's another new parameter to display_root_movie_download_failed_message (see #14715), but I think it should work as another way to accomplish #20770 without a hard-coded URL.

@danielhjacobs danielhjacobs added A-web Area: Web & Extensions T-fix Type: Bug fix (in something that's supposed to work already) labels Jun 20, 2025
@danielhjacobs danielhjacobs changed the title web: Denote actual fetched URL when root movie download fails web: If fetch fails because HttpNotOk, always display error-swf-fetch Jun 20, 2025
@@ -2070,9 +2073,12 @@ export class InnerPlayer {
) {
this.addOpenInNewTabMessage(openInNewTab, this.swfUrl);
} else {
const fetchStatusNotOk = fetchError.includes(
"HTTP Status is not OK:",
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ruffle/core/src/loader.rs

Lines 182 to 183 in fb9bddd

#[error("HTTP Status is not OK: {0} redirected: {1}")]
HttpNotOk(String, u16, bool, u64),

@danielhjacobs
Copy link
Contributor Author

PR has been changed (commits should be squashed if this approach is taken, they are separate so I can revert to the old approach if preferred).

Now, if the fetch response has a status code, we display the error-swf-fetch message, even if the request was cross-origin.

@@ -185,6 +185,7 @@ function createPanicError(error: Error | null): {

if (
window.location.origin === error.swfUrl?.origin ||
error.statusNotOk ||
// The extension's internal player page is not restricted by CORS
Copy link
Contributor Author

@danielhjacobs danielhjacobs Jun 20, 2025

Choose a reason for hiding this comment

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

The other checks can probably be removed from here, but not certain.

Copy link
Member

@n0samu n0samu left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-web Area: Web & Extensions T-fix Type: Bug fix (in something that's supposed to work already)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants