1603699 - Enable DefaultURI use for unknown schemes

Status: ASSIGNED

(Core :: Networking, task, P3)

Description

[Valentin Gosu [:valentin] (he/him)]

Steps:

• [ ] Merge https://github.com/servo/rust-url/pull/537
• [ ] Update rust-url in Gecko
• [ ] Flip the network.url.useDefaultURI pref
• [ ] Fix expected-fail WPT tests
• [ ] Restrict origins to schemes allowed by the URL spec + the ones we add to the allow list (bug 1553105)

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Rust URL isn't perfectly ready yet for use, and I don't have the cycles to fix it right now. Unassigning for now.

Comment 2

[Ed Guloien [:edgul]]

Attachment: Bug 1603699 - Workaround to make DefaultURI return null origin. r=valentin

Comment 3

[Ed Guloien [:edgul]]

Attachment: Bug 1603699 - Enable default URI pref. r=#necko,valentin

Depends on D186796

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1603699 - Fix DefaultURI::Mutator::SetPathQueryRef for URLs that cannot be a base r=edgul

The expectation for SetPathQueryRef for URLs that cannot be a base is
that they will replace everything after the scheme.
Since the url crate doesn't have a method similar to SetPathQueryRef
we need to special case this in DefaultURI.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f3c08da2ac9
Fix DefaultURI::Mutator::SetPathQueryRef for URLs that cannot be a base r=edgul
https://hg.mozilla.org/integration/autoland/rev/f8a1e1d313cc
Enable default URI pref. r=valentin,necko-reviewers,extension-reviewers,robwu

Comment 6

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/7f3c08da2ac9
https://hg.mozilla.org/mozilla-central/rev/f8a1e1d313cc

Comment 7

[Tom S [:evilpie]]

We might want to consider doing a release note for this?

Comment 8

[Ed Guloien [:edgul]]

Can you point me in the right direction?

Comment 9

[Donal Meehan [:dmeehan]]

Drive by comment - the Release Note Process info should outline how to add a release note nomination on this bug. You provide some wording and release management will take it from there.

Comment 10

[Tom S [:evilpie]]

Thanks Donal.

Comment 11

[Ed Guloien [:edgul]]

Release Note Request
[Why is this notable]: This change alters the fallback URL parser to one that is more comprehensive. As a result of this change we are now passing hundreds more web platform tests which ultimately improves our spec adherence and web compatibility.
[Affects Firefox for Android]: Yes
[Suggested wording]: Changed our fallback URL parser for unknown schemes to DefaultURI for improved spec adherence and web compatibility

Comment 12

[Donal Meehan [:dmeehan]]

Thanks, added to the Fx122 nightly release notes, please allow 30 minutes for the site to update.
Keeping the relnote-firefox flag as ? to keep it on the radar for inclusion in the final Fx122 release notes

Comment 13

[Hamish Willee]

FF122 MDN docs for this can be tracked in https://github.com/mdn/content/issues/31100

But what is the direct impact of this change to website-developers? What should they do differently because of this?
I think the answer is nothing, and that this does not require MDN documentation right?

If not, can you explain what documents we might need to touch?

Comment 14

[Ed Guloien [:edgul]]

This shouldn't affect mdn content. Thanks.

Comment 15

[Hamish Willee]

Thank you

Comment 16

[Hamish Willee]

FYI, and I don't know who can update it, this means that the docs in https://firefox-source-docs.mozilla.org/networking/url_parsers.html for DefaultURI are out of date.

Comment 17

[Valentin Gosu [:valentin] (he/him)]

Thanks for pointing that out.
I'll make sure to update the docs.

Comment 19

[Tom S [:evilpie]]

I have strong doubts about the web compatibility of this change considering the number of regression.

Comment 20

[Valentin Gosu [:valentin] (he/him)]

(In reply to Tom S [:evilpie] from comment #19)

I have strong doubts about the web compatibility of this change considering the number of regression.

These issues would have been visible to people using safari, but Chrome hasn't shipped this change yet.
I assume we could have a list of protocols that we exclude from regular URL parsing.
The ed2k protocol seems like the best candidate for that, as even their wikipedia article explaining the URI format has this bad scheme: https://en.wikipedia.org/wiki/Ed2k_URI_scheme#File_link_format

I suppose we could also ship a webcompat intervention to fix specific links on specific sites, but I don't think that's possible for all broken protocols.

Ultimately the legacy protocol list is probably the best option. Thoughts?

Comment 21

[Tom Schuster (MoCo)]

I opened an issue in the WhatWG URL repository: https://github.com/whatwg/url/issues/815

Comment 22

[Valentin Gosu [:valentin] (he/him)]

We disabled this in bug 1877753.