Bugfix FXIOS-11569 [Focus URIFixup] Always encode characters in valid URLs

[umbralOptimatum]

📜 Tickets

Jira ticket
Github issue

💡 Description

Encode spaces and other characters in otherwise-valid URLs using the default scheme instead of rejecting the URL entirely.

This PR replicates the same URIFixup changes from PR #25191 in the equivalent Firefox Focus file. I also expanded the unit tests to check that URLs without a scheme are handled correctly. I'm not experienced with this testing framework so please double check that the new function actually gets executed.

📝 Checklist

You have to check all boxes before merging

• Filled in the above information (tickets numbers and description of your work)
• Updated the PR name to follow our PR naming guidelines
• Wrote unit tests and/or ensured the tests suite is passing
• When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
• If needed, I updated documentation / comments for complex code and public methods
• If needed, added a backport comment (example @Mergifyio backport release/v120)

[adudenamedruby]

@razvanlitianu I haven't done much with Focus, so I'm tagging you here. This looks fine to me and I'd be good to approve, but just making sure.

[umbralOptimatum]

Looks like it's failing because Focus lacks the URLComponents checks that the current Firefox code uses, without which it lets through some malformed URLs (at least on older Swift versions). As I understand it, the project to replace Firefox's URIFixup will also replace the Focus version (see #21764), so would it make sense to just copy over the rest of the Firefox version's checks?

[lmarceau]

As I understand it, the project to replace Firefox's URIFixup will also replace the Focus version

That was indeed the case at that point in time, but plan has changed and that's not in the pipeline anymore. Either we adapt the code to do the bugfix in the Focus application, or we start using the BrowserKit.URLFormatter into the Focus project. I'm fine either way, but would love to have @razvanlitianu input

[umbralOptimatum]

I went ahead and added the additional URLComponents check for malformed URLs.

I do think it would make sense to move towards a single shared URL-handling class for ease of maintenance. Deciding which other functionality to add (e.g. punycoding, currently only in Firefox's URIFixup) is beyond me though.

[razvanlitianu]

lgtm, the only thing is testInvalidURLs() is now failing because some URLs became valid after escaping.

[github-actions]

This PR has been automatically marked as stale. Please leave any comment to keep this PR opened. It will be closed automatically if no further update occurs in the next 7 days. Thank you for your contributions!

[umbralOptimatum]

Haven't forgotten about this, I'm hoping to have the time in the next few days.

[github-actions]

This PR has been automatically marked as stale. Please leave any comment to keep this PR opened. It will be closed automatically if no further update occurs in the next 7 days. Thank you for your contributions!

[umbralOptimatum]

Think I've figured out the issue I was stuck on, hoping to finish this tonight.

[github-actions]

This PR has been automatically marked as stale. Please leave any comment to keep this PR opened. It will be closed automatically if no further update occurs in the next 7 days. Thank you for your contributions!