Allow extensions to add exposed protocols in nsMsgContentPolicy

RESOLVED FIXED in Thunderbird 43.0

Status

MailNews Core
Security
--
enhancement
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: rkent, Assigned: jingnan.si)

Tracking

Trunk
Thunderbird 43.0
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 7 obsolete attachments)

(Reporter)

Description

5 years ago
In Exchange Web Services, embedded cid: images are actually stored as external attachments to the message that must be downloaded separately from the body.

For normal attachments, I handle that with a custom exquilla protocol that has a url looking something like this:

exquilla://YourFolderUri?part=1.2&number=574&filename=abcd.jpg

This all works just fine for normal attachments. But for embedded attachments, the html document has a cid: reference. Following libmime, I rewrite the spec of that embedded item to use the correct URL above.

Unfortunately the docshell refuses to load that image. It is being blocked by nsMsgContentPolicy. To pass, it must either be in the exposed protocol list in nsMsgContentPolicy::IsExposedProtocol, or be one of the delineated exceptions in nsMsgContentPolicy::ShouldBlockUnexposedProtocol

Since these are hard wired to include only the standard message protocols (imap, pop3, etc.) then the content policy fails, and my embedded image fails to load.

What I would like to do is to add a call method to nsMsgContentPolicy allowing me to add my protocol to the list of exposed protocols so that it can be a first-class protocol along with imap and pop3..
(Assignee)

Comment 1

2 years ago
Created attachment 8621430 [details] [diff] [review]
patch nsMsgContentPolicy and make it accept more scheme

add a new interface nsIMsgContentPolicy to provide functions which make extension developer easily to show embedded stuff using their own scheme

Comment 2

2 years ago
Hi, please see https://developer.mozilla.org/en/docs/Getting_your_patch_in_the_tree#Getting_the_patch_reviewed
In this case rkent is probably a good reviewer for this.
Also note that patches need to against the trunk version, not 31.

Updated

2 years ago
Assignee: nobody → jingnan.si

Comment 3

2 years ago
Comment on attachment 8621430 [details] [diff] [review]
patch nsMsgContentPolicy and make it accept more scheme

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

some minor nits

