70478 - The "quote original message" option is missing when creating a reply or new message (restore feature to quote selected messages)

Status: RESOLVED FIXED

(SeaMonkey :: MailNews: Message Display, enhancement)

Description

[[e-mail]]

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

[Boris Zbarsky [:bzbarsky]]

confirming rfe

Comment 2

[Sheela Ravindran]

change ->qa contact to myself, cc esther

Comment 3

[J Luh]

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.

Comment 4

[Mark Lord]

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.

Comment 5

[Tom Mraz]

*** Bug 87017 has been marked as a duplicate of this bug. ***

Comment 6

[Eric Vaandering (no email)]

*** Bug 88644 has been marked as a duplicate of this bug. ***

Comment 7

[scottputterman]

adding nsenterprise-

Comment 8

[Greg K.]

*** Bug 91948 has been marked as a duplicate of this bug. ***

Comment 9

[[e-mail]]

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

[TucsonTester1]

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

[Sheela Ravindran]

*** Bug 104604 has been marked as a duplicate of this bug. ***

Comment 12

[R.K.Aa.]

*** Bug 118746 has been marked as a duplicate of this bug. ***

Comment 13

[Jim Henderson]

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

[Frank Griffin]

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

[Sheela Ravindran]

*** Bug 121050 has been marked as a duplicate of this bug. ***

Comment 16

[Pete Zha]

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

[Roland Traulaner]

4xp bugs are by definition not enhancements. This is important 
basic functionality that is still missing from Mozilla.
Severity should be changed to normal.

Comment 18

[Pete Zha]

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.

Comment 19

[Pete Zha]

Attachment: Workarround code (patch)

Add a quote stream listener to messenger compose module.
Tested on my workstation.
Please review it, thanks!

Comment 20

[[e-mail]]

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

[Ben Ruppel]

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.  

Comment 22

[[e-mail]]

Ben, yes I tested this and it works with as many messages from any folder
as you want. Wonderful Pete!

Comment 23

[J Luh]

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.

Comment 24

[[e-mail]]

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?

Comment 25

[Pete Zha]

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.

Comment 26

[[e-mail]]

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? :)

Comment 27

[Pete Zha]

johann: Thank you for testing. I'm asking for review. Hope this function can be
checked in soon :-)

Comment 28

[scottputterman]

Adding Jean-Francois for the review and Jennifer to see if she wants this.

Comment 29

[jglick]

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

[Jean-Francois Ducarroz]

I'll review it later this week...

Comment 31

[[e-mail]]

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

[R.K.Aa.]

*** Bug 125694 has been marked as a duplicate of this bug. ***

Comment 33

[Jean-Francois Ducarroz]

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

[Jean-Francois Ducarroz]

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.

Comment 35

[[e-mail]]

um, its Pete's patch. 

Comment 36

[Jean-Francois Ducarroz]

sorry Pete, my mistake...

Comment 37

[Pete Zha]

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

[Jean-Francois Ducarroz]

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...

Comment 39

[[e-mail]]

Pete, do you think this could make it into the 1.0 branch?

Comment 40

[Pete Zha]

johann: No, this is a workarround patch. There are some problems with it. I have
plan to work out a complete one recently.

Comment 41

[Pete Zha]

Attachment: Patch to fix it

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?

Comment 42

[Pete Zha]

Attachment: Fix some problem

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);

Comment 43

[Jean-Francois Ducarroz]

>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

Comment 44

[Pete Zha]

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

[Jean-Francois Ducarroz]

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

[Jean-Francois Ducarroz]

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

Comment 47

[Pete Zha]

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

[Jean-Francois Ducarroz]

good! Are you ready for a code review?

Comment 49

[Pete Zha]

Jean-Francois,I think so. Can you?

Comment 50

[Jean-Francois Ducarroz]

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.

Comment 51

[Pete Zha]

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

[Jean-Francois Ducarroz]

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

try {
  mailSession.topmostMsgWindow;
  return true;
} catch () { return false;}

Comment 53

[Pete Zha]

Attachment: Patch

Jean-Francois Ducarroz, 
I have fixed those problem by your comments. Please review it. Thanks!

Comment 54

[Pete Zha]

Attachment: Patch (add ';')

Lost ';' after gMailSession.topmostMsgWindow
Ignore last patch please.

Comment 55

[Jean-Francois Ducarroz]

Comment on attachment 78307 [details] [diff] [review]
Patch (add ';')

on little thing:
you need to reset gMailSession to null in the function ReleaseGlobalVariable().

Comment 56

[Pete Zha]

Attachment: Patch (release the gMailSession)

Release gMailSession in ReleaseGlobalVariables()

Comment 57

[Jean-Francois Ducarroz]

Comment on attachment 78493 [details] [diff] [review]
Patch (release the gMailSession)

very good. R=ducarroz

Comment 58

[Pete Zha]

Can anyone sr= it? Thanks!

Comment 59

[David :Bienvenu]

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?

Comment 60

[Pete Zha]

Attachment: Patch

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!

Comment 61

[David :Bienvenu]

+  // 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.

Comment 62

[Pete Zha]

Attachment: patch

I changed several place to use NS_ENSURE_SUCCESS and return rv directly.

Comment 63

[Pete Zha]

bienvenu, can you sr= it?
Thanks

Comment 64

[David :Bienvenu]

Comment on attachment 81099 [details] [diff] [review]
patch

sr=bienvenu

Comment 65

[Pete Zha]

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!

Comment 66

[Jean-Francois Ducarroz]

sure , R=ducarroz

Comment 67

[Randell Jesup [:jesup] (needinfo me)]

Since this requires string changes, this needs approval from the localization
team ([e-mail]) before approval for branch checkin can be given.

Comment 68

[Scott MacGregor]

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? 

Comment 69

[Pete Zha]

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.

Comment 70

[Pete Zha]

fix checked in.

Comment 71

[Pete Zha]

Who can verify this bug?

Comment 72

[Sheela Ravindran]

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 73

[Sheela Ravindran]

Reassign now  meehansqa will be qa contact

Comment 74

[Boris 'pi' Piwinger]

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

[Boris 'pi' Piwinger]

*** Bug 147079 has been marked as a duplicate of this bug. ***

Comment 76

[Pete Zha]

VERIFIED according comments 74
Filed another bug 147502 for moving (Quote Message)  from "Option" to "Edit"

Comment 77

[Pete Zha]

->myself for tracking

Comment 78

[Matthias Versen [:Matti]]

*** Bug 149550 has been marked as a duplicate of this bug. ***

Comment 79

[Matthias Versen [:Matti]]

*** Bug 149550 has been marked as a duplicate of this bug. ***

Comment 80

[Jean-Francois Ducarroz]

*** Bug 151829 has been marked as a duplicate of this bug. ***

Comment 81

[esther]

*** Bug 137887 has been marked as a duplicate of this bug. ***

Comment 82

[esther]

*** Bug 154489 has been marked as a duplicate of this bug. ***

Comment 83

[Jarek Nieplocha]

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

Comment 84

[David :Bienvenu]

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

[Jarek Nieplocha]

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

[David :Bienvenu]

re-closing as fixed. Perhaps someone who feels strongly about this can file a
new rfe for a quote button...

Comment 87

[jglick]

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

[J Luh]

This feature does not seem to be working properly with plain text mail. Please
see bug 158918.

Comment 89

[Pete Zha]

jdunn, can you evaluate this one and let me know whether I can check into OEM
branch?

Comment 90

[Jerome Droniou]

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