Closed
Bug 324574
Opened 19 years ago
Closed 19 years ago
No event/notify when Compose windows is ready
Categories
(Thunderbird :: Message Compose Window, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: hesslow, Assigned: hesslow)
References
Details
(Keywords: fixed1.8.1)
Attachments
(3 files, 1 obsolete file)
(deleted),
patch
|
Bienvenu
:
review+
mscott
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Bienvenu
:
review+
Bienvenu
:
approval-branch-1.8.1+
|
Details | Diff | Splinter Review |
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•19 years ago
|
||
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.
Comment 2•19 years ago
|
||
I thought we added an event very similar to this for the smime folks several years ago.
Assignee | ||
Comment 3•19 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•19 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•19 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•19 years ago
|
||
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•19 years ago
|
||
Comment on attachment 219413 [details] [diff] [review]
patch #2
looks good for the trunk.
Attachment #219413 -
Flags: review?(bienvenu) → review+
Assignee | ||
Updated•19 years ago
|
Attachment #219413 -
Flags: superreview?(mscott)
Assignee | ||
Comment 8•19 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•19 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•19 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•19 years ago
|
||
This fixes the small comments from Scott. So if someone could check this in that would be good.
Comment 12•19 years ago
|
||
fix checked into the trunk, marking fixed. Thx, Emil!
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 13•19 years ago
|
||
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•19 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•18 years ago
|
||
Hey David, I can land this on the branch if you think it's ready.
Comment 16•18 years ago
|
||
landed on the 1.8.1 branch - thx for the reminder, Emil.
Keywords: fixed1.8.1
You need to log in
before you can comment on or make changes to this bug.
Description
•