::: comm-esr31/mailnews/base/public/nsIMsgContentPolicy.idl
@@ +1,5 @@
> +#include "nsISupports.idl"
> +
> +/**
> + * This interface provide functions which help extension developers
> + * add their customized schema to the exposed protocls of nsMsgContentPolicy

missing period at end of sentence

::: comm-esr31/mailnews/base/src/nsMsgContentPolicy.cpp
@@ +43,4 @@
>  NS_IMPL_ISUPPORTS(nsMsgContentPolicy, 
>                     nsIContentPolicy,
>                     nsIWebProgressListener,
> +                  nsIMsgContentPolicy,

please align (well, the two other too in this case as they are misaligned)

@@ +391,4 @@
>        MsgLowerCaseEqualsLiteral(contentScheme, "about"))
>      return true;
>  
> +  //check if customized expose schema

add space after //

@@ +393,5 @@
>  
> +  //check if customized expose schema
> +  if (mCustomizedExportProtocolScheme.Contains(contentScheme))
> +    return true;
> +  

remove trailing whitespace

::: comm-esr31/mailnews/base/src/nsMsgContentPolicy.h
@@ +55,1 @@
>    

remove trailing whitespace
(Assignee)

Comment 4

2 years ago
Created attachment 8621937 [details] [diff] [review]
nsMsgContentPolicy.patch update based on trunk

update the patch based on comments and using trunk
Attachment #8621430 - Attachment is obsolete: true

Comment 5

2 years ago
Don't forget to ask for review, or the patch risks going unnoticed. (See the link for how.)
(Assignee)

Updated

2 years ago
Attachment #8621937 - Flags: review?(rkent)
(Assignee)

Comment 6

2 years ago
Created attachment 8624577 [details] [diff] [review]
nsMsgContentPolicy.patch

update patch, previous one missing a file.
Attachment #8621937 - Attachment is obsolete: true
Attachment #8621937 - Flags: review?(rkent)
Attachment #8624577 - Flags: review?(rkent)
(Reporter)

Comment 7

2 years ago
Comment on attachment 8624577 [details] [diff] [review]
nsMsgContentPolicy.patch

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

Sorry for the slow review. I have been completely tied up getting Thunderbird 38 ready for use.

We may also need a second reviewer at some point because this has some security sensitivity. I think this is the correct thing to do (hence the feedback+) but I will need a second opinion at some point.

::: mailnews/base/public/nsIMsgContentPolicy.idl
@@ +1,1 @@
> +#include "nsISupports.idl"

You need a standard license and mode preamble to this file.

@@ +5,5 @@
> + * add their customized schema to the exposed protocls of nsMsgContentPolicy.
> + */
> +[scriptable, uuid(c29b2fd3-64d0-4083-a096-c20a9b847a99)]
> +interface nsIMsgContentPolicy : nsISupports {
> +    void addExposedProtocolScheme(in ACString scheme);

Any new idl requires detailed documentation. See nsIMsgPluggableStore as an example of a newer idl that is properly documented.

I think that you should shorten this (and the remove) to just addExposedProtocol/removeExposedProtocol

::: mailnews/base/src/nsMsgContentPolicy.cpp
@@ +41,3 @@
>  #define kAllowRemoteContent 2
>  
>  NS_IMPL_ISUPPORTS(nsMsgContentPolicy, 

Nit: can you remove the trailing space in this line, even though it was there already?

@@ +905,5 @@
>  {
>    return NS_OK;
>  }
> +
> +/** 

Nit: trailing space on this line and the next.

::: mailnews/base/src/nsMsgContentPolicy.h
@@ +84,5 @@
>                                         nsIURI **aURI);
>    nsresult SetDisableItemsOnMailNewsUrlDocshells(nsIURI *aContentLocation,
>                                                   nsISupports *aRequestingContext);
> +
> +  nsTHashtable<nsCStringHashKey> mCustomizedExportProtocolScheme;

I don't think that a hash is the correct data structure here. This is a table that will typically have zero or one entry. Just use a simple nsTArray<ncCString> here instead. This will affect your other code as well of course, since add and lookup will change.

Also, the name of this seems excessively long to me. How about just mCustomExposedProtocols?
Attachment #8624577 - Flags: review?(rkent)
Attachment #8624577 - Flags: review-
Attachment #8624577 - Flags: feedback+
(Assignee)

Comment 8

2 years ago
Thanks, I update the names and document on idl about. 
One thing about the nsTHashtable, I follow the description about hashtable on https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Guide/Hashtables and I think it is fast for look up if we could use a hash table.
Is there any problem on the nsTHashtable implementation which make the hash table can only handle 0 or 1 entries?

-Jingnan
(Assignee)

Comment 9

2 years ago
Created attachment 8630872 [details] [diff] [review]
nsMsgContentPolicy_2.patch
Attachment #8624577 - Attachment is obsolete: true
Attachment #8630872 - Flags: review?(rkent)
(Assignee)

Comment 10

2 years ago
(In reply to Kent James (:rkent) from comment #7)
> Comment on attachment 8624577 [details] [diff] [review]
> nsMsgContentPolicy.patch
> 
> Review of attachment 8624577 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Sorry for the slow review. I have been completely tied up getting
> Thunderbird 38 ready for use.
> 
> We may also need a second reviewer at some point because this has some
> security sensitivity. I think this is the correct thing to do (hence the
> feedback+) but I will need a second opinion at some point.
> 
> ::: mailnews/base/public/nsIMsgContentPolicy.idl
> @@ +1,1 @@
> > +#include "nsISupports.idl"
> 
> You need a standard license and mode preamble to this file.
> 
> @@ +5,5 @@
> > + * add their customized schema to the exposed protocls of nsMsgContentPolicy.
> > + */
> > +[scriptable, uuid(c29b2fd3-64d0-4083-a096-c20a9b847a99)]
> > +interface nsIMsgContentPolicy : nsISupports {
> > +    void addExposedProtocolScheme(in ACString scheme);
> 
> Any new idl requires detailed documentation. See nsIMsgPluggableStore as an
> example of a newer idl that is properly documented.
> 
> I think that you should shorten this (and the remove) to just
> addExposedProtocol/removeExposedProtocol
> 
> ::: mailnews/base/src/nsMsgContentPolicy.cpp
> @@ +41,3 @@
> >  #define kAllowRemoteContent 2
> >  
> >  NS_IMPL_ISUPPORTS(nsMsgContentPolicy, 
> 
> Nit: can you remove the trailing space in this line, even though it was
> there already?
> 
> @@ +905,5 @@
> >  {
> >    return NS_OK;
> >  }
> > +
> > +/** 
> 
> Nit: trailing space on this line and the next.
> 
> ::: mailnews/base/src/nsMsgContentPolicy.h
> @@ +84,5 @@
> >                                         nsIURI **aURI);
> >    nsresult SetDisableItemsOnMailNewsUrlDocshells(nsIURI *aContentLocation,
> >                                                   nsISupports *aRequestingContext);
> > +
> > +  nsTHashtable<nsCStringHashKey> mCustomizedExportProtocolScheme;
> 
> I don't think that a hash is the correct data structure here. This is a
> table that will typically have zero or one entry. Just use a simple
> nsTArray<ncCString> here instead. This will affect your other code as well
> of course, since add and lookup will change.
> 
> Also, the name of this seems excessively long to me. How about just
> mCustomExposedProtocols?


Thanks, I update the names and document on idl about. 
One thing about the nsTHashtable, I follow the description about hashtable on https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Guide/Hashtables and I think it is fast for look up if we could use a hash table.
Is there any problem on the nsTHashtable implementation which make the hash table can only handle 0 or 1 entries?

-Jingnan
(Assignee)

Comment 11

2 years ago
Created attachment 8630873 [details] [diff] [review]
nsMsgContentPolicy_2.patch

add missing idl file to diff
Attachment #8630872 - Attachment is obsolete: true
Attachment #8630872 - Flags: review?(rkent)
Attachment #8630873 - Flags: review?(rkent)
(Reporter)

Comment 12

2 years ago
Comment on attachment 8630873 [details] [diff] [review]
nsMsgContentPolicy_2.patch

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

Sprry for the slow review, we were a bit tired from the 38 release so got a little behind.

Overall I am content to do this, it seems necessary to support alternate messaging protocols which we want to do. Most of the comments are minor.

You really should an an xpcshell test for this. It will be quite trivial, just create a content object, then show that it rejects a new protocol, add it, show it accepts, remove it, show it rejects again.

::: mailnews/base/public/nsIMsgContentPolicy.idl
@@ +8,5 @@
> +[scriptable, uuid(c29b2fd3-64d0-4083-a096-c20a9b847a99)]
> +
> +/**
> + * This interface provide functions which help extension developers
> + * add their customized schema to the exposed protocls of nsMsgContentPolicy.

nit: protocols not protocls

Also, this description does not really help anyone new to this understand what this does, and how it is used. Let me suggest some additional text:

By default, a list of existing protocols (such as imap and nntp) are allowed to process urls locally, while non-matching urls are required to be processed as external. This interface allows additional protocols to be added to the list of protocols that are processed locally. Typically this would be used in cases where a new messaging protocol is being added by an extension.

@@ +12,5 @@
> + * add their customized schema to the exposed protocls of nsMsgContentPolicy.
> + */
> +interface nsIMsgContentPolicy : nsISupports {
> +	/**
> +	 * add the specific aScheme to nsMsgContentPolicy's exposed protools

nit: protocols not protools

@@ +14,5 @@
> +interface nsIMsgContentPolicy : nsISupports {
> +	/**
> +	 * add the specific aScheme to nsMsgContentPolicy's exposed protools
> +	 *
> +	 * @param aScheme scheme who will be added to nsMsgContentPolicy's exposed protocols 

Nit: trailing space

@@ +17,5 @@
> +	 *
> +	 * @param aScheme scheme who will be added to nsMsgContentPolicy's exposed protocols 
> +	 */
> +	void addExposedProtocol(in ACString aScheme);
> +	

Nit: trailing space

@@ +19,5 @@
> +	 */
> +	void addExposedProtocol(in ACString aScheme);
> +	
> +	/**
> +	 * remove the specific aScheme from nsMsgContentPolicy's exposed protools

Not: protocols not protools

@@ +21,5 @@
> +	
> +	/**
> +	 * remove the specific aScheme from nsMsgContentPolicy's exposed protools
> +	 *
> +	 * @param aScheme scheme who will be removed from nsMsgContentPolicy's exposed protocols 

Nit: trailing space

@@ +23,5 @@
> +	 * remove the specific aScheme from nsMsgContentPolicy's exposed protools
> +	 *
> +	 * @param aScheme scheme who will be removed from nsMsgContentPolicy's exposed protocols 
> +	 */
> +	void removeExposedProtocol(in ACString scheme);

Nit: aScheme not scheme

::: mailnews/base/src/nsMsgContentPolicy.cpp
@@ +391,4 @@
>        MsgLowerCaseEqualsLiteral(contentScheme, "about"))
>      return true;
>  
> +  // check if customized expose scheme

Nit: exposed not expose

@@ +906,5 @@
> + * Implementation of nsIMsgContentPolicy
> + *
> + */
> +NS_IMETHODIMP
> +nsMsgContentPolicy::AddExposedProtocol(const nsACString & scheme)

Nit: const nsACString &scheme (that is, with no space before the variable name)

Also convention is to precede parameters with "a" so aScheme not scheme

(also in RemoveExposedProtocol)

::: mailnews/base/src/nsMsgContentPolicy.h
@@ +84,5 @@
>                                         nsIURI **aURI);
>    nsresult SetDisableItemsOnMailNewsUrlDocshells(nsIURI *aContentLocation,
>                                                   nsISupports *aRequestingContext);
> +
> +  nsTHashtable<nsCStringHashKey>  mCustomExposedProtocols;

This list of exposed protocols will usually have a length of zero, and even when used only a length of a handful of items. A hashtable is overkill for this. Just use a string array (created from nsTArray).

This will of course affect the rest of your code as well, as the way that you add, subtract, and search elements will be different.
Attachment #8630873 - Flags: review?(rkent) → feedback+
(Assignee)

Comment 13

2 years ago
Created attachment 8642218 [details] [diff] [review]
nsMsgContentPolicy_20150802.patch

update the description and change to nsTArray.
Add a unit test
Attachment #8630873 - Attachment is obsolete: true
Attachment #8642218 - Flags: review?(rkent)
(Reporter)

Comment 14

2 years ago
Comment on attachment 8642218 [details] [diff] [review]
nsMsgContentPolicy_20150802.patch

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

In spite of all of my comments, this is really very close to ready. Most are nits.

Also sorry I am so slow with reviews. We are still dealing with critical issues in the Thunderbird 38 release, and those issues have had priority. Hopefully we will be past that soon, and can get back to a more normal review schedule.

::: mailnews/base/public/nsIMsgContentPolicy.idl
@@ +9,5 @@
> +
> +/**
> + * This interface provide functions which help extension developers
> + * add their customized schema to the exposed protocls of nsMsgContentPolicy.
> + * By default, a list of existing protocols (such as imap and nntp) are allowed to process urls locally, 

nit: trailing space. Also, these lines should be limited to 80 characters. That is actually supposed to be the official standard in all code, but as you can see that rule is frequently broken in older code.

@@ +10,5 @@
> +/**
> + * This interface provide functions which help extension developers
> + * add their customized schema to the exposed protocls of nsMsgContentPolicy.
> + * By default, a list of existing protocols (such as imap and nntp) are allowed to process urls locally, 
> + * while non-matching urls are required to be processed as external. 

nit: trailing space

@@ +15,5 @@
> + * This interface allows additional protocols to be added to the list of protocols that are processed locally.
> + * Typically this would be used in cases where a new messaging protocol is being added by an extension.
> + */
> +interface nsIMsgContentPolicy : nsISupports {
> +	/**

Nit: indent all of this only two spaces.

@@ +16,5 @@
> + * Typically this would be used in cases where a new messaging protocol is being added by an extension.
> + */
> +interface nsIMsgContentPolicy : nsISupports {
> +	/**
> +	 * add the specific aScheme to nsMsgContentPolicy's exposed protocols

Nit: treat this as a sentence (start with capital letter, end with period)

@@ +21,5 @@
> +	 *
> +	 * @param aScheme scheme who will be added to nsMsgContentPolicy's exposed protocols
> +	 */
> +	void addExposedProtocol(in ACString aScheme);
> +	

Nit: extra space

@@ +23,5 @@
> +	 */
> +	void addExposedProtocol(in ACString aScheme);
> +	
> +	/**
> +	 * remove the specific aScheme from nsMsgContentPolicy's exposed protocols

Nit: treat this as a sentence (start with capital letter, end with period)

::: mailnews/base/src/nsMsgContentPolicy.cpp
@@ +391,5 @@
>        MsgLowerCaseEqualsLiteral(contentScheme, "about"))
>      return true;
>  
> +  // check if customized exposed scheme
> +  PRUint32 len = mCustomExposedProtocols.Length();

PRUint32 is now obsolete. use uint32_t instead (here and other places as well).

@@ +392,5 @@
>      return true;
>  
> +  // check if customized exposed scheme
> +  PRUint32 len = mCustomExposedProtocols.Length();
> +  

Nit: trailing space.

@@ +393,5 @@
>  
> +  // check if customized exposed scheme
> +  PRUint32 len = mCustomExposedProtocols.Length();
> +  
> +  for (PRUint32 i = 0;i < len; i++) {

nit: space after 0; so this should be:
  for (uint32_t i = 0; i < len; i++) {

Other issues:

You really don't need the temp len variable, just use mCustomExposedProtocols.Length() directly in the for statement.

Also, TArray claims that it has a Contains method that accepts a comparator. I have not tested this, and I could not find any existing example of this, but you probably can just do simply:

if (mCustomExposedProtocols.Contains(contentScheme, nsCaseInsensitiveCStringComparator())
  return true;

(Here and elsewhere where you have this loop).

@@ +918,5 @@
> +NS_IMETHODIMP
> +nsMsgContentPolicy::AddExposedProtocol(const nsACString &aScheme)
> +{
> +  PRUint32 len = mCustomExposedProtocols.Length();
> +  

Nit: trailing space

@@ +919,5 @@
> +nsMsgContentPolicy::AddExposedProtocol(const nsACString &aScheme)
> +{
> +  PRUint32 len = mCustomExposedProtocols.Length();
> +  
> +  for (PRUint32 i = 0;i < len; i++) {

Nit: as before, space after 0

@@ +934,5 @@
> +NS_IMETHODIMP
> +nsMsgContentPolicy::RemoveExposedProtocol(const nsACString &aScheme)
> +{
> +  PRUint32 len = mCustomExposedProtocols.Length();
> +  

Nit: trailing space.

@@ +935,5 @@
> +nsMsgContentPolicy::RemoveExposedProtocol(const nsACString &aScheme)
> +{
> +  PRUint32 len = mCustomExposedProtocols.Length();
> +  
> +  for (PRUint32 i = 0;i < len; i++) {

(same issue as before)

::: mailnews/base/test/unit/test_nsIMsgContentPolicy.js
@@ +10,5 @@
> +  return ioService.newURI(aURL, null, null);
> +}
> +
> +function run_test() {
> +	var content_policy = Cc["@mozilla.org/messenger/content-policy;1"]

Nit: all of the code in this test should only be indented 2 spaces in this function.

@@ +21,5 @@
> +	Assert.ok(msg_content_policy);
> +
> +	var req_uri = makeURI("custom-scheme://custom_url/1.emal");
> +	Assert.ok(req_uri);
> +	

Nit: trailing space.

@@ +34,5 @@
> +	                                         null);
> +	Assert.notEqual(decision,
> +	                Ci.nsIContentPolicy.ACCEPT,
> +	                "customized protocol should not load");
> +	

Nit: trailing space

@@ +36,5 @@
> +	                Ci.nsIContentPolicy.ACCEPT,
> +	                "customized protocol should not load");
> +	
> +	msg_content_policy.addExposedProtocol("custom-scheme");
> +	

Nit: trailing space

@@ +38,5 @@
> +	
> +	msg_content_policy.addExposedProtocol("custom-scheme");
> +	
> +	decision = content_policy.shouldLoad(Ci.nsIContentPolicy.TYPE_IMAGE,
> +	                                         content_uri,

Typically if a parameter list gets split over multiple lines like this, it is indented to match the first parameter. So content_url should be directly under Ci.nsIContent... as in your previous two calls. (Same issue below as well in multiple places).

@@ +46,5 @@
> +	                                         null);
> +	Assert.equal(decision,
> +	                Ci.nsIContentPolicy.ACCEPT,
> +	                "customized protocol should load");
> +	

Nit: trailing space

@@ +48,5 @@
> +	                Ci.nsIContentPolicy.ACCEPT,
> +	                "customized protocol should load");
> +	
> +	msg_content_policy.removeExposedProtocol("custom-scheme");
> +	

Nit: trailing space

::: mailnews/base/test/unit/xpcshell.ini
@@ +44,1 @@
>  # Not yet working for non-Mac OS

The comments and adjustments to tests in xpcshell.ini are put AFTER the test, so this comment and the skip-if apply to your new test (which is not what you intend). Please place your test after the skip-if
Attachment #8642218 - Flags: review?(rkent) → review-
(Assignee)

Comment 15

2 years ago
Created attachment 8654468 [details] [diff] [review]
nsMsgContentPolicy_20150828.patch

Thank you for your review and sorry for take so long time to response, I fight with a heavy code and tooth pain:)

This patch
1. clean up trailing spaces
2. update the comparing loop to using Contains with comparator, but I have to create a new comparator class since the nsTArray didn't accept the nsCaseInsensitiveCStringComparator, but I found a similar class in nsNntpIncomingServer.cpp, maybe we should prompt that class to a public header.

Jingnan
Attachment #8642218 - Attachment is obsolete: true
Attachment #8654468 - Flags: review?(rkent)
(Reporter)

Comment 16

2 years ago
Comment on attachment 8654468 [details] [diff] [review]
nsMsgContentPolicy_20150828.patch

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

Overall OK, but with a few comments. In the interest of avoiding another review cycle, I'll attach a modified version of your patch with my suggesteds changes and r+. If you are happy with that, set checkin-needed, otherwise modify for re-review.

::: mailnews/base/src/nsMsgContentPolicy.cpp
@@ +31,5 @@
>  static const char kTrustedDomains[] =  "mail.trusteddomains";
>  
>  using namespace mozilla::mailnews;
>  
> +/**

I don't see any reason why this comparison needs to be case insensitive, and in fact I can think of good reasons why it should not be (like so that a protocol is spoofed by using a mixed-case name).

@@ +411,5 @@
>  
> +  // check if customized exposed scheme
> +  nsTArrayCaseInsensitiveCStringComparator comparator;
> +  if (mCustomExposedProtocols.Contains(contentScheme, comparator))
> +	  return true;

Nit: uses spaces, not tabs.

@@ +933,5 @@
> +nsMsgContentPolicy::AddExposedProtocol(const nsACString &aScheme)
> +{
> +  nsTArrayCaseInsensitiveCStringComparator comparator;
> +
> +  if (mCustomExposedProtocols.Contains(nsCString(aScheme), comparator))

Make simple Contains without using comparator (also in ::Remove ...)

::: mailnews/base/test/unit/test_nsIMsgContentPolicy.js
@@ +5,5 @@
> + */
> +
> +function makeURI(aURL) {
> +  var ioService = Cc["@mozilla.org/network/io-service;1"]
> +                  .getService(Ci.nsIIOService);

Nit: I prefer the . under the [

@@ +11,5 @@
> +}
> +
> +function run_test() {
> +  var content_policy = Cc["@mozilla.org/messenger/content-policy;1"]
> +      .getService(Ci.nsIContentPolicy);

Nit: also . under [ here.
Attachment #8654468 - Flags: review?(rkent) → review-
(Reporter)

Comment 17

2 years ago
Created attachment 8662041 [details] [diff] [review]
With RKJ nits fixed, also now case-sensitive. RKJ r+ of jingnan.si patch
Attachment #8654468 - Attachment is obsolete: true
Attachment #8662041 - Flags: review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed
(Assignee)

Comment 18

2 years ago
Thanks, I am OK with the changes,
by the way, I am quite new to bugzilla, Is "set checkin-needed" what I did by add a "checkin-needed" keyword?

@jingnan
(Reporter)

Comment 19

2 years ago
Right, you did the correct thing.

Comment 20

2 years ago
https://hg.mozilla.org/comm-central/rev/6203f811717fb2c7c2c10eebce4ebb3a466ca4af
Bug 764987 - "Allow extensions to add exposed protocols in nsMsgContentPolicy". r=rkent

Updated

2 years ago
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 43.0
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.