No event/notify when Compose windows is ready

RESOLVED FIXED

Status

Thunderbird
Message Compose Window
RESOLVED FIXED
12 years ago
10 years ago

People

(Reporter: Emil Hesslow, Assigned: Emil Hesslow)

Tracking

({fixed1.8.1})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

12 years ago
From what I can see there is no way to know when the compose window is ready. eComposeFieldsReady is fired when the compose fields are ready but that is before the body of the mail is set up. Or that is at least what I can see here: http://lxr.mozilla.org/mailnews/source/mailnews/compose/src/nsMsgCompose.cpp#1369

This means that it is very hard to make any kind of extension that inserts text to the body automatically without replacing the init-function or polling the editor. Polling the editor is possible but you have to check for a specific number of changes of the editor and it is different if it is a reply, you have a signature or not. And changing the init-function means that you probably will break something. I'm the developer of Quicktext (http://www.hesslow.se/quicktext) and there is many feature I would like to add if I could. Like text automatically got inserted when you reply a mail and stuff like that.

I would be very greatful if this was added and I'm sure everyone that uses my extension also will be.
(Assignee)

Comment 1

12 years ago
Created attachment 216065 [details] [diff] [review]
patch #1

This patch both adds an event so extensions know when gMsgCompose is initialized so they can attach an statelistener. It also adds the ComposeBodyReady notify which is sent when the body of the mail is ready so extension could insert text to the body. It also changes TStateListenerNotification to an interface.

I know that the name of variablename/functionname probably could change. But I didn't come up which anything better.
Assignee: mscott → hesslow
Status: NEW → ASSIGNED
Attachment #216065 - Flags: review?(bienvenu)

Comment 2

12 years ago
I thought we added an event very similar to this for the smime folks several years ago.
(Assignee)

Comment 3

12 years ago
Scott: If you are right it would be great because then I could use it in my extension today. But I have not been able to find anything like this and no one I've asked has been able to tell me how this could be done.

Comment 4

12 years ago
Comment on attachment 216065 [details] [diff] [review]
patch #1

This looks pretty good. Here's one thing to keep in mind, however - I assume you want this to go into 2.0, and for 2.0, we're trying really hard not to change any interfaces, so changing
notifyStateListeners is not great for 2.0

also, this seems to be a bit strange:

+interface nsIMsgComposeNotificationType : nsISupports


should it just be

+interface nsIMsgComposeNotificationType

? Or does that mean you can't pass it into notifyStateListeners? I don't know what declaring it as an nsISupports does when it gets passed in. You could also just define the consts in nsIMsgCompose instead of having a separate type...

thoughts? For 2.0, the less interface changes the better.

Comment 5

12 years ago
Scott, this would require adding a method to nsIMsgComposeStateListener to get called when the compose window is ready for the user, e.g., the reply quoting is done and the sig has been inserted. Do we think that's OK for 2.0? Or is there some other way of knowing this, e.g., some UI element gets enabled?
(Assignee)

Comment 6

12 years ago
Created attachment 219413 [details] [diff] [review]
patch #2

This just removes nsISupports that David taked about. If you don't want me to change the enum to its own interface for 2.0 because it also changes nsIMsgCompose interface just tell me and I fix a patch without that change.
Attachment #216065 - Attachment is obsolete: true
Attachment #219413 - Flags: review?(bienvenu)
Attachment #216065 - Flags: review?(bienvenu)

Comment 7

12 years ago
Comment on attachment 219413 [details] [diff] [review]
patch #2

looks good for the trunk.
Attachment #219413 - Flags: review?(bienvenu) → review+
(Assignee)

Updated

12 years ago
Attachment #219413 - Flags: superreview?(mscott)
(Assignee)

Comment 8

12 years ago
When you have time to review Scott it would be nice if you could tell me if you want this for 2.0 (I want :)) and if so if you want the change for the nsIMsgCompose-interface or I should remove that for 2.0.

Comment 9

12 years ago
(In reply to comment #3)
> Scott: If you are right it would be great because then I could use it in my
> extension today. But I have not been able to find anything like this and no one
> I've asked has been able to tell me how this could be done.
> 

I must have been thinking of window-compose-reopen which doesn't do what you want.

Comment 10

12 years ago
Comment on attachment 219413 [details] [diff] [review]
patch #2

I think the event name should be compose-window-init to be consistent with the other events we fire from this window:

compose-window-close
compose-window-reopen
compose-window-sendmessage
etc.

In lines like:

+      compose->NotifyStateListeners(nsIMsgComposeNotificationType::ComposeProcessDone,aStatus);
add a space after the comma even though the earlier code didn't have that space, it should have.

Let's put together a separate patch for the branch only which doesn't have the interface change. But we can do that after you make my minor two nits and we land this on the trunk. 

Thanks for working on this and for your patience.
Attachment #219413 - Flags: superreview?(mscott) → superreview+
(Assignee)

Comment 11

12 years ago
Created attachment 222477 [details] [diff] [review]
patch #3

This fixes the small comments from Scott. So if someone could check this in that would be good.

Comment 12

12 years ago
fix checked into the trunk, marking fixed. Thx, Emil!
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Assignee)

Comment 13

12 years ago
Created attachment 223360 [details] [diff] [review]
patch #1 for TB 2.0

This is the same patch as the last one except that I removed the change in nsIMsgCompose.
Attachment #223360 - Flags: review?(bienvenu)
Attachment #223360 - Flags: approval-branch-1.8.1?(bienvenu)

Comment 14

12 years ago
Comment on attachment 223360 [details] [diff] [review]
patch #1 for TB 2.0

I'll apply and run with this change in my 1.8.1 branch build before checking in.
Attachment #223360 - Flags: review?(bienvenu)
Attachment #223360 - Flags: review+
Attachment #223360 - Flags: approval-branch-1.8.1?(bienvenu)
Attachment #223360 - Flags: approval-branch-1.8.1+

Comment 15

12 years ago
Hey David, I can land this on the branch if you think it's ready.

Comment 16

11 years ago
landed on the 1.8.1 branch - thx for the reminder, Emil.
Keywords: fixed1.8.1

Updated

11 years ago
Blocks: 360488

Updated

10 years ago
No longer blocks: 360488
Depends on: 341312
You need to log in before you can comment on or make changes to this bug.