The "quote original message" option is missing when creating a reply or new message

RESOLVED FIXED

Status

--
enhancement
RESOLVED FIXED
18 years ago
14 years ago

People

(Reporter: johann.petrak, Assigned: zhayupeng)

Tracking

({qawanted})

Trunk
qawanted

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 7 obsolete attachments)

(Reporter)

Description

18 years ago
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

Comment 2

18 years ago
change ->qa contact to myself, cc esther
QA Contact: esther → sheelar

Comment 3

18 years ago
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

18 years ago
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.
(Reporter)

Updated

18 years ago
Keywords: 4xp
(Reporter)

Updated

18 years ago
Keywords: nsCatFood

Comment 5

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

Updated

18 years ago
Keywords: nsenterprise
*** Bug 88644 has been marked as a duplicate of this bug. ***

Comment 7

18 years ago
adding nsenterprise-
Keywords: nsenterprise → nsenterprise-

Comment 8

18 years ago
*** Bug 91948 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 9

18 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

17 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

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

Comment 12

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

Comment 13

17 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

17 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

17 years ago
*** Bug 121050 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 16

17 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

17 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

17 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

17 years ago
Created attachment 68271 [details] [diff] [review]
Workarround code (patch)

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

Comment 20

17 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

17 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

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

Comment 23

17 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

17 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

17 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

17 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

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

Comment 28

17 years ago
Adding Jean-Francois for the review and Jennifer to see if she wants this.
Status: NEW → ASSIGNED

Comment 29

17 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. 
I'll review it later this week...
(Reporter)

Comment 31

17 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

17 years ago
*** 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.
(Reporter)

Comment 35

17 years ago
um, its Pete's patch. 
sorry Pete, my mistake...
(Assignee)

Comment 37

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

17 years ago
Pete, do you think this could make it into the 1.0 branch?
(Assignee)

Comment 40

17 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

17 years ago
Created attachment 76180 [details] [diff] [review]
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?
Attachment #68271 - Attachment is obsolete: true
(Assignee)

Updated

17 years ago
Whiteboard: seeking r=
(Assignee)

Comment 42

17 years ago
Created attachment 76399 [details] [diff] [review]
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);
(Assignee)

Updated

17 years ago
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
(Assignee)

Comment 44

17 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!
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
(Assignee)

Comment 47

17 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.
good! Are you ready for a code review?
(Assignee)

Comment 49

17 years ago
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+
(Assignee)

Comment 51

17 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()).
Then just use a try/catch to catch the error. Something like that:

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

Comment 53

17 years ago
Created attachment 78305 [details] [diff] [review]
Patch

Jean-Francois Ducarroz, 
I have fixed those problem by your comments. Please review it. Thanks!
Attachment #76399 - Attachment is obsolete: true
(Assignee)

Comment 54

17 years ago
Created attachment 78307 [details] [diff] [review]
Patch (add ';')

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+
(Assignee)

Comment 56

17 years ago
Created attachment 78493 [details] [diff] [review]
Patch (release the gMailSession)

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+
(Assignee)

Comment 58

17 years ago
Can anyone sr= it? Thanks!
Whiteboard: seeking r= → seeking sr=

Comment 59

17 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

17 years ago
Created attachment 80906 [details] [diff] [review]
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!
Attachment #78493 - Attachment is obsolete: true

Comment 61

17 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

17 years ago
Created attachment 81099 [details] [diff] [review]
patch

I changed several place to use NS_ENSURE_SUCCESS and return rv directly.
Attachment #80906 - Attachment is obsolete: true
(Assignee)

Comment 63

17 years ago
bienvenu, can you sr= it?
Thanks

Comment 64

17 years ago
Comment on attachment 81099 [details] [diff] [review]
patch

sr=bienvenu
Attachment #81099 - Flags: superreview+
(Assignee)

Comment 65

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

Comment 68

17 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

17 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

17 years ago
fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
(Assignee)

Comment 71

17 years ago
Who can verify this bug?
(Assignee)

Updated

17 years ago
Keywords: qawanted
Whiteboard: seeking sr=

Comment 72

17 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 73

17 years ago
Reassign now  meehansqa will be qa contact
QA Contact: sheelar → meehansqa

Comment 74

17 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

17 years ago
*** Bug 147079 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 76

17 years ago
VERIFIED according comments 74
Filed another bug 147502 for moving (Quote Message)  from "Option" to "Edit"
Status: RESOLVED → VERIFIED
(Assignee)

Comment 77

17 years ago
->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. ***

Comment 81

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

Comment 82

17 years ago
*** Bug 154489 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

17 years ago
Whiteboard: branchOEM

Comment 83

17 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

17 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

17 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

17 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
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED

Comment 87

17 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

17 years ago
This feature does not seem to be working properly with plain text mail. Please
see bug 158918.
(Assignee)

Comment 89

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

Comment 90

16 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

16 years ago
Keywords: branchOEM
Whiteboard: branchOEM
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.