Bug 1812192 Comment 5 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Tomer Yavor from comment #2)
> 1. We thought also to upgrade in [fixupURIProtocol](https://searchfox.org/mozilla-central/rev/3a13c5ad9c1dbd065e484a399af75eb98c235def/docshell/base/URIFixup.sys.mjs#957,960)  but @Gijs had some second thoughts about it. Since it gets called by more than only the URLbar functions it might lead to problems (@Gijs I hope I "recited" you correctly).

Long term it's the right thing to do, but I concur with the fact UriFixup is used by a lot of consumers, sometimes even inappropriately. The work Gijs is doing in Bug 1810141 and its dependencies would help creating a list of those consumers that still rely on fixup and see how they'd react to this change. Plus it will remove some of the inappropriate uses of it, making potentially easier to migrate.
The main consumer remains the urlbar of course, and my concerns are the ones I expressed in comment 1, related to not having a fallback if https doesn't effectively work for a certain origin.

> 2. To ensure that the fallback to http is guaranteed we thought to introduce a flag in [fixupURIProtocol](https://searchfox.org/mozilla-central/rev/3a13c5ad9c1dbd065e484a399af75eb98c235def/docshell/base/URIFixup.sys.mjs#957,960) that we would check [here](https://searchfox.org/mozilla-central/source/netwerk/ipc/DocumentLoadListener.cpp#2293,2311-2312) and use the HTTPSFirst fallback mechanism for that request.

Is the plan to allow consumers to selectively opt-in to https first fallback, so you can examine one consumer at a time?
Long term we may reach a point where all consumers are working fine with it, and the added argument could be removed.

> But since we were unsure if starting in [fixupURIProtocol](https://searchfox.org/mozilla-central/rev/3a13c5ad9c1dbd065e484a399af75eb98c235def/docshell/base/URIFixup.sys.mjs#957,960) is the correct call

I think so, the alternative would be to either:
* change default but allow the user to revert, that was our initial suggestion to just rely on browser.fixup.alternate.protocol. This of course may break users expectations without a proper fallback
* intercept the load, that I assume is what https-first does, but this will still show http in the urlbar and it's not the ideal long term target

> and also how to introduce a flag that reaches to [MaybeHandleLoadErrorWithURIFixup](https://searchfox.org/mozilla-central/source/netwerk/ipc/DocumentLoadListener.cpp#2293,2311-2312) we did not take further steps.

(In reply to Frederik Braun [:freddy] from comment #3)
> There's a point I want to make sure is emphasized clearly: If the address bar is able to transfer and hand down a flag that ends up in a loadState (and eventually loadInfo), we could tie this into the existing mechanisms for fallbacks.

All the urlbar loads go through a _loadURL method that calls into openTrustedLinkIn and finally to openLinkIn
https://searchfox.org/mozilla-central/rev/9de332d5c8faac58dc1232b8a6383ce6cb1400f4/browser/components/urlbar/UrlbarInput.sys.mjs#2613,2719
https://searchfox.org/mozilla-central/rev/9de332d5c8faac58dc1232b8a6383ce6cb1400f4/browser/base/content/utilityOverlay.js#276
There's already multiple params there.
There it may either open a window or a load in a tab, so you may have to handle both cases.
The browser then goes to nsiWebNavigation.LoadURI
https://searchfox.org/mozilla-central/rev/9de332d5c8faac58dc1232b8a6383ce6cb1400f4/browser/base/content/tabbrowser.js#7059,7061
That ends in a nsDocShellLoadState::CreateFromLoadURIOptions, so there should be no problem adding additional flags to reach it.
(In reply to Tomer Yavor from comment #2)
> 1. We thought also to upgrade in [fixupURIProtocol](https://searchfox.org/mozilla-central/rev/3a13c5ad9c1dbd065e484a399af75eb98c235def/docshell/base/URIFixup.sys.mjs#957,960)  but @Gijs had some second thoughts about it. Since it gets called by more than only the URLbar functions it might lead to problems (@Gijs I hope I "recited" you correctly).

Long term it's the right thing to do, but I concur with the fact UriFixup is used by a lot of consumers, sometimes even inappropriately. The work Gijs is doing in Bug 1810141 and its dependencies would help creating a list of those consumers that still rely on fixup and see how they'd react to this change. Plus it will remove some of the inappropriate uses of it, making potentially easier to migrate.
The main consumer remains the urlbar of course, and my concerns are the ones I expressed in comment 1, related to not having a fallback if https doesn't effectively work for a certain origin.

> 2. To ensure that the fallback to http is guaranteed we thought to introduce a flag in [fixupURIProtocol](https://searchfox.org/mozilla-central/rev/3a13c5ad9c1dbd065e484a399af75eb98c235def/docshell/base/URIFixup.sys.mjs#957,960) that we would check [here](https://searchfox.org/mozilla-central/source/netwerk/ipc/DocumentLoadListener.cpp#2293,2311-2312) and use the HTTPSFirst fallback mechanism for that request.

Is the plan to allow consumers to selectively opt-in to fixup to https, so you can examine one consumer at a time?
Long term we may reach a point where all consumers are working fine with it, and the added argument could be removed.

> But since we were unsure if starting in [fixupURIProtocol](https://searchfox.org/mozilla-central/rev/3a13c5ad9c1dbd065e484a399af75eb98c235def/docshell/base/URIFixup.sys.mjs#957,960) is the correct call

I think so, the alternative would be to either:
* change default but allow the user to revert, that was our initial suggestion to just rely on browser.fixup.alternate.protocol. This of course may break users expectations without a proper fallback
* intercept the load, that I assume is what https-first does, but this will still show http in the urlbar and it's not the ideal long term target

> and also how to introduce a flag that reaches to [MaybeHandleLoadErrorWithURIFixup](https://searchfox.org/mozilla-central/source/netwerk/ipc/DocumentLoadListener.cpp#2293,2311-2312) we did not take further steps.

(In reply to Frederik Braun [:freddy] from comment #3)
> There's a point I want to make sure is emphasized clearly: If the address bar is able to transfer and hand down a flag that ends up in a loadState (and eventually loadInfo), we could tie this into the existing mechanisms for fallbacks.

All the urlbar loads go through a _loadURL method that calls into openTrustedLinkIn and finally to openLinkIn
https://searchfox.org/mozilla-central/rev/9de332d5c8faac58dc1232b8a6383ce6cb1400f4/browser/components/urlbar/UrlbarInput.sys.mjs#2613,2719
https://searchfox.org/mozilla-central/rev/9de332d5c8faac58dc1232b8a6383ce6cb1400f4/browser/base/content/utilityOverlay.js#276
There's already multiple params there.
There it may either open a window or a load in a tab, so you may have to handle both cases.
The browser then goes to nsiWebNavigation.LoadURI
https://searchfox.org/mozilla-central/rev/9de332d5c8faac58dc1232b8a6383ce6cb1400f4/browser/base/content/tabbrowser.js#7059,7061
That ends in a nsDocShellLoadState::CreateFromLoadURIOptions, so there should be no problem adding additional flags to reach it.

Back to Bug 1812192 Comment 5