1478441 - Introduce nsIURIWithSpecialOrigin for Thunderbird to replace nsIURIWithPrincipal which was removed in bug 1228139

Status: RESOLVED FIXED

(Core :: Networking, enhancement, P5)

Description

[Jorg K (CEST = GMT+2)]

Usage of nsIURIWithPrincipal was introduced in Mailnews in bug 1347598 when its removal was already on the books in bug 1228139 :-(

I understand that to solve the <img crossorigin="anonymous" ...> problem, we were relying on the code that just got removed here:
https://hg.mozilla.org/mozilla-central/rev/09917998d963#l1.36 or
https://hg.mozilla.org/mozilla-central/rev/09917998d963#l2.56

Boris, can you please suggest a replacement.

Meanwhile I'll have to do whatever emergency surgery that is necessary to get us compiling again.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Andrea?  Note that this is an _implementation_ of nsIURIWithPrincipal that is not blob, so I'm not sure there's a way to do this right now after bug 1228139 landed...

Comment 2

[Jorg K (CEST = GMT+2)]

Well, if you want to stick with the removal of nsIURIWithPrincipal, you might need some conditional code in ContentPrincipal.cpp. Something like

#ifdef MOZ_THUNDERBIRD
  nsCOMPtr<nsIMsgMailNewsUrl> uriWithPrincipal = do_QueryInterface(origin);
  if (uriWithPrincipal) {
    nsCOMPtr<nsIPrincipal> uriPrincipal;
    rv = uriWithPrincipal->GetPrincipal(getter_AddRefs(uriPrincipal));
    NS_ENSURE_SUCCESS(rv, rv);
    return uriPrincipal->GetOriginNoSuffix(aOriginNoSuffix);
  }
#endif

Intolerable, right? Let alone the problem to get to nsIMsgMailNewsUrl.

Comment 3

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-part1.patch - emergency surgery

This would be the emergency surgery. I could add
  readonly attribute nsIPrincipal principal;
  readonly attribute nsIURI principalUri;
to nsIMsgMailNewsUrl.idl to maintain the getter functions, but there's no point since no one would call them.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Well, the other option is something similar to ORIGIN_IS_FULL_SPEC but that delegates some sort of complex checking to the URI impl.

And another option is to change mailbox: URIs to be nsIStandardURL, with per-message hostnames, and then just remove the ORIGIN_IS_FULL_SPEC thing and have them do security checks like any other hostname-based URI.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/7ee9f9040488
Port 1228139: Replace use of nsIURIWithPrincipal, part 1, emergency measures. rs=bustage-fix

Comment 6

[Jorg K (CEST = GMT+2)]

OK, I've just extinguished the fire, so now let's see what to do with the debris ;-(

Boris, thanks for comment #4, sadly I'm not an expert on any of that, but since the departure of Kent, the last man standing. So let's see. Looks like we already do ORIGIN_IS_FULL_SPEC, see:
https://searchfox.org/mozilla-central/rev/bdfd20ef30d521b57d5b6feeda71325e8b4cad66/netwerk/base/nsIProtocolHandler.idl#19
So I don't know what "something similar to ORIGIN_IS_FULL_SPEC" is meant to mean.

The next option is to change our mailbox: URIs, which are mostly like file: URIs to include a hostname? We (sadly) already have various formats (quoting from some old bugs):

mailbox://[e-mail]@server.de/Inbox?number=1
mailbox:///C:/Users/jorgk/AppData/Roaming/Thunderbird/Profiles/5gfhfw7f.POP%20tbtest/Mail/server.de/Inbox?number=1
and there's also
mailbox://nobody@Local%20Folders/xxx
and
mailbox:///C:/Users/jorgk/AppData/Local/Temp/nsmail-13.eml&number=0 for single messages.

Making any changes there would be a major operation.

So what's at the base of the <img crossorigin="anonymous" ...> we tried to solve in the first place?

Comment 7

[Jorg K (CEST = GMT+2)]

I think I remember it now. Since the origin is the full spec, so with all the little bits and pieces Mailnews adds, there is no chance that Gecko can do any decent security checks. That's why we introduced the "principal spec" which was a normalised spec without the disturbing bits, and that was retrieved via nsIURIWithPrincipal. Anyway, something along those lines.

But even if I added a server/hostname, it would be hard to do for local folders. They would all get the same server, well, unless I invent a "per folder" server. Oh, you even talked about a "per-message hostname" in comment #4.

So something like:
mailbox://[e-mail]/C:/Users/jorgk/etc/Inbox?number=1
That's certainly a big ask.

And we're only talking mailbox: here? There are IMAP messages with imap:// URLs. They already have a server, but you don't want one message on the server to get access to other messages on the same server. So there you'd need a "real" server to address the message and a "fake" per-message hostname for security checking? Hmm. It was certainly more handy to be able to tell Gecko which spec to use for the checks.

Comment 8

[Boris Zbarsky [:bzbarsky]]

> mailbox://[e-mail]/C:/Users/jorgk/etc/Inbox?number=1

Well.   You could do something like, using your examples from comment 6:

  mailbox://tbtest%40example.com%40server.de%2FInbox%3Fnumber%3D1
  mailbox://%2FC%3A%2FUsers%2Fjorgk%2FAppData%2FRoaming%2FThunderbird%2FProfiles%2F5gfhfw7f.POP%2520tbtest%2FMail%2Fserver.de%2FInbox%3Fnumber%3D1
  mailbox://nobody%40Local%2520Folders%2Fxxx

As in, take the entire bit after "//", or just the "principal spec" part, escape it, and use the result as the hostname.

> And we're only talking mailbox: here?

Presumably whichever things currently implement nsIURIWithPrincipal...  For imap:// there might be complication here, I agree.

I think we should see what Andrea says.

Comment 9

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-disable-test.patch

Comment 10

[Boris Zbarsky [:bzbarsky]]

I thought I had submitted this....

> So I don't know what "something similar to ORIGIN_IS_FULL_SPEC" is meant to mean.

I mean another flag, that says "for this URI, ask it directly whether the two URIs are same-origin or not"  or some such.  This would presumably need a new interface such URIs would need to implement; whether that's something we really want is unclear.

> The next option is to change our mailbox: URIs, which are mostly like file: URIs to include a hostname?

Yes.  Fundamentally, the web security model is built around hostnames.  So the simplest way to get security boundaries is to make them look like hostnames.

> Making any changes there would be a major operation.

Sure.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/37be8758d57e
Part 2: disable failing test test-image-display.js. rs=bustage-fix

Comment 12

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-M-C-part.patch - WIP

I need to head out for a few hours, so I thought I'll present my idea as a WIP. I didn't get it compiled yet, somehow the derived class nsMsgMailNewsUrl doesn't appear to see the method to override it.

Anyway, perhaps something along these lines can work. No word from Andrea yet.

Comment 13

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-M-C-part.patch (v2)

I wrestled a bit with the compiler. The problem is that
#if defined(MOZ_THUNDERBIRD) || defined(MOZ_SUITE)
#define IS_ORIGIN_IS_FULL_SPEC_DEFINED 1
#endif
is in nsIProtocolHandler.idl and that's not included everywhere.

So for now I duplicated this. So far it compiles. Changing nsIURI.idl is no fun since it recompiles half the system.

I'll ask for review when I have the C-C part.

Comment 14

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-GetPrincipal-C-C-part.patch

This is the C-C part. However, I can't test it since the program doesn't even start, not even with only the M-C patch applied. So those M-C changes are pretty destructive.

The debug console says:
[5900, Unnamed thread 193be60c800] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file c:/mozilla-source/comm-central/xpcom/base/nsTraceRefcnt.cpp, line 209
[5900, Main Thread] WARNING: NS_ENSURE_SUCCESS_VOID(rv) failed with result 0x80520012: file c:/mozilla-source/comm-central/dom/base/nsFrameMessageManager.cpp, line 1381
[5900, Main Thread] WARNING: CheckLinkStatus called on main thread! No check performed. Assuming link is up, status is unknown.: file c:/mozilla-source/comm-central/netwerk/system/win32/nsNotifyAddrListener.cpp, line 710

What is so destructive about that I'm doing. Boris, could you take a look please.

Comment 15

[Jorg K (CEST = GMT+2)]

That seems like a false alarm. Here are the equivalent lines when running without the M-C patch:

[10940, Main Thread] WARNING: CheckLinkStatus called on main thread! No check performed. Assuming link is up, status is unknown.: file c:/mozilla-source/comm-central/netwerk/system/win32/nsNotifyAddrListener.cpp, line 710
[10940, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file c:/mozilla-source/comm-central/netwerk/protocol/res/SubstitutingProtocolHandler.cpp, line 342

Comment 16

[Boris Zbarsky [:bzbarsky]]

> What is so destructive about that I'm doing

Well, you're adding stuff to the nsIURI vtable that is in a C++ block, but xpconnect doesn't know about it.  That will make calls through xpconnect into any idl API that inherits from nsIURI jump off into lala-land.  That includes nsIMozIconURI and nsIURL.

That's my best guess for what's going wrong, at least.

Comment 17

[Jorg K (CEST = GMT+2)]

Thanks, yes, that sounds plausible. So how to fix that damage? And what about the approach in general?

Comment 18

[Boris Zbarsky [:bzbarsky]]

Don't change nsIURI.  Add a new interface or something and QI to it...

Comment 19

[Jorg K (CEST = GMT+2)]

(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #18)
> Don't change nsIURI.  Add a new interface or something and QI to it...
OK. So we basically add parts of what was removed back again. Call it nsIURIWithPrincipal? ;-) - Then I can just back out the C-C patches and everything will work again.

Comment 20

[Boris Zbarsky [:bzbarsky]]

Again, please talk to Andrea.

Comment 21

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-restore-nsIURIWithPrincipal.patch (v1)

This restores the bits we need.

Comment 22

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-restore-nsIURIWithPrincipal.patch (v1b)

Fixed ugly variable name. Sorry 'bout the noise.

Comment 23

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-restore-nsIURIWithPrincipal.patch (v1c)

Typo in Mailnews :-( - Sorry again about more noise.

Comment 24

[Jorg K (CEST = GMT+2)]

Comment on attachment 8995361 [details] [diff] [review]
1478441-restore-nsIURIWithPrincipal.patch (v1c)

Well, no word from Andrea. Maybe he'll react to a review request.

Comment 25

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8995361 [details] [diff] [review]
1478441-restore-nsIURIWithPrincipal.patch (v1c)

Andrea should review this.

Comment 26

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-restore-nsIURIWithPrincipal.patch (v1c) - changed reviewer

Comment 27

[Jorg K (CEST = GMT+2)]

Note that there is already code in M-C to support Thunderbird's URLs:
https://searchfox.org/mozilla-central/search?q=IS_ORIGIN_IS_FULL_SPEC_DEFINED&case=false&regexp=false&path=

So restoring previously removed code protected with a conditional compile shouldn't be a problem, IMHO. Not having this code regresses bug 1347598. Fixing the issue any other way would entail significant effort and great risk.

Comment 28

[Andrea Marchesini [:baku]]

Comment on attachment 8995442 [details] [diff] [review]
1478441-restore-nsIURIWithPrincipal.patch (v1c) - changed reviewer

Review of attachment 8995442 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/base/nsIURIWithPrincipal.idl
@@ +10,5 @@
> + * nsIURIWithPrincipal is implemented by URIs which are associated with a
> + * specific principal. This is exclusively used in comm-central's Mailnews module.
> + */
> +[scriptable, builtinclass, uuid(626a5c0c-bfd8-4531-8b47-a8451b0daa33)]
> +interface nsIURIWithPrincipal : nsISupports

Sorry, I was in PTO Thursday and Friday.
This patch re-introduce nsIURIWithPrincipal just in a couple of places.

You should probably restore these too:

https://dxr.mozilla.org/mozilla-beta/rev/fc0a1b63ccc727f9cb4ed424d3ac88e86529fc46/caps/NullPrincipal.cpp#184
https://dxr.mozilla.org/mozilla-beta/rev/fc0a1b63ccc727f9cb4ed424d3ac88e86529fc46/netwerk/base/nsNetUtil.cpp#2520

Probably Thunderbird doesn't use first-party-isolation. Ignore this:
https://dxr.mozilla.org/mozilla-beta/rev/fc0a1b63ccc727f9cb4ed424d3ac88e86529fc46/caps/OriginAttributes.cpp#67

There are 2 reasons why I wanted/want to remove nsIURIWithPrincipal:
1. in firefox, this is used only for blobURLs and I don't want to implement 'remote' blobURLs for content processes: a blobURL doesn't know its principal/blobImpl until it's opened for real.

2. We want to make nsIURI and nsIPrincipal thread-safe objects. nsIURI is partially thread-safe (the CTOR must be executed on the main-thread). nsIPrincipal is not thread-safe at all. It's easier to make them thread-safe if there is not an interface that 'links' the 2 objects.

Here my idea: it seems that you don't really care about the principal if you can have the origin as nsIURI. You need to extra part of the mailbox: URL and consider it as 'origin'. What about if you introduce something like:

interface nsIURIWithSpecialOrigin {
  readonly nsIURI origin;
}

This should not require a huge refactoring of how thunderbird deals with mailbox: URLs. Could it work?
I'm online on IRC if you want to discuss this issue more.

Comment 29

[Jorg K (CEST = GMT+2)]

Thank you for the detailed answer.

(In reply to Andrea Marchesini [:baku] from comment #28)
> You should probably restore these too:
I restored what made our test pass ;-)

> Here my idea: it seems that you don't really care about the principal if you
> can have the origin as nsIURI. You need to extra part of the mailbox: URL
> and consider it as 'origin'. ... Could it work?
Looks doable. I'll look into it during the "night shift" ;-)

> I'm online on IRC if you want to discuss this issue more.
Let me explore your idea.

Comment 30

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-nsIURIWithSpecialOrigin.patch - WIP (v1)

Something like this? Should I still add the bits in NullPrincipal.cpp and nsNetUtil.cpp you poineted out in comment #28.

Comment 31

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-use-nsIURIWithSpecialOrigin.patch - WIP - C-C part (v1)

This is the C-C part. Compiles and works ;-) We should rename GetPrincipalSpec() to GetNormalizedSpec(), but that's another story.

Comment 32

[Andrea Marchesini [:baku]]

Comment on attachment 8995912 [details] [diff] [review]
1478441-nsIURIWithSpecialOrigin.patch - WIP (v1)

Review of attachment 8995912 [details] [diff] [review]:
-----------------------------------------------------------------

Yes. Thanks.

Comment 33

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+2) from comment #30)
> Should I still add the bits in NullPrincipal.cpp and nsNetUtil.cpp you pointed out in comment #28?

Comment 34

[Andrea Marchesini [:baku]]

(In reply to Jorg K (GMT+2) from comment #33)
> (In reply to Jorg K (GMT+2) from comment #30)
> > Should I still add the bits in NullPrincipal.cpp and nsNetUtil.cpp you pointed out in comment #28?

Yes. that is needed. In particular nsNetUtil.cpp where we check if 2 URLs can load each other.

Comment 35

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-use-nsIURIWithSpecialOrigin.patch - C-C part (v2)

This is the C-C part. It needs to be applied on to of a backout of the "emergency measures" landed as revision 7ee9f9040488. I did it this way so the transition from nsIURIWithPrincipal with nsIURIWithSpecialOrigin is clearly visible.

Comment 36

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-nsIURIWithSpecialOrigin.patch - M-C part (v2)

OK, I've added the parts in NullPrincipal.cpp and nsNetUtil.cpp. Hard to tell for me whether I got this right.

Here's a try run with the M-C and C-C patches applied:
https://hg.mozilla.org/try/rev/ae848c8f3284449011653d11d1ce2b3887c15ecb (M-C)
used by
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=5bb85f23de2234a175ae7860ea4609a4fa1e33d8
(The trick is in modifying .taskcluster.yml to point to that revision on M-C try.)

Comment 37

[Andrea Marchesini [:baku]]

Comment on attachment 8995961 [details] [diff] [review]
1478441-nsIURIWithSpecialOrigin.patch - M-C part (v2)

Review of attachment 8995961 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/base/nsIURIWithSpecialOrigin.idl
@@ +17,5 @@
> +     * Special origin.
> +     */
> +    readonly attribute nsIURI origin;
> +};
> +

no extra line here.

Comment 38

[Andrea Marchesini [:baku]]

Comment on attachment 8995948 [details] [diff] [review]
1478441-use-nsIURIWithSpecialOrigin.patch - C-C part (v2)

Review of attachment 8995948 [details] [diff] [review]:
-----------------------------------------------------------------

good. Thanks!

Comment 39

[Andrea Marchesini [:baku]]

Comment on attachment 8995961 [details] [diff] [review]
1478441-nsIURIWithSpecialOrigin.patch - M-C part (v2)

Review of attachment 8995961 [details] [diff] [review]:
-----------------------------------------------------------------

You want to change nsContentUtil::GetUTFOrigin (2 methods). Separate patch?

Comment 40

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-nsIURIWithSpecialOrigin.patch - M-C part (v3)

I've modified
  nsContentUtils::GetUTFOrigin(nsIURI* aURI, nsAString& aOrigin).

The new IDL file accidentally had Windows CRLF line endings in the previous patch, so the compile totally failed. Here we go again:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=d3b50ffce01cf87d9630ece34974522951b2aed9

Comment 41

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-nsIURIWithSpecialOrigin.patch - M-C part (v3b)

Moved variable down a bit, sorry about the noise.

Comment 42

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-nsIURIWithSpecialOrigin.patch - M-C part (v3c)

More noise ;-( - I've been thinking whether those new hunks should do some error checking, but since most of the functions to which I'm adding don't return a status, checking the origin URI or checking the status is the same.

In nsContentUtils::GetUTFOrigin() returns a result, so I'm checking the status there.

Comment 43

[Jorg K (CEST = GMT+2)]

So this is the new unreviewed bit that wasn't in (v2) which had r+:
https://bugzilla.mozilla.org/attachment.cgi?oldid=8995961&action=interdiff&newid=8996035&headers=1

The try run is "green" BTW, so this isn't doing any damage.

Comment 44

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-use-nsIURIWithSpecialOrigin.patch - C-C part (v2b)

Changed "normalised" (British English) to "normalized" (US English) for consistency with the function name.

Comment 45

[Andrea Marchesini [:baku]]

Comment on attachment 8996035 [details] [diff] [review]
1478441-nsIURIWithSpecialOrigin.patch - M-C part (v3c)

Review of attachment 8996035 [details] [diff] [review]:
-----------------------------------------------------------------

::: caps/NullPrincipal.cpp
@@ +194,5 @@
> +      RefPtr<BasePrincipal> principal = BasePrincipal::CreateCodebasePrincipal(origin, attrs);
> +      return principal == this;
> +    }
> +  }
> +#endif

Actually, this block is wrong, because you are creating a new principal and that is different than 'this' for sure.
Remove this block. In general, you don't have nullprincipal for mailbox: URL, right?

::: dom/base/nsContentUtils.cpp
@@ +6350,5 @@
> +  // Check if either URI has a special origin.
> +  nsCOMPtr<nsIURIWithSpecialOrigin> uriWithSpecialOrigin = do_QueryInterface(aURI);
> +  if (uriWithSpecialOrigin) {
> +    nsCOMPtr<nsIURI> origin;
> +    nsresult rv2 = uriWithSpecialOrigin->GetOrigin(getter_AddRefs(origin));

why rv2? maybe add: nsresult rv; at the beginning of the method and use rv here and at line 6362.

::: netwerk/base/nsNetUtil.cpp
@@ +2487,5 @@
> +    // Check if either URI has a special origin.
> +    nsCOMPtr<nsIURI> origin;
> +    nsCOMPtr<nsIURIWithSpecialOrigin> uriWithSpecialOrigin = do_QueryInterface(sourceBaseURI);
> +    if (uriWithSpecialOrigin) {
> +      uriWithSpecialOrigin->GetOrigin(getter_AddRefs(origin));

nsresult rv = ...
if (NS_WARN_IF(NS_FAILED(rv))) {
  return false;
}

MOZ_ASSERT(origin);

@@ +2488,5 @@
> +    nsCOMPtr<nsIURI> origin;
> +    nsCOMPtr<nsIURIWithSpecialOrigin> uriWithSpecialOrigin = do_QueryInterface(sourceBaseURI);
> +    if (uriWithSpecialOrigin) {
> +      uriWithSpecialOrigin->GetOrigin(getter_AddRefs(origin));
> +      if (origin)

{}

@@ +2493,5 @@
> +        sourceBaseURI = origin;
> +    }
> +    uriWithSpecialOrigin = do_QueryInterface(targetBaseURI);
> +    if (uriWithSpecialOrigin) {
> +      uriWithSpecialOrigin->GetOrigin(getter_AddRefs(origin));

same here.

Comment 46

[Jorg K (CEST = GMT+2)]

Thanks. Following the same scheme, shouldn't the code in ContentPrincipal.cpp be like this for consistency?

+#if defined(MOZ_THUNDERBIRD) || defined(MOZ_SUITE)
+  nsCOMPtr<nsIURIWithSpecialOrigin> uriWithSpecialOrigin = do_QueryInterface(aURI);
+  if (uriWithSpecialOrigin) {
+    nsCOMPtr<nsIURI> origin;
+    rv = uriWithSpecialOrigin->GetOrigin(getter_AddRefs(origin));
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return false;
+    }
+    MOZ_ASSERT(origin);
+    OriginAttributes attrs;
+    RefPtr<BasePrincipal> principal = BasePrincipal::CreateCodebasePrincipal(origin, attrs);
+    return nsIPrincipal::Subsumes(principal);
+  }
+#endif

And in BasePrincipal.cpp:

+#if defined(MOZ_THUNDERBIRD) || defined(MOZ_SUITE)
+  nsCOMPtr<nsIURIWithSpecialOrigin> uriWithSpecialOrigin = do_QueryInterface(aURI);
+  if (uriWithSpecialOrigin) {
+    nsCOMPtr<nsIURI> origin;
+    rv = uriWithSpecialOrigin->GetOrigin(getter_AddRefs(origin));
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return nullptr;
+    }
+    MOZ_ASSERT(origin);
+    OriginAttributes attrs;
+    RefPtr<BasePrincipal> principal = CreateCodebasePrincipal(origin, attrs);
+    return principal.forget();
+  }
+#endif

Not ideal to return a nullptr there, but there's a NS_ENSURE_SUCCESS(rv, nullptr); later in the code.

Thanks for your patience!

Comment 47

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-nsIURIWithSpecialOrigin.patch - M-C part (v4)

Addressed review issues from comment #45. Carrying forward :baku's r+.

Comment 48

[Jorg K (CEST = GMT+2)]

Attachment: 1478441-nsIURIWithSpecialOrigin.patch - M-C part (v5)

Here's a version which changes the error handling as per my comment #46.

Andreas, please do an interdiff to v4 and then pick the one you prefer.

Comment 49

[Jorg K (CEST = GMT+2)]

Sorry, Andrea, no "s".

Comment 50

[Andrea Marchesini [:baku]]

Comment on attachment 8996403 [details] [diff] [review]
1478441-nsIURIWithSpecialOrigin.patch - M-C part (v5)

Review of attachment 8996403 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 51

[Jorg K (CEST = GMT+2)]

Thank you, Andrea!

Making sure it compiles for FF not get into trouble with the sheriffs ;-)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=34381248c3b1d1e724425365e814a00e0ade71fc

And a last try run for TB:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f184b489c1d53540492de170730cba65ec8d6bfa

Comment 52

[Jorg K (CEST = GMT+2)]

Dear Sheriff,
please check in "1478441-nsIURIWithSpecialOrigin.patch - M-C part (v5)". I'll handle the C-C part myself.
Thanks you.

P.S.: I had to repeat the try for TB, successful here:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=a2f531fb566c374f96fd7182a79f6385b610d14b

Comment 53

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/01533e166085
Introduce nsIURIWithSpecialOrigin needed for Thunderbird. r=baku

Comment 54

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/01533e166085

Comment 55

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/071b02e0acfd
Backed out changeset 37be8758d57e to re-enable tests. a=backout
https://hg.mozilla.org/comm-central/rev/d5c25d3deed9
Backed out emergency measures from changeset 7ee9f9040488. a=backout
https://hg.mozilla.org/comm-central/rev/1c28beaa36fb
Replace nsIURIWithPrincipal with nsIURIWithSpecialOrigin. r=baku