1240823 - Add observer to compose to allow extensions to manipulate outgoing server

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[Kent James (:rkent)]

This is a followup to bug 1240327, fixing the observer notification since accountKey is unreliable (bug 449482 comment 16).

Comment 1

[Kent James (:rkent)]

Attachment: Add an observer and test

Comment 2

[Magnus Melin [:mkmelin]]

Comment on attachment 8709637 [details] [diff] [review]
Add an observer and test

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

::: mailnews/compose/src/nsMsgCompose.cpp
@@ +1079,5 @@
> +
> +    // Allow extensions to specify an outgoing server.
> +    nsCOMPtr<nsIObserverService> observerService =
> +      mozilla::services::GetObserverService();
> +    if (observerService)

shouldn't !observerService just error out?

@@ +1085,5 @@
> +      // Assemble a string with sending parameters.
> +      nsAutoString sendParms;
> +
> +      // First parameter: account key. This is not reliable, see
> +      // bug 449482 comment 16.

per comment 22 that should not be the case anymore though

@@ +1086,5 @@
> +      nsAutoString sendParms;
> +
> +      // First parameter: account key. This is not reliable, see
> +      // bug 449482 comment 16.
> +      sendParms.AppendASCII(accountKey && *accountKey ? accountKey : "none");

should this really be "none"? not just ""?

::: mailnews/compose/test/unit/test_sendObserver.js
@@ +32,5 @@
> +  fields.cc = "Alex <[e-mail]>";
> +  fields.subject = "Let's test the observer";
> +
> +  yield richCreateMessage(fields, [], identity);
> +  // observer data should have: 

nit: trailing space here and a few lines down

Comment 3

[Kent James (:rkent)]

https://hg.mozilla.org/comm-central/rev/fa1e66e01549

One comment on review:

"per comment 22 that should not be the case anymore though"

Yes, but there are other places in the code where SendMsg is called with a null account, so I changed the message to a simple warning that the account might be null.

Comment 4

[Kent James (:rkent)]

Comment on attachment 8709637 [details] [diff] [review]
Add an observer and test

[Triage Comment] This is a feature, giving a hook for extensions, that should be in TB 45. It should not have any effect on existing usage. Please land after an initial successful comm-central build and tests. (Land from the hg rev, not this actual patch).

Comment 5

[Jorg K (CEST = GMT+2)]

Aurora:
https://hg.mozilla.org/releases/comm-aurora/rev/d667a01136d0
Of course landed via "hg transplant"!