+++ This bug was initially created as a clone of Bug #1550945 +++

In bug 1536744, M-C removed nsIProtocolHandler.newURI and hard-coded all schemes in the system here:
https://searchfox.org/mozilla-central/source/netwerk/base/nsNetUtil.cpp
(sadly Searchfox hasn't updated at time of writing, look for scheme.EqualsLiteral("http").

We followed with
https://hg.mozilla.org/comm-central/file/tip/mailnews/base/util/nsNewMailnewsURI.cpp

I'll post real Searchfox perma-links when the become available.

That file hard-codes all schemes used in Thunderbird, including Calendar, but so far doesn't cater for JS Account which might be broken now although there are no test failures (keep in mind the JS Account tests which are switched off, see bug 1447492).

So while bug 1550945 fixed the bustage (some tests still failing at time of writing), there is scope for some clean-up:

1. Maybe fix the hard-coded include paths at the top of the file
2. Make sure JS Account is adequately catered for
3. Consider removing some "simple" URL implementations, see bug 1550945 comment #0, quote: "remove some of the implementations".

Ben, is this something your team can handle?

I don't know how this is supposed to work now. In JSAccount, it's the JS implementation that supplies the URL scheme. We use e.g. "owl" and "owl-ews" in Owl.

There is a fallback at the end:
https://hg.mozilla.org/comm-central/file/tip/mailnews/base/util/nsNewMailnewsURI.cpp

Does OWL still work in today's Daily?

(In reply to Jorg K from comment #2)

Does OWL still work in today's Daily?

To cut a long story short, Owl does not work in today's Daily.

http://lists.thunderbird.net/pipermail/maildev_lists.thunderbird.net/2018-March/001052.html
Quote:

So I concluded a year ago that JsAccount is not likely to be a viable
path forward for either new account types, or for JS implementations of
existing account types. Interesting experiment, and it can be made to
work, but just too complex and fragile.

I vaguely recall a discussion with the gist that in order to make URIs thread-safe, the ability to create them in JS code, also adding new schemes, would be discontinued at some stage. Sadly I can't find it now.

I came up with a few approaches to allow Owl to continue to work in Trunk versions of Thunderbird. I would have attached them earlier, but unfortunately my build predated yesterday's latest fixes from bug 1550945 that allowed inline images to display. Even so, for #3 I had to switch from nsMsgMailNewsUrl to JaUrl, otherwise the security checks didn't work (I didn't have time to find out why).

Nice, the master at work :-) - Please let me know which approach works best for you. Are you planning to port ExQuilla to TB 68? Ben posted somewhere (where? - I can't find it now) that Owl hasn't quite reached feature-parity with ExQuilla, so maybe a solution that works for both would be best.

EDIT: Ben's quote at http://lists.thunderbird.net/pipermail/maildev_lists.thunderbird.net/2019-May/001636.html

Owl already works for TB 68, and we have an ExQuilla for Thunderbird 68 in the testing phase and plan to release it soon.

I think #1 and #2 could be combined, in that you create it directly, if it's the main thread, and otherwise using the runnable.
I don't really understand #3, we'll have to talk about that internally.

(In reply to Ben Bucksch (:BenB) from comment #10)

Owl already works for TB 68, and we have an ExQuilla for Thunderbird 68 in the testing phase and plan to release it soon.

Great news!

Neil and I discussed this.

• Gecko moved the creation of the URI object into the thread that calls NewURI. That means they need to hardcode all the code for NewURI for all protocols and make sure that it's all thread safe, because the actual protocol handler might live on another thread than whatever created the URI object.
• Owl and ExQuilla implement NewURI themselves, and they also set the folder object on the URL. This folder object is then stored in the URL object and can be retrieved via GetFolder from a member variable. Thunderbird calls GetFolder() in various places (also for IMAP and POP3).
• With patch 3, Owl can no longer set the folder object on the URL, because Owl's implementation of NewURI is not called anymore, so we have to use a workaround to get the folder object from the server URL and query the folder every time. That's slower and also hardcodes everything in TB C++. It moves the code from Owl to TB and makes it less efficient and less flexible.
• With patch 2, TB continues to call NewURI implementation in Owl/ExQuilla, and they can continue to set the folder on the URL. However, we need to dispatch to the main thread, because Owl lives there.
• Patch 2 already avoids the dispatch, if we are already on the main thread. But it might be slightly faster to make the query explicit, by integrating patch 1 into patch 2. But that's strictly optional.

Neil will integrate patch 1 into patch 2, and post it.

Looking forward for to the final version ;-)

Explicitly proxies the call for clarity.

(In reply to comment #19)

but we could add initialisation code for these variables in Owl anyway.

Actually, we should definitely initialise the URL type; automatic resizing of attached images displayed inline isn't working in Owl right now.

(In reply to Jorg K from comment #22)

Comment on attachment 9069607 [details] [diff] [review]
Fix JSAccount

Review of attachment 9069607 [details] [diff] [review]:

::: mailnews/base/util/nsNewMailnewsURI.cpp
@@ +107,5 @@

•  if (NS_IsMainThread()) {
  

•    NewURI();
  

•  } else {
  

•    nsCOMPtr<nsIRunnable> task = NS_NewRunnableFunction("NewURI", NewURI);
  

•    mozilla::SyncRunnable::DispatchToThread(mozilla::GetMainThreadEventTarget(), task);
  

Excuse the ignorance, isn't there a NS_DispatchToMainThread?

That's not synchronous.

@Jörg: The patch already has review (and you can see that we did the review properly). Owl is completely broken on trunk without this patch.

Well, I was wondering why you never requested check-in. I'll land it in the next batch.

Thanks!

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/ec6a2a472a5e
New URI handling for JS Account based schemes. r=BenB

Actually, there were more things to do in this bug, see the list of 1. to 3. in comment #0. Looks like I have to file another bug now.

Please do a try run next time.

Try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=9f0fb7107a708b94982202fb7e355bd23f4418a7

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/37270fb55bca
New URI handling for JS Account-based schemes. r=BenB

Blame any strange formatting on clang-reformat:
https://hg.mozilla.org/comm-central/rev/37270fb55bca#l3.48