1527462 - Backout temporary fix allowing Thunderbird to use % in URLs from bug 1527236, rev. aa39bdeb511f - Make Thunderbird internal URL escaping standards compliant

Status: NEW

(Core :: Networking, enhancement, P5)

Description

[Jorg K (CEST = GMT+2)]

In bug 1527236 a patch was landed that allowed TB to use % in its mailnews: URLs.

Once TB has reworked that area, the patch can be backed out:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa39bdeb511f
https://hg.mozilla.org/mozilla-central/rev/aa39bdeb511f

Comment 1

[Jorg K (CEST = GMT+2)]

Also see bug 1527236 comment #27 for some comments.

Comment 3

[Henri Sivonen (:hsivonen)]

The "temporary" fix is problematic when pushing the check for WHATWG forbidden domain code point deeper into the dependencies (all the way to the idna crate).

mkmelin, how feasible would it be to implement option 2 or 3 from bug 1527236 comment #27?

Comment 4

[Magnus Melin [:mkmelin]]

It should be feasible, but does require a fair bit of work.
We also do an annoying non-standards compliant re-escaping on the username. Should fix that at the same time: https://searchfox.org/comm-central/rev/0a3cadbb953e3caeda7f2d4a9b349a60d376ba25/mailnews/imap/src/nsImapUtils.cpp#65

Comment 5

[Henri Sivonen (:hsivonen)]

(In reply to Magnus Melin [:mkmelin] from comment #4)

It should be feasible, but does require a fair bit of work.

Thanks. I interpret that as too much work to happen before bug 1889536. In bug 1889536, I'm now in the process of retaining this quirk not for all URLs in Thunderbird and Suite builds but for mailnews: URLs in any build. Are there more schemes that need this special case?

For future reference, my current expectation is that the relevant code will move from nsStandardURL.cpp to netwerk/base/idna_glue/src/lib.rs.

Comment 6

[[e-mail]]

Are you assuming that mailnews: is a scheme? It's not. The schemes used by MailNews Core code can be found here and below:
https://searchfox.org/comm-central/rev/6ed52969390e2998785fb7587ef2a0b798bc8469/mailnews/base/src/nsNewMailnewsURI.cpp#48
Called here:
https://searchfox.org/mozilla-central/rev/e69f323af80c357d287fb6314745e75c62eab92a/netwerk/base/nsNetUtil.cpp#2020

Comment 7

[Mathew Hodson]

Is this a duplicate of bug 1908033?

Comment 8

[Magnus Melin [:mkmelin]]

Kind of but I think that just moved where the workaround is.
I think the root cause is still there, no?

Comment 9

[Geoff Lankow (:darktrojan)]

There is still a workaround in mozilla-central, yes. We have got a plan to remove the need for it altogether, hopefully sometime in the next ~12 months. (I know comments like that often age really badly, but we can hope this one doesn't!)