Closed
Bug 70478
Opened 23 years ago
Closed 22 years ago
The "quote original message" option is missing when creating a reply or new message (restore feature to quote selected messages)
Categories
(SeaMonkey :: MailNews: Message Display, enhancement)
SeaMonkey
MailNews: Message Display
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: johann.petrak, Assigned: zhayupeng)
References
Details
(Keywords: qawanted)
Attachments
(1 file, 7 obsolete files)
17.70 KB,
patch
|
zhayupeng
:
review+
Bienvenu
:
superreview+
|
Details | Diff | Splinter Review |
From Bugzilla Helper: User-Agent: Mozilla/4.5 [en] (X11; I; SunOS 5.6 sun4u) BuildID: 2001022808, 0.8, all Netscape 4.x has an "quote original message" option that allows to paste as a quotation the currently selected message. This can be used to paste several different messages as quotations into one email by activating different messages in the list. This option should be included in Mozilla, as it has many advantages to the "paste as quotation" option currently there (which should be kept): It also pastes the name of the sender. It needs much less steps to paste one or more emails When doing a reply, the correct email is already activated. Note that this is especially important for people who have set the preference NOT to automatically quote the original when replying (e.g. because they often get very big messages) Reproducible: Always Steps to Reproduce: n/a
![]() |
||
Comment 1•23 years ago
|
||
confirming rfe
Status: UNCONFIRMED → NEW
Ever confirmed: true
Hardware: PC → All
Mozilla used to have a quote button for per-message quoting, but it was taken out: See bug 35516. I'd like to see it come back. The way it is now, users basically have to choose between always quoting and never quoting the original message in a reply. The workarounds (using paste as quotation or deleting the quoted text) are painful.
Wow.. total shock that the quote button is missing at this late date! I believe this feature counts as "basic functionality". Nobody would knowingly want to release an email client that is missing this feature. The email app is very nice right now, but the lack of a quote button prevents me from using the darned thing for serious work.
Keywords: nsenterprise
Reporter | ||
Comment 9•23 years ago
|
||
Why is this an enhancement when it is in 4xp? And I agree with mlord: this is basic functionality. It is especially indispensible for anybody who has to answer multiple emails when using email for discussion among more than two people. It shouldnt be too much effort either, since the automatic quote function for new messages can probably reused.
Comment 10•23 years ago
|
||
One additional reason I would like to see this button`s return is for its convenience of taking news stories that I gather from frequented sites, pasting their text into E-Mail. It is a one-button solution for pasting the entire story into an E-Mail message and usually formats it complete with lines wrapped and ``quote''-marks, ready for sending without a lot of intervention in most cases. Without the button, the quoted feature is not utilised (including the word-wrap setting), and highlighting, copying, and pasting becomes a multiple-step process. Just my two-cents. !
Comment 11•22 years ago
|
||
*** Bug 104604 has been marked as a duplicate of this bug. ***
Comment 12•22 years ago
|
||
*** Bug 118746 has been marked as a duplicate of this bug. ***
Comment 13•22 years ago
|
||
I agree with everyone else's comments here - this is very basic functionality that should be in the mail/news portion of Moz. Having the option to always quote or never quote doesn't provide enough flexibility. I've also noticed in recent builds that "Paste as Quotation" is also missing.
Comment 14•22 years ago
|
||
This bug is coming up on its first anniversary, and all of the comments seem to agree that it ought to be fixed. Is there someone on the Mozilla team who thinks this *shouldn't* be fixed, and if so, why ?
Comment 15•22 years ago
|
||
*** Bug 121050 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 16•22 years ago
|
||
I'm working on this problem. We have a function BuildQuoteMessageAndSignature in nsMsgCompose But it is only for auto quote... and it's resule based on pref setting. So, We must implement another method to do the quote. It should be QuoteOriginalMessage(const char *originalMsgURI) This function should get data from original message and quote it to composer.
Comment 17•22 years ago
|
||
4xp bugs are by definition not enhancements. This is important basic functionality that is still missing from Mozilla. Severity should be changed to normal.
Assignee | ||
Comment 18•22 years ago
|
||
I have implemented this function in my tree. I think I can supply a patch later. I implement another nsIStreamListener to get selected message's data. And put them to composer as cited quotation. It works for me now and just same as Netscape 4.x.
Assignee | ||
Comment 19•22 years ago
|
||
Add a quote stream listener to messenger compose module. Tested on my workstation. Please review it, thanks!
Reporter | ||
Comment 20•22 years ago
|
||
Pete I applied the patch to my source tree (Jan 25, 2002) and i *love* it! Just two small remarks: There might be a problem based on the fact that quote message is never deactivated when no message is active/selected (this can happen e.g. by switching folders and not selecting a message). The console shows the following error in that case: JavaScript error: line 0: uncaught exception: [Exception... "Component returned failure code: 0x80070057 (NS_ERROR_ILLEGAL_VALUE) [nsIMsgCompose.QuoteMessage]" nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)" location: "JS frame :: chrome://messenger/content/messengercompose/MsgComposeCommands.js :: QuoteSelectedMessage :: line 608" data: no] I also wonder why the "quote message" menu entry is in the "Options" menu and not in the "Edit" menu? Wouldnt it be nice to have it in the context menu too?
Comment 21•22 years ago
|
||
Should we file a different bug to get a "quote" button on the toolbar (4xp)? Also, does this patch work for any message that you have selected? For instance, in NS4.x, I could compose a message and quote several other messages by selecting them each and hitting the quote button in turn.
Reporter | ||
Comment 22•22 years ago
|
||
Ben, yes I tested this and it works with as many messages from any folder as you want. Wonderful Pete!
Comment 23•22 years ago
|
||
Re: comment 21--I think this behavior could be confusing. How about if I open message A, start composing a reply to message A, open up message B to get someone's e-mail address, and then return to my reply and press the quote button? I want to quote the text from message A, but Mozilla ends up quoting message B instead.
Reporter | ||
Comment 24•22 years ago
|
||
james: definitely not - the whole idea of quote message is to quote whatever message is currently selected and thus be able to respond to several messages in one reply. please see also the earlier comments for some very good reasons why this is good. Concerning an extra button: this would be handy and there is enough space left, but wouldnt it also mean that all themes have to be updated for this?
Assignee | ||
Comment 25•22 years ago
|
||
Thanks all for testing my patch :-) Yes, quote message need do more test and even need some test case I think. I just do the same thing as Netscape 4.x Because most of people use mozilla is from Netscape 4.x right? Add quote to context menu is a good idea, but add it to toolbar need all themes make change. This is another issue about UI change.
Reporter | ||
Comment 26•22 years ago
|
||
I did some more testing with both news and email messages and it works perfectly. When there is more than one 3-pane message window open one can put together a message that contains both quotes of emails and newsgroup articles -- wonderful! So what is necessary to get this into the daily builds? :)
Assignee | ||
Comment 27•22 years ago
|
||
johann: Thank you for testing. I'm asking for review. Hope this function can be checked in soon :-)
Comment 28•22 years ago
|
||
Adding Jean-Francois for the review and Jennifer to see if she wants this.
Status: NEW → ASSIGNED
Comment 29•22 years ago
|
||
Adding this item to the Options menu for Mail Compose would be fine. Please add it here: Select Addresses... Check Spelling Quote Original Message Rewrap ------------------- Please do not add a Toolbar button though. We tried very hard to keep the Toolbar uncluttered, only having the Most commonly used items on it. We learned from 4.x that "Quote" was not commonly used by average users (most we asked didn't even know what it did). It is more of an advanced user feature.
Comment 30•22 years ago
|
||
I'll review it later this week...
Reporter | ||
Comment 31•22 years ago
|
||
Just a minor remark: I think the current "quote message" text (for english locale) is better than "quote original message" because it doesnt sound like if this is just for quoting one particial message.
Comment 32•22 years ago
|
||
*** Bug 125694 has been marked as a duplicate of this bug. ***
Comment 33•22 years ago
|
||
First comment about the UI side of the patch (more comment to comes...) 1) you need to reactivate cmd_quoteMessage in defaultController.supportsCommand and defaultController.isCommandEnabled (see also comment 4 about mailSession) 2) could you move the function QuoteSelectedMessage after the function SelectAddress, that would be cleaner. 3) please remove the dump in the function QuoteSelectedMessage 4) instead of using a nsIWindowMediator to retreive the mail 3Pane window, use a nsIMsgMailSession. You would need to define a sMailSession which you can use also in defaultController.isCommandEnabled.
Comment 34•22 years ago
|
||
quoting the original message during reply or pressing the quote message button should give the same result, right? Therefore I don't see why you need to write a new QuoteStreamListener which does pratically the same thing than the existing one. You should be able to modify the current one to do the job. Johann, you did a good job implementing this missing feature but your patch need some more work before I can accept it.
Reporter | ||
Comment 35•22 years ago
|
||
um, its Pete's patch.
Comment 36•22 years ago
|
||
sorry Pete, my mistake...
Assignee | ||
Comment 37•22 years ago
|
||
Jean-Francois Ducarroz: Thank you for your suggestion. I will improve my patch later. Only one problem: Did you see the QuoteOutputStreamListener? It is for initial composer only. I think it is difficult to modify it to let it do the job. Because it contains lots of hard code logic for initial... Maybe it need rewrite... Can you give me some suggestion on this problem? Maybe I can use a flag to let the listener do different thing when onStopRequest ()...? Just have no idea...
Comment 38•22 years ago
|
||
Right, ou need to add a flag to the listener. Then you should be able to just modify the part that insert the data into the composer. Bassically, you should just have to be sure you insert the data at the current insertion point and don't modify the insertion point after, also don't try to insert a signature...
Reporter | ||
Comment 39•22 years ago
|
||
Pete, do you think this could make it into the 1.0 branch?
Assignee | ||
Comment 40•22 years ago
|
||
johann: No, this is a workarround patch. There are some problems with it. I have plan to work out a complete one recently.
Assignee | ||
Comment 41•22 years ago
|
||
With this patch, you can quote all message that you have selected. Jean-Francois, I could not find out how to use nsIMsgMailSession to get selected message's URI... Can you help me?
Attachment #68271 -
Attachment is obsolete: true
Assignee | ||
Comment 42•22 years ago
|
||
Only NotifyListener when quote original message. Because it will init to/cc list when quote selected message. only change following codes: - compose->NotifyStateListeners(eComposeFieldsReady, NS_OK); + if (mQuoteOriginal) + compose->NotifyStateListeners(eComposeFieldsReady, NS_OK);
Attachment #76180 -
Attachment is obsolete: true
Comment 43•22 years ago
|
||
>Jean-Francois, I could not find out how to use nsIMsgMailSession to get
>selected message's URI... Can you help me?
I don't know either, look around mailnews code to see how we use it
Assignee | ||
Comment 44•22 years ago
|
||
Jean-Francois, I can't find function like GetSelectedMessages from nsIMailSession or any other interface... I search in lxr and by keyword:GetSelectedMessage http://lxr.mozilla.org/seamonkey/search?string=GetSelectedMessages And only find this function is in msgMail3PaneWindow.js and messageWindow.js. If you ever knew other way to GetSelectedMessages, please help me to finger it out. Thanks!
Comment 45•22 years ago
|
||
Looks like we need to have access to the nsIMsgDBView but I canot find a way to get it from the backend. I'll ask help from mscott and bienvenu...
Comment 46•22 years ago
|
||
nsIMsgMailSession does not let us get back the domWindow which is what we want in order to access it's JS function like GetSelectedMessages. The reason I asked you to use nsIMsgMailSession instead of the window mediator is because getMostRecentWindow does not give you necessary the top most window but rather the last one created (that what I beleive)! But varada told me it wasn't true anymore. Can you do the following test: 1) Open the mail3Pane window (A) 2) Open a second mail3Pane window (B) by double-clicking on a folder in the window A. 3)The window B should be in front, open a compose window, select a message on the window B and try the quote function. 4) Activate the window A, select a message, come back to the compose window and try the quote function. if step 3 & 4 quote the correct message, don't need to use nsIMsgMailSession. Else, we will need to expand nsIMsgMailSession in order to be able to retreive the dom window. cc'ing mscott for his advice
Assignee | ||
Comment 47•22 years ago
|
||
Jean-Francois, I tested your case and they all quoted the correct message. I think it doesn't matter because the MostRecentWindow of mail3Pane type should be the last focused mail3Pane window.
Comment 48•22 years ago
|
||
good! Are you ready for a code review?
Assignee | ||
Comment 49•22 years ago
|
||
Jean-Francois,I think so. Can you?
Comment 50•22 years ago
|
||
Comment on attachment 76399 [details] [diff] [review] Fix some problem Good job, the patch works well. I have only few comment: 1) @@ -497,8 +498,8 @@ return !focusedElement; case "cmd_outputFormat": return composeHTML; -// case "cmd_quoteMessage": -// return mailSession && mailSession.topmostMsgWindow; + case "cmd_quoteMessage": + return true; case "cmd_rewrap": return !composeHTML && !focusedElement; We should not enable the "Quote Message" menu item if no mail3Pane window are open! Instead of always returning true, why not reactivating the old code! 2) function in IDL file should start with a lower case. It's not always the case for old IDL file (which need to be converted one day) but at least use a lower case for new addition + for (i = 0; i < selectedURIs.length; i++) + gMsgCompose.QuoteMessage(selectedURIs[i]); + } + /* ... */ + void QuoteMessage(in string msgURI); + 3)don't put the function void QuoteMessage(in string msgURI); before the void Initialize(in nsIDOMWindowInternal aWindow, in nsIMsgComposeParams aParams); put it rather after the function abort() in the idl file. 4) the implementation of the function NS_IMETHODIMP QuotingOutputStreamListener::InsertToCompose(nsIEditorShell *aEditorShell, PRBool aHTMLEditor) should goes after the function SetMimeHeaders(nsIMimeHeaders * headers) in the cpp file.
Attachment #76399 -
Flags: needs-work+
Assignee | ||
Comment 51•22 years ago
|
||
Thanks, Jean-Francois Ducarroz I tried mailSession.topmostMsgWindow, but it cause a JavaScript error when no mail3Window is opened * Call to xpconnect wrapped JSObject produced this error: * [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [n sIMsgMailSession.topmostMsgWindow]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: chrome://messenger/content/messengercompose/MsgComposeCom mands.js :: anonymous :: line 511" data: no] And I checked the code in nsMailSession.cpp. It will return NS_ERROR_FAILUE if no mail3Pane or messageWindow is open. I think we can always return true is isCommandEnabled, but do nothing when no mail3Window is opened(Controled by QuoteSelectedMessage()).
Comment 52•22 years ago
|
||
Then just use a try/catch to catch the error. Something like that: try { mailSession.topmostMsgWindow; return true; } catch () { return false;}
Assignee | ||
Comment 53•22 years ago
|
||
Jean-Francois Ducarroz, I have fixed those problem by your comments. Please review it. Thanks!
Attachment #76399 -
Attachment is obsolete: true
Assignee | ||
Comment 54•22 years ago
|
||
Lost ';' after gMailSession.topmostMsgWindow Ignore last patch please.
Attachment #78305 -
Attachment is obsolete: true
Comment 55•22 years ago
|
||
Comment on attachment 78307 [details] [diff] [review] Patch (add ';') on little thing: you need to reset gMailSession to null in the function ReleaseGlobalVariable().
Attachment #78307 -
Flags: needs-work+
Assignee | ||
Comment 56•22 years ago
|
||
Release gMailSession in ReleaseGlobalVariables()
Attachment #78307 -
Attachment is obsolete: true
Comment 57•22 years ago
|
||
Comment on attachment 78493 [details] [diff] [review] Patch (release the gMailSession) very good. R=ducarroz
Attachment #78493 -
Flags: review+
Comment 59•22 years ago
|
||
this is cool, thx for doing this. The prevailing braces style in this file is if (a) { } not if (a) { } could you convert your patch to use the prevailing braces style? You don't need to initialize nsCOMPtr's to nsnull; + nsCOMPtr<nsISelection> selection = nsnull; + nsCOMPtr<nsIDOMNode> parent = nsnull; + // i'm not sure if you need to move the selection back to before the + // break. expirement. can we clean up this comment? I'm guessing you mean "experiment". What's the result of the experiment?
Assignee | ||
Comment 60•22 years ago
|
||
bienvenu, I leave js file use old style, because we should use same style in same file. Do you agree? And I removed comments and remove init nsCOMPtr to null. Please review. thanks!
Attachment #78493 -
Attachment is obsolete: true
Comment 61•22 years ago
|
||
+ // Create a mime parser (nsIStreamConverter)! + mQuote = do_CreateInstance(NS_MSGQUOTE_CONTRACTID, &rv); + if (NS_FAILED(rv) || !mQuote) + return NS_ERROR_FAILURE; you don't need to check mQuote - you should just do an NS_ENSURE_SUCCESS(rv, rv) here. Other than that, it looks good.
Assignee | ||
Comment 62•22 years ago
|
||
I changed several place to use NS_ENSURE_SUCCESS and return rv directly.
Attachment #80906 -
Attachment is obsolete: true
Assignee | ||
Comment 63•22 years ago
|
||
bienvenu, can you sr= it? Thanks
Comment 64•22 years ago
|
||
Comment on attachment 81099 [details] [diff] [review] patch sr=bienvenu
Attachment #81099 -
Flags: superreview+
Assignee | ||
Comment 65•22 years ago
|
||
Comment on attachment 81099 [details] [diff] [review] patch ducarroz: Carry your r= here, is it ok? I will check in it if you feel ok. Thanks!
Attachment #81099 -
Flags: review+
Comment 66•22 years ago
|
||
sure , R=ducarroz
Comment 67•22 years ago
|
||
Since this requires string changes, this needs approval from the localization team (mcarlson@netscape.com) before approval for branch checkin can be given.
Comment 68•22 years ago
|
||
this really doesn't seem like the kind of fix we want to take on the moz 1.0 branch. Maybe a month or two ago but not right now, IMHO. What's the motivation to take it on the branch Randell?
Assignee | ||
Comment 69•22 years ago
|
||
I just want to let things easy when we want to release Netscape 7.x on Solaris. Because Netscape 6.2.2 on Sun already has this feature and works well. You know, Netscape 7.x will beased on moz1.0. If I can't let it in, I will have to do much efforts to let it in Netscape 7.x. But, if you guys feel it could make risk for moz1.0 or it's really not the good time to land it, it's ok. I will only check it into trunk.
Assignee | ||
Comment 70•22 years ago
|
||
fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 71•22 years ago
|
||
Who can verify this bug?
Comment 72•22 years ago
|
||
Pete, We are focusing on verifying bugs checked into branch currently. I will verify this bug when we get back to the trunk builds.
Comment 74•22 years ago
|
||
Works in Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.0+) Gecko/20020522. I don't understand why it is called option (it actually isn't an option, but an action and hence should be in edit, nevermind). pi
Comment 75•22 years ago
|
||
*** Bug 147079 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 76•22 years ago
|
||
VERIFIED according comments 74 Filed another bug 147502 for moving (Quote Message) from "Option" to "Edit"
Status: RESOLVED → VERIFIED
Comment 78•22 years ago
|
||
*** Bug 149550 has been marked as a duplicate of this bug. ***
Comment 79•22 years ago
|
||
*** Bug 149550 has been marked as a duplicate of this bug. ***
Comment 80•22 years ago
|
||
*** Bug 151829 has been marked as a duplicate of this bug. ***
Comment 81•22 years ago
|
||
*** Bug 137887 has been marked as a duplicate of this bug. ***
Comment 82•22 years ago
|
||
*** Bug 154489 has been marked as a duplicate of this bug. ***
Comment 83•22 years ago
|
||
Why isn't a fix to this bug included in Mozilla 1.1beta? Perhaps it does not work in this version? It has been SIX months already since the problem was declared as fixed. I am using Mozilla 1.1beta on Win-2000. Thanks, --Jarek
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Comment 84•22 years ago
|
||
Jarek, did you try the Option menu, Quote Message? It worked fine for me. I think this bug is about having the basic functionality, not about have a quote button.
Comment 85•22 years ago
|
||
You are right. The option is indeed there the bug status should indeed be closed. Similarly to many Communicator 4.X users I always accessed this functionality through the Quote Button rather than Option Menu and missed it in the current version. It would be nice to have the button back too... Thanks. Jarek
Comment 86•22 years ago
|
||
re-closing as fixed. Perhaps someone who feels strongly about this can file a new rfe for a quote button...
Status: REOPENED → RESOLVED
Closed: 22 years ago → 22 years ago
Resolution: --- → FIXED
Comment 87•22 years ago
|
||
There is already an existing bug for the Quote toolbar button. The plan is to have customizable toolbars (hopefully in the near future), in which users who want this button would be able to add it to their compose toolbar.
Comment 88•22 years ago
|
||
This feature does not seem to be working properly with plain text mail. Please see bug 158918.
Assignee | ||
Comment 89•22 years ago
|
||
jdunn, can you evaluate this one and let me know whether I can check into OEM branch?
Comment 90•22 years ago
|
||
The "option->quote" added in Mozilla1.0 to fix this bug has disappeared in mozilla1.0.1 (build 2002082607), but is present in Mozilla1.1 Jerome
Updated•19 years ago
|
Product: Browser → Seamonkey
Updated•3 years ago
|
Summary: The "quote original message" option is missing when creating a reply or new message → The "quote original message" option is missing when creating a reply or new message (restore feature to quote selected messages)
You need to log in
before you can comment on or make changes to this bug.
Description
•