321890 - Enhancements to GenericSendMessage

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, enhancement)

Description

[Patrick Brunschwig]

GenericSendMessage as it is now dispatches an event for preprocessing of messages just before the message is sent. While this is certainly a good step into a pluggable sending architecture, at least for Enigmail, two things are missing for the event handler:

1. the event handler needs to know the value of msgType (e.g. make it a global variable or add it to gMsgCompose.compFields)

2. the event handler should be able to cancel message sending e.g. in case something goes wrong or in case the user aborts sending. This may just be my ignorance, but I couldn't find a way how to abort message sending -- not even throwing an exception stopped the operation.

Comment 2

[Patrick Brunschwig]

Attachment: patch adding msgType and cancelSendMessage to the event

I have created a very minimal patch that adds msgType and cancelSendMessage directly as variables to the event. I don't know if that's according to the usual programming style; otherwise I'd be glad to accept any input for how to do it in a better way.

In order to cancel sending the message in this solution from within the event handling, you' have to do:
 event.stopPropagation();
 event.cancelSendMessage=true;

Comment 3

[David :Bienvenu]

Comment on attachment 275655 [details] [diff] [review]
patch adding msgType and cancelSendMessage to the event

I don't know if there's any prior art here, but it seems OK to me. Requesting sr from Scott.

Comment 4

[Scott MacGregor]

Neil might have opinions on whether this is the best approach.

Comment 5

[[e-mail]]

I'm not sure what the best way to expose the message send type is but expando properties on DOM events aren't pinned and could be deleted at any time.

Since the event has been created as cancelable the way to cancel it is to call event.preventDefault() on it or return false from an event listener and make GenericSendMessage use event.getPreventDefault() to see if it was cancelled.

Comment 6

[Patrick Brunschwig]

Attachment: New patch, using UIEvent

Neil, thanks for your input. The main problem regarding event cancellation was that 'Event' was used (which does not provide the getPreventDefault method), instead of 'UIEvent'.

Concerning the msgType, I'm adding it now as attribute to the event target XUL element.

Comment 7

[David :Bienvenu]

Comment on attachment 275940 [details] [diff] [review]
New patch, using UIEvent

looks ok to me, but could you lose the extra braces here:

+        if (event.getPreventDefault()) {
+          throw Components.results.NS_ERROR_ABORT;
+        }

Is it possible that we might want to set the msgtype attribute when the msg window is created, instead of right before it's sent? I wonder if other extensions might care earlier...

Comment 8

[David :Bienvenu]

Comment on attachment 275940 [details] [diff] [review]
New patch, using UIEvent

never mind about setting the msgType earlier - Neil corrected my thinking there.

Comment 9

[[e-mail]]

Comment on attachment 275940 [details] [diff] [review]
New patch, using UIEvent

Looks good to me.

Comment 10

[Scott MacGregor]

Patrick, can you make a patch against CVS for both the thunderbird and seamonkey files? It's a lot easier for contributors with CVS access to do a fly-by checkin if they can easily apply it. Thanks again for the patch...

Comment 11

[Patrick Brunschwig]

Attachment: Patch for Thunderbird and Seamonkey

Here is the patch again, this time for Seamonkey and Thunderbird together.

I found that on Seamonkey the "compose-send-message" event is issued much earlier than on Thunderbird. I couldn't find a reason why, but for the sake of consistency it should be issued at the same time on both products -- especially regarding the comment just above the event creation (on Thunderbird).

Comment 12

[David :Bienvenu]

Comment on attachment 276759 [details] [diff] [review]
Patch for Thunderbird and Seamonkey

so, the TB change is fine, but for SM, you'll want someone like Neil to make sure that moving the call is OK with him.

Comment 13

[[e-mail]]

Comment on attachment 276759 [details] [diff] [review]
Patch for Thunderbird and Seamonkey

>+        document.getElementById("msgcomposeWindow").dispatchEvent(event);
This line shouldn't have been copied. sr=me with it removed.

Comment 14

[Patrick Brunschwig]

Attachment: final patch

Ah, right Neil, I have overlooked that line. Here we are then!

Comment 15

[Phil Ringnalda (:philor)]

mail/components/compose/content/MsgComposeCommands.js 1.116
mailnews/compose/resources/content/MsgComposeCommands.js 1.399