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

web: Resolve URLs without a protocol in the extension player and improve error messages #16913

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

Conversation

Korne127
Copy link
Contributor

Currently, only URLs explicitly including the web protocol (HTTPS or HTTP) in the beginning can be resolved in the extension player. Additionally, if the URL doesn't include the HTTPS or HTTP protocol in the beginning, a wrong error is displayed. It states that the user ran Ruffle on the file: protocol which is wrong and confusing:
Bildschirmfoto 2024-06-28 um 15 53 04

This pull request fixes this.
With this pull request, the extension player resolves URLs without a protocol. It adds https:// or http:// if the URL is likely a valid web URL and file:/// if the URL is a valid file URL.
To determine whether HTTPS or HTTP should be used, Ruffle tests whether the server supports HTTPS. HTTP is only used if the server only supports HTTP and not HTTPS. This test happens while the loading screen is displayed.

This means that the user can now enter URLs without a leading https:// like you (usually) do in the browser.

Additionally, this pull request fixes and improves the error behaviour if the URL can't be resolved or used.
Instead of the wrong error, one of four more specific errors will be displayed:

  • One new error if the given URL is a local file URL:
    Bildschirmfoto 2024-06-28 um 15 56 26
  • One new error if the given URL uses a different protocol than https, http or file (e.g. ftp):
    Bildschirmfoto 2024-06-28 um 15 57 44
  • One new error if the given URL can't be resolved to any valid URL for any protocol
    Bildschirmfoto 2024-06-28 um 15 58 07
  • And the usual SWF file not found error if the file under the given web URL couldn't be found.
    That error has also been modified to also include the tip that the HTTP protocol should be (explicitly) used for websites that don't support HTTPS:
    Bildschirmfoto 2024-06-28 um 15 54 12

Also, a bug that caused the document title to not be set correctly if the SWF URL was the main page of a domain has been fixed. The handling of setting the document title of the extension player has generally been made more consistent.

@Croworbit
Copy link

hello Korne,
useful PR! :)
sorry but there is a typo on the 4th image!

@Korne127
Copy link
Contributor Author

Hey :)
Thanks for the nice response and the feedback :D
I'm sorry, but I genuinely don't see the typo… can you point it out for me? 😅

@@ -42,9 +42,12 @@ error-wasm-mime-type =
error-invalid-swf =
Ruffle cannot parse the requested file.
The most likely reason is that the requested file is not a valid SWF.
error-swf-fetch =
error-swf-fetch-v2 =
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the addition of the -v2? Crowdin will throw out all the old translations regardless, and if you're not also keeping error-swf-fetch as a separate translation string I don't see a reason to change this key.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Crowdin will throw out all the old translations regardless

Huh? I didn't know that. That's actually not really good because this is utilising a core Fluent feature: The separation of minor changes that don't affect translations and major changes that do affect them:

Fluent's use of unique identifiers allows for at least two of those three states to remain separated - minor changes may be applied and if the developer does not alter the unique identifier (a.k.a does not change the social contract), all translations remain valid. On the other hand if the developer changes the ID, all translations become invalid and must be updated.

I wish Fluent would have implemented the idea of message versioning they're investigating as a third option because this is the ideal case in which this would be used (as old translations are still better than none, but should be marked as outdated and updated eventually). However, the related issue hasn't been updated in years :/ I might try to get it more attention again in the future.

@Croworbit
Copy link

Hey :) Thanks for the nice response and the feedback :D I'm sorry, but I genuinely don't see the typo… can you point it out for me? 😅

"The url blabla test is no valid url"
I expected "the url blablatest is not a valid url"

@Korne127
Copy link
Contributor Author

Hmm, I would have thought that both is valid tbh, although the second one is probably better.
I can change it, but when I went back to look at the messages, I don't really like the repetition. I think I might change it to "Bla-Bla-Test is not a valid URL".
(And change enter to include on the last screenshot.)

@Croworbit
Copy link

amittedly its a nitpick 😅

@kjarosh kjarosh added the web Issues relating to the HTML5 frontend label Jun 28, 2024
@Korne127 Korne127 force-pushed the resolve-extension-player-urls branch from 699e67a to 86352c1 Compare June 29, 2024 12:56
@Korne127
Copy link
Contributor Author

@Croworbit Don't worry, I really appreciate it! Especially the things that I overlooked myself are good to know :)

}

try {
// Only use http if it works but https doesn't
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not totally convinced that we shouldn't just default to https:

Copy link
Contributor

Choose a reason for hiding this comment

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

That's because HTTP website cannot fetch HTTPS resources, and vice versa.

Copy link
Collaborator

Choose a reason for hiding this comment

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

How is this relevant? The "page" fetching this is our extension.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, then that won't be a problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

But what if the website only supports http and not https.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about that for a longer time too. But Ruffle is used to play Flash games, mostly legacy content. Old SWF files are especially likely to be hosted on outdated websites which don't support HTTPS yet.
And if the website uses HTTPS, that's just used. HTTP is only used if it's the websites only supports it, but not HTTPS.

Previously, only URLs explicitly including the web protocol (HTTPS or
HTTP) in the beginning could be resolved in the extension player.
This has been changed. The extension player now resolves URLs without a
protocol and adds https:// or http:// if the URL is likely a valid web
URL and file:/// if the URL is a valid file URL.
HTTP is only used if the server only supports HTTP and not HTTPS.
Previously, the Ruffle player had shown an incorrect error message if
the entered URL wasn't a valid web URL.
This has been fixed and improved. Three new error messages have been
added for this case: One in case the entered URL is a local file URL,
one in case the entered URL uses a different protocol than https, http
or file and one in case the entered URL isn't a valid URL for any
protocol at all.
Additionally, the error message in case Ruffle couldn't load the SWF
file (using a valid web URL) has been improved to also include the tip
that the HTTP protocol should be (explicitly) used for websites that
don't support HTTPS.
Previously, when entering a URL in the extension player, it had been
resolved before creating a RufflePlayer instance and displaying a
loading screen. As resolving can include testing for server availability
(to decide whether HTTPS or HTTP is used), this could create a wait time
without the loading screen.
This has been fixed. The URL is now resolved while the loading screen is
displayed. Additionally, the timeouts have been increased (they were low
because of the lack of a loading animation).

A bug that caused the document title to not be set correctly if the SWF
URL was the main page of a domain has also been fixed.
@Korne127 Korne127 force-pushed the resolve-extension-player-urls branch from 86352c1 to 1621c78 Compare June 30, 2024 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
web Issues relating to the HTML5 frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants