Closed Bug 70478 Opened 24 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)

Product:
SeaMonkey
Component:
MailNews: Message Display
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: johann.petrak, Assigned: zhayupeng)

References

Details

(Keywords: qawanted)

Attachments

(1 file, 7 obsolete files)

patch
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
confirming rfe
Status: UNCONFIRMED → NEW
Ever confirmed: true
Hardware: PC → All
change ->qa contact to myself, cc esther
QA Contact: esther → sheelar
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: 4xp
Keywords: nsCatFood
*** Bug 87017 has been marked as a duplicate of this bug. ***
Keywords: nsenterprise
*** Bug 88644 has been marked as a duplicate of this bug. ***
adding nsenterprise-
Keywords: nsenterprise → nsenterprise-
*** Bug 91948 has been marked as a duplicate of this bug. ***
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. 
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.
 !
*** Bug 104604 has been marked as a duplicate of this bug. ***
*** Bug 118746 has been marked as a duplicate of this bug. ***
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.
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 ?
*** Bug 121050 has been marked as a duplicate of this bug. ***
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.
4xp bugs are by definition not enhancements. This is important 
basic functionality that is still missing from Mozilla.
Severity should be changed to normal.
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.
Attached patch Workarround code (patch) (obsolete) — — Splinter Review 
Add a quote stream listener to messenger compose module.
Tested on my workstation.
Please review it, thanks!
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?

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.  
Ben, yes I tested this and it works with as many messages from any folder
as you want. Wonderful Pete!
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.
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?
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.
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? :)
johann: Thank you for testing. I'm asking for review. Hope this function can be
checked in soon :-)

Adding Jean-Francois for the review and Jennifer to see if she wants this.
Status: NEW → ASSIGNED
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. 
I'll review it later this week...
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.
*** Bug 125694 has been marked as a duplicate of this bug. ***
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.
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.
um, its Pete's patch. 
sorry Pete, my mistake...
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...
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...
Pete, do you think this could make it into the 1.0 branch?
johann: No, this is a workarround patch. There are some problems with it. I have
plan to work out a complete one recently.
Attached patch Patch to fix it (obsolete) — — Splinter Review 
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
Whiteboard: seeking r=
Attached patch Fix some problem (obsolete) — — Splinter Review 
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
>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
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!
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...
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
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.

good! Are you ready for a code review?
Jean-Francois,I think so. Can you?
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+
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()).

Then just use a try/catch to catch the error. Something like that:

try {
  mailSession.topmostMsgWindow;
  return true;
} catch () { return false;}
Attached patch Patch (obsolete) — — Splinter Review 
Jean-Francois Ducarroz, 
I have fixed those problem by your comments. Please review it. Thanks!
Attachment #76399 - Attachment is obsolete: true
Attached patch Patch (add ';') (obsolete) — — Splinter Review 
Lost ';' after gMailSession.topmostMsgWindow
Ignore last patch please.
Attachment #78305 - Attachment is obsolete: true
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+
Attached patch Patch (release the gMailSession) (obsolete) — — Splinter Review 
Release gMailSession in ReleaseGlobalVariables()
Attachment #78307 - Attachment is obsolete: true
Comment on attachment 78493 [details] [diff] [review]
Patch (release the gMailSession)

very good. R=ducarroz
Attachment #78493 - Flags: review+
Can anyone sr= it? Thanks!
Whiteboard: seeking r= → seeking sr=
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?
Attached patch Patch (obsolete) — — Splinter Review 
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
+  // 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.
Attached patch patch — — Splinter Review 
I changed several place to use NS_ENSURE_SUCCESS and return rv directly.
Attachment #80906 - Attachment is obsolete: true
bienvenu, can you sr= it?
Thanks
Comment on attachment 81099 [details] [diff] [review]
patch

sr=bienvenu
Attachment #81099 - Flags: superreview+
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+
sure , R=ducarroz
Since this requires string changes, this needs approval from the localization
team (mcarlson@netscape.com) before approval for branch checkin can be given.
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? 
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.
fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Who can verify this bug?
Keywords: qawanted
Whiteboard: seeking sr=
Pete,
We are focusing on verifying bugs checked into branch currently. I will verify 
this bug when we get back to the trunk builds.  
Reassign now  meehansqa will be qa contact
QA Contact: sheelar → meehansqa
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
*** Bug 147079 has been marked as a duplicate of this bug. ***
VERIFIED according comments 74
Filed another bug 147502 for moving (Quote Message)  from "Option" to "Edit"
Status: RESOLVED → VERIFIED
->myself for tracking
Assignee: sspitzer → pete.zha
*** Bug 149550 has been marked as a duplicate of this bug. ***
*** Bug 149550 has been marked as a duplicate of this bug. ***
*** Bug 151829 has been marked as a duplicate of this bug. ***
*** Bug 137887 has been marked as a duplicate of this bug. ***
*** Bug 154489 has been marked as a duplicate of this bug. ***
Whiteboard: branchOEM
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 → ---
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.
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

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
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.
This feature does not seem to be working properly with plain text mail. Please
see bug 158918.
jdunn, can you evaluate this one and let me know whether I can check into OEM
branch?
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
Keywords: branchOEM
Whiteboard: branchOEM
Product: Browser → Seamonkey
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.

Attachment

General

Creator:
Created:
Updated:
Size: