Last Comment Bug 23394 - Quote just the selected portion of a message during Reply
: Quote just the selected portion of a message during Reply
Status: VERIFIED FIXED
:
Product: MailNews Core
Classification: Components
Component: Composition (show other bugs)
: Trunk
: All All
: P3 enhancement with 104 votes (vote)
: mozilla1.9
Assigned To: Jeff Beckley
:
:
Mentors:
http://www.xs4all.nl/%7Ejs/gnksa/
: 69129 89610 107553 129650 131365 148669 193370 223094 223098 249267 257404 268106 272898 281400 299823 305549 312198 326992 353829 361435 381818 419063 423904 488642 (view as bug list)
Depends on: 466330 543152 610376
Blocks: gnksa eudora 359227
  Show dependency treegraph
 
Reported: 2000-01-07 15:59 PST by John Moreno
Modified: 2012-04-11 12:04 PDT (History)
88 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Implementation of reply quote selected (16.58 KB, patch)
2008-01-21 15:36 PST, Jeff Beckley
no flags Details | Diff | Splinter Review
New patch from comments (18.54 KB, patch)
2008-01-25 09:41 PST, Jeff Beckley
mozilla: review+
Details | Diff | Splinter Review
Now with multi-word check (19.72 KB, patch)
2008-01-29 17:27 PST, Jeff Beckley
no flags Details | Diff | Splinter Review
Un bit-rotted and fixed reply all (24.95 KB, patch)
2008-03-06 10:47 PST, Jeff Beckley
dmose: superreview-
Details | Diff | Splinter Review
New patch addressing dmose's comments (24.94 KB, patch)
2008-04-04 09:53 PDT, Jeff Beckley
dmose: superreview+
Details | Diff | Splinter Review
Changes addressing dmose's latest comments (24.99 KB, patch)
2008-06-02 22:14 PDT, Jeff Beckley
smontagu: review-
beckley: superreview+
Details | Diff | Splinter Review
More robust word detection (25.70 KB, patch)
2008-06-10 00:24 PDT, Jeff Beckley
smontagu: review+
beckley: superreview+
Details | Diff | Splinter Review

Description John Moreno 2000-01-07 15:59:12 PST
This is one of the GNKSA SHOULDs that earlier versions of Netscape "failed".
Quoting just part of a message makes it easier to focus a reply on just the
relevant portion.
Comment 1 lchiang 2000-01-07 16:54:59 PST
cc: phil.  Phil, didn't you file bugs for a bunch of the gnksa items already?
Comment 2 Jean-Francois Ducarroz 2000-01-11 17:01:59 PST
No Idea if we will do it, will see...could be nice...
Comment 3 Matthew Paul Thomas 2000-04-12 01:59:32 PDT
Implementation note: If the user selects some of the message, the text of the 
menu item (and the toolbar button tooltips) should change from `Reply to Message' 
to `Reply to Selected Text'.

Note that if the bug for snipping a signature on reply is fixed, this bug should 
be fixed at about the same time, so that a user can still include the signature 
in their reply (for the purposes of commenting on the sig or whatever) -- by 
pressing Ctrl+A (select all), then Ctrl+R (reply to selected text).
Comment 4 Ben Bucksch (:BenB) 2000-06-07 01:25:24 PDT
<quote src="http://www.xs4all.nl/%7Ejs/gnksa/gnksa.txt">
As a direct counterpart to this requirement, the software SHOULD offer
the user some means of selecting exactly which part of a Usenet posting
she wishes to followup to, and quote only that part initially.  (A
special case of this is when a user wishes to react to what's in a
signature.)
</quote>

BTW: A workaround would be "select - copy - reply - select all - delete - paste
as quotation". It does the same, but is a long sequence.

> Note that if the bug for snipping a signature on reply is fixed, this bug
> should be fixed at about the same time, so that a user can still include the
> signature in their reply (for the purposes of commenting on the sig or
> whatever) -- by pressing Ctrl+A (select all), then Ctrl+R (reply to selected
> text).

Agreed, but live was different :-(. I agree, that we should fix this bug as soon
as reasonable.

Unfortunately, the implementation involves lots of JS/DOM stuff, I guess, so I
won't be able to do much for it :-(.
Comment 5 Akkana Peck 2000-06-07 12:20:45 PDT
I looked into this once, hoping there might be a quick fix.  The problem I ran
up against was that quoting in the message window is asynchronous and comes from
the mail database, not from the DOM in the message window, whereas the selection
is against the message window DOM.

Now that it's possible to get the selection in JS from a Window or Document
object, it might be easier to implement this purely in JS: if you can get the
window corresponding to the message pane content, then you can get the selection
from that, do a ToString on it, bring up a new compose window (no reply), then
do an InsertAsQuotation, and not even need to go into C++ code.  Alas, I had to
go work on dogfood/beta bugs and ran out of time to play around with this. 
Perhaps these suggestions will help someone else ...
Comment 6 sol 2000-07-19 17:51:51 PDT
Moving to future milestone. This would be really nice to have, but I don't think 
we'll have time to get this in.
Comment 7 Akkana Peck 2000-07-20 10:14:19 PDT
Looks like the helpwanted keyword got lost -- this should clearly be a
helpwanted bug.
Comment 8 Ben Bucksch (:BenB) 2000-07-23 10:05:53 PDT
> bring up a new compose window (no reply)

huh? why not a reply? If reply, that's exactly the problem with your proposal:
You won't easily get the headers etc.. So, I think, this bug would involve C++
hacking (nsMsgCompose, I think).
Comment 9 Akkana Peck 2000-07-24 15:11:15 PDT
Not a reply because then the C++ code wouldn't auto-insert the whole quoted
material, and we'd be free to get the contents of the selection and insert it
ourselves from JS (once the window came up).
Comment 10 Ben Bucksch (:BenB) 2000-07-24 15:33:49 PDT
But you loose all of the reply-info: recipients, threading etc..

I don't think, this bug is hard to fix. It just crosses a lot of code (UI,
nsMsgCompose, possibly nsIMsgCompose).
Comment 11 Boris Zbarsky [:bz] (still a bit busy) 2001-10-30 08:51:31 PST
*** Bug 89610 has been marked as a duplicate of this bug. ***
Comment 12 Boris Zbarsky [:bz] (still a bit busy) 2001-10-30 08:51:37 PST
*** Bug 107553 has been marked as a duplicate of this bug. ***
Comment 13 Brice Ruth 2001-10-30 09:09:22 PST
Wow, its been over a year since this has been looked at ... in that time, many
things have changed, I suppose.  What's the current take on how difficult this
would be?
Comment 14 R.K.Aa. 2001-12-10 21:41:44 PST
*** Bug 69129 has been marked as a duplicate of this bug. ***
Comment 15 mental 2002-03-16 00:14:58 PST
*** Bug 129650 has been marked as a duplicate of this bug. ***
Comment 16 Boris 'pi' Piwinger 2002-04-17 23:21:09 PDT
*** Bug 131365 has been marked as a duplicate of this bug. ***
Comment 17 Charles 2002-06-02 12:24:05 PDT
I am very surprised this feature request is so old...

It is an extremely useful feature that most mailers have...

Also, a 'Paste as quotation' would also be very nice, once this feature is
implemented, so that *any* copied/pasted text could be pasted as a quotation...
or, at the very least, the ability to 'quote only selected text' with
discontiguous blocks of text in a message.

In addition (and similarly), the ability to re-wrap blocks of text, cleaning up
the quoting characters, would be awesome!  I have sorely missed this feature
that I have only seen in Outlook Express on the Mac.

Would it be possible to 'borrow' code from some other project?

I know of one Open Source mailer that does this very well - Sylpheed - but it is
done in GTK+...

*Please* bump this up in priority, guys - I would really love to start using
Mozilla and Mozilla Mail as our company's exclusive Browser/Mailer of choice.

Thanks

Charles Marcus
I.T. Director
Media Brokers International
Comment 18 Ben Bucksch (:BenB) 2002-06-02 12:34:02 PDT
> Paste as quotation

Exists.

> the ability to re-wrap blocks of text, cleaning up
> the quoting characters

Exists, independently for both HTML composer and plaintext composer.
Comment 19 Charles 2002-06-02 13:05:20 PDT
Well, I posted this feature request on the n.p.m.wishlist newsgroup, and someone
pointed out that this feature request was an old one - and boy was I surprised
to find out it was over two *years* old - thats a long time to wait for this
functionality.  After reading all of the comments on it, I decided to submit a
new one with a couple of additional-but-related feature requests of mine, as a
new issue, and mark the old one (this one) as a duplicate of the new one.  I
hope I am not out of line doing this - this is my first foray into the Bugzilla
world.  If this was not the proper way to do this, my apologies...
Comment 20 Charles 2002-06-02 13:10:13 PDT
>> Paste as quotation

>Exists.

I know - I just realized it... man, must be getting old...

>> the ability to re-wrap blocks of text, cleaning up
>> the quoting characters

> Exists, independently for both HTML composer and plaintext composer.

Doesn't work - at least, not for me.  I putposefully messed up some paragraphs,
and it didn't matter if I selected the paragraph in question ot not, Rewrap
didn't do anything.

Maybe I am not using it properly?
Comment 21 Boris 'pi' Piwinger 2002-06-02 13:10:36 PDT
*** Bug 148669 has been marked as a duplicate of this bug. ***
Comment 22 Betsy 2002-08-13 07:32:48 PDT
There should be a keyboard combination for 'paste as quote'. 

I use selective quotes all the time, and would like to be able to use the
keyboard, not the mouse.
Comment 23 Jean-Francois Ducarroz 2002-08-13 07:44:19 PDT
-->varada
Comment 24 Garth Wallace 2002-08-14 01:11:32 PDT
Betsy: that would be a separate bug entirely.
Comment 25 Betsy 2002-08-14 08:00:43 PDT
I got carried away with the power of writing to the bug list - i'm just an
ordinary consumer not a programmer :-)

I've written to feedback about it. Can i report it as a bug? It really annoys me.
Comment 26 Akkana Peck 2002-08-14 20:37:06 PDT
Betsy: If you have a middlemouse button and set the "middlemouse.paste" pref (on
by default on linux, off on the other platforms), then ctrl-middleclick does a
paste as quotation.  But you'd have to file another bug to request a key binding
for it.
Comment 27 Betsy 2002-08-20 10:28:57 PDT
It turns out that this is possible but there is no menu entry.

This is a quote that was posted in Bug 100567:
http://bugzilla.mozilla.org/show_bug.cgi?id=100567

"Additional Comment #4 From Dave North  2002-06-25 18:24 -------

... just discovered (on OS X) that apple-shift-v does the trick. My "bug" would
be: no accelerator listed in the menu, even though there is one.

Windows ... ctrl-shift-v.

... Linux ctrl-shift-v .

So it looks like all that needs to be done is including a menu entry to let the
userverse know this."
Comment 28 Ben Bucksch (:BenB) 2002-08-20 15:41:45 PDT
Ctrl-Shift-V = Paste As Quotation. So, there is a menu entry, but it's not this
bug (see comment 4). (The keyboard shortcut is not in the menu, that's true, but
again, it's not this bug.)
Changing summary to narrow bug.
Comment 29 (not reading, please use seth@sspitzer.org instead) 2002-12-16 12:41:16 PST
taking all of varada's bugs.
Comment 30 Michael A. Koenecke 2002-12-16 13:02:29 PST
In response to all the comments helpfully pointing out the "Paste as Quotation"
feature: yes, I know about that, but quoting just the selected portion is more
efficient than Ctrl-C to Copy, Reply, Select All, Ctrl-V to Paste. I regret that
I don't speak C++, but figure all that is needed is one snippet of code, which,
in a scripting language would look something like this:

If (NOT ?SelectedText)
EditSelectAll()
EndIf
vSource = ?SelectedText
[Existing code to switch to compose window, insert recipient, go to text entry box]
PasteasQuotation (vSource)

Seems to me like one little conditional statement is all that would be needed: a
fairly trivial addition to save a few steps in responding to e-mails. Also, it
would work the same way in Usenet replies, where often one wishes only to
respond to a portion of a message. "Paste as Quotation" is a nice workaround,
but it's still just a workaround.
Comment 31 Ben Bucksch (:BenB) 2002-12-16 13:07:49 PST
> vSource = ?SelectedText
> [Existing code to switch to compose window, insert recipient, go to text entry
box]
> PasteasQuotation (vSource)

Well, yeah, but one of the problems, as so often, is to pass around that
vSource. Which means that it's way more than "one snipplet of code".
Comment 32 Michael A. Koenecke 2002-12-16 13:54:53 PST
My apologies. I just wanted to point out, as others did a couple of years ago,
that other e-mail clients and/or newsreaders have this functionality, and that
Paste As Quotation, in my personal opinion and preferences, is not the solution
to this issue. Cheers.
Comment 33 Brice Ruth 2002-12-16 18:45:18 PST
I concur with Michael's statements. And while it may be difficult passing around
vSource, let's not shoot down a commenter just because he isn't a C++ guru. In
the grand scheme of Mozilla development, this functionality is a
drop-in-the-bucket, with respect to difficulty. And while it may not be as easy
as presented by Michael, it isn't as difficult as the inaction on this request
might indicate.

On the other hand, it should be understood that there may be more important bugs
that are requiring developer's attention at the moment. To make this bug "more
important", in a way - vote for it.

Cheers.
Comment 34 David Burband 2003-02-09 03:18:59 PST
I also concour with  Michael A. Koenecke, that this is a necssity for a
competent mail client. Even OSS clients like Sylpheed have had this for at least
a year. If I was a competent programmer, I wouldn't hesitate to do it myself.
Unfortunately not all of us are - I'm simply a lowly beta tester, who
contributes the best way I can. Find bugs and request needed features.

Please Seth Spizer, I know you're overworked but we need this. I think it's
telling that somebody of John Moreno acutally opened this.

Cheers,
krutch
Comment 35 John Moreno 2003-02-10 01:21:03 PST
Seth, given that you yourself reported a duplicate of this bug, any idea as 
to whether or not you'll actually be able to work on it?
Comment 36 Sander 2003-02-14 11:25:42 PST
*** Bug 193370 has been marked as a duplicate of this bug. ***
Comment 37 Robert La Ferla 2003-05-30 15:57:28 PDT
I agree that this is an important enhancement to make.  It is especially useful
for replying to digest mail where you just want to quote one of the messages but
there are many other uses.  Please implement it.
Comment 38 David Fraser 2003-07-31 23:19:11 PDT
I recently implemented a patch for the QuickReply extension to Thunderbird that
enables you to quote just the selection:
http://groups.google.com/groups?hl=en&lr=&ie=UTF-8&threadm=bgbafm%24lp7%241%40ctb-nnrp2.saix.net&rnum=1&prev=/groups%3Fq%3Dgroup%253Anetscape.public.mozilla.mail-news%2Bquickreply
Anyone interested in this bug might like to try this out.

One of the real frustrations of trying to implement this patch was that I looked
at the standard Thunderbird coding for replies, which goes through a complicated
mechanism of loading the original message from the message database... it seems
there is no easy way (without stream listeners, etc) to just get the message body.
Having looked at this bug, I think the real reason that it hasn't been fixed is
that the whole reply mechanism is too complicated. If somebody looked at the
whole thing and simplified it, it would be fairly easy to do the
reply-to-selection. It was much easier to implement for quick-reply by just
getting the message pane frame, which you can ask for the selection. Look at the
patch if you're interested in details.
Comment 39 Joe Infla 2003-10-21 09:18:15 PDT
*** Bug 223094 has been marked as a duplicate of this bug. ***
Comment 40 Joe Infla 2003-10-21 09:21:03 PDT
*** Bug 223098 has been marked as a duplicate of this bug. ***
Comment 41 Greg Baugher 2003-10-21 09:48:47 PDT
This bug is something that would be VERY helpful to the people I support.  It is
not uncommon to have huge messages that include some personal and some business
correspondence.  The user then wishes to forward only part of the message to
someone else without going through a copy/paste routine.  While this is a
work-around, the inability to print/forward/reply selected automatically is a
major inconvenience.  These users have had mailers in the past the offer this
function, so it is an expected feature when they move to MozMail.  They let me
know that they miss it, and if they care enough to call me and ask how to do it,
then I know that it consciously bothers them.  Please look into implementing
this.  From my standpoint, it should be a matter of Mail understanding that some
text in the body is highlighted, and when a forward/reply/print button is
pressed, only move the selected text.  Options regarding inline/attachment and
respond above/below quote should be honoured.
Comment 42 John Mora 2003-10-27 09:25:01 PST
This would be a great feature for people who only like replying to one specific
portion of a message. Even a preference that would allow this would be awesome.

Thanks for considering it!
Comment 43 John Mora 2003-11-05 09:49:03 PST
Other than the Quickreply extension, is there currently a workaround to enable
this behavior? Quickreply is a nice piece of software, but it isn't quite
"right" for me (no offense).

Any hacks or suggestions?
Comment 44 Ben Bucksch (:BenB) 2003-11-05 10:15:00 PST
Select, Ctrl-C (Copy), Ctrl-R (Reply), Ctrl-Shift-Home (select everything),
Shift-Cursor-Down (unselect atribution line), Delete, Ctrl-Shift-V (Paste as
Quotation)
Comment 45 Bjoern Sjut 2004-06-27 15:16:35 PDT
I just wanted to add another "please" to this bug. I just switched over from
PMMail2000, and quoting only the selected parts of a message really speeds up
the reply process. 

In fact, it's the one and only fact, I miss the most about Mozilla Mail.
Unfortunately I'm not a programmer, so I can't help with coding that stuff, but
hey, I'll be able to provide a lot of encouragement, good karma and stuff.. :-)

So please pick this bug up again. A lot of users will thank you for it - even if
they don't know it by now.

Best, Bjoern
Comment 46 Bogdan Stroe 2004-06-30 16:08:13 PDT
*** Bug 249267 has been marked as a duplicate of this bug. ***
Comment 47 Jonas Diemer 2004-08-30 08:23:02 PDT
*** Bug 257404 has been marked as a duplicate of this bug. ***
Comment 48 Christian :Biesinger (don't email me, ping me on IRC) 2004-11-06 09:21:51 PST
*** Bug 268106 has been marked as a duplicate of this bug. ***
Comment 49 Jack Naylor, P.E. 2004-11-17 19:44:00 PST
and make it appear in an alternate color with an introduction stating:

"Message Text in <insert color here> written by <insert name here>".
Comment 50 Jo Hermans 2004-12-03 01:55:44 PST
*** Bug 272898 has been marked as a duplicate of this bug. ***
Comment 51 Charles 2004-12-13 08:50:41 PST
For anyone interested:

Thunderbird 1.0 with the 'Reset Quote Headers' extension now provides this
functionality, as does the 'QuickReply' extension, each with one caveat:

'Reset Quote Headers' works, but only if you are replying from the main window, and

'QuickReply' works only if replying from the QuickReply textbox.

Charles
Comment 52 Charles 2004-12-13 08:51:41 PST
For anyone interested:

Thunderbird 1.0 with the 'Reset Quote Headers' extension now provides this
functionality, as does the 'QuickReply' extension, each with one caveat:

'Reset Quote Headers' works, but only if you are replying from the main window, and

'QuickReply' works only if replying from the QuickReply textbox.

Charles
Comment 53 Joseph Delaney 2004-12-13 11:13:07 PST
(In reply to comment #52)
> Thunderbird 1.0 with the 'Reset Quote Headers' extension now provides this
> functionality, as does the 'QuickReply' extension, each with one caveat:

OK, I can find QuickReply at http://quickreply.mozdev.org/ (which does not
appear on the "active projects" list) but can't find "Reset Quote Headers"
anywhere. Link?
Comment 54 Charles 2004-12-13 11:25:17 PST
My apologies - should have procided links...

QuickReply (repackaged for TBird 1.0):
www.extensionsmirror.nl/extthunderbird/QuickReply_0.5.5_bump.xpi

Reset Quote Headers:
www.extensionsmirror.nl/extthunderbird/Reset_Quote_Header_0.2.8.xpi

www.extensionmirror.nl is a great place for extensions, if you didn't already
know...

Charles
Comment 55 Hiro 2005-02-08 20:05:28 PST
*** Bug 281400 has been marked as a duplicate of this bug. ***
Comment 56 Jo Hermans 2005-07-06 08:29:43 PDT
*** Bug 299823 has been marked as a duplicate of this bug. ***
Comment 57 Adam Guthrie 2005-08-22 20:18:25 PDT
*** Bug 305549 has been marked as a duplicate of this bug. ***
Comment 58 Mike Morris 2005-09-27 01:13:43 PDT
(In reply to comment #54)
> 
> QuickReply (repackaged for TBird 1.0):
> www.extensionsmirror.nl/extthunderbird/QuickReply_0.5.5_bump.xpi

IMHO "quickreply" is not exactly the same as the original (now over 5 years
old!) proposal, though there is overlapping functionality.  Would be much better
to see this as part of the mailclient core.
Comment 59 Charles 2005-09-27 05:56:29 PDT
You are correct - however, there is a new extension that I like even better, and
*is* much closer to what the original requst was asking for:

http://quickquote.mozdev.org/installation.html
Comment 60 Magnus Melin 2005-10-12 10:43:16 PDT
*** Bug 312198 has been marked as a duplicate of this bug. ***
Comment 61 Charles 2005-10-12 11:41:17 PDT
Is this EVER going to make into the core code???????
Comment 62 Karsten Düsterloh 2005-10-12 14:37:48 PDT
Yes. The new century has only just begun. :-P
Comment 63 benimnetz 2006-01-06 09:40:45 PST
That would be a nice feature, i think its kinda shame that this still doesnt work. I really love how this is implemented in sylpheed!
Comment 64 Charles 2006-01-06 12:07:58 PST
QuickQuote is now even better - you can integrate its functionality with the 'Reply' and 'Reply All' toolbar buttons.

http://quickquote.mozdev.org/installation.html

So, install the extension, go to the options and check the boxes to integrate with the toolbar buttons, highlight some text in a message, and click a button.

It still isn't perfect, but it's getting there.

Hopefully this will get integrated into the core code soon. Then I'll only be complaining about the lack of a proper Siganture Manager (yes, I am aware of the two extensions (Signature and Signature_Switch) for this, but they don't work very well.
Comment 65 Boris Kirkorowicz 2006-01-06 13:09:16 PST
(In reply to comment #64)
> http://quickquote.mozdev.org/installation.html

Install fails in Mozilla; seems only to work with Firefox/Thunderbird?
Comment 66 Fudd 2006-01-26 20:18:01 PST
Hi folks. This comment may seem a little out of context because it's not a complaint or even an observation about the progress of this project. I think the extension is great and it works well for me.

Fact is, I've been searching for some workaround to function as a REPLY-USING-TEMPLATE. As you know, templates in the current TBird are limited and awkward. This is where the search has led, to QuickQuote, because it has all the basic functions to reply to a message in a concise way. The only thing it really needs to provide users with a functional templates system is the ability to specify a file or template target into which the quoted text can be inserted (and to push a bit further- quote entire content). 

It's not evident when TB will get around to implementing a full templates system, but it seems to me and many of the forum posts made clear that templates are a primary function which should have been addressed long ago. 

So consider this a fervent appeal to broaden this extension a little, and gain a lot more utility. Many thanks for all your hard work. QuickQuote is great.
Comment 67 Bjoern Sjut 2006-01-27 04:41:01 PST
Well,

Im not sure if this is the right place to talk about the Quickquote extension. 

That's why I posted your comment at the author's homepage in the bugs section:

http://quickquote.mozdev.org/bugs.html

Best, Bjoern
Comment 68 Karsten Düsterloh 2006-02-13 03:33:14 PST
*** Bug 326992 has been marked as a duplicate of this bug. ***
Comment 69 rabbitambulance 2006-03-23 04:51:09 PST
"Reset Quote Header" stopped working with 1.5, which is really annoying, for several reasons.
BTW, does anyone know if there's a bug for the inclusion of somewhat more verbose Reply headers? The normal ones in TBird make replied mails pretty useless in a professional setting (at least at my workplace).
Comment 70 Charles 2006-03-23 08:18:57 PST
You can change the Quote HEader with QuickQuote as well... it really does do a very good job of fulfilling this feature request... check it out:

http://quickquote.mozdev.org/
Comment 71 vhsgg70bvulosu7 2006-04-12 02:35:58 PDT
I am not a programmer, but is this feature so hard to implement?
Comment 72 Ian P. Christian 2006-07-18 06:43:48 PDT
With regards to : http://quickquote.mozdev.org/

It does an *ok* job of providing the behaviour - it doens't put the text cursor at the bottom of the mail (encouraging top posting, bad bad)

This should be built into the mail client, as the quickquote thing seems like a bit of a hack, it doesnt' replace the reply feature, it creates it's own, and you can see it selecting the main window, highlighting hte text, and presumably grabbing it and throwing it into the compose windows. (no offence to the author of this extention, you're doing a great job with what you have to work with I'm sure)


Comment 73 Jo Hermans 2006-09-22 15:48:26 PDT
*** Bug 353829 has been marked as a duplicate of this bug. ***
Comment 74 (not reading, please use seth@sspitzer.org instead) 2006-09-22 15:52:08 PDT
I'm not working on this, but would really like to see it!
Comment 75 Jo Hermans 2006-11-21 15:20:02 PST
*** Bug 361435 has been marked as a duplicate of this bug. ***
Comment 76 Doug Lerner 2007-01-21 22:36:19 PST
I notice it's been SEVEN YEARS since people have requested a built-in solution to this. I hope somebody takes it up sometime! :)

doug
Comment 77 Phil Ringnalda (:philor) 2007-05-23 22:23:48 PDT
*** Bug 381818 has been marked as a duplicate of this bug. ***
Comment 78 jslavin 2007-08-30 12:48:53 PDT
This is essential.
Comment 79 jslavin 2007-08-30 12:49:46 PDT
This is essential.
Comment 80 Jeff Beckley 2008-01-21 15:36:16 PST
Created attachment 298363 [details] [diff] [review]
Implementation of reply quote selected

Here's an implementation of this feature.  It adds two new prefs.  "mailnews.reply_quoting_selection" controls whether or not the feature happens or not.  "mailnews.reply_quoting_selection.only_if_chars" is a list of characters, one of which must be present in the selection in order to quote just the selection on the reply.  The default value for the pref is newline, period, exclamation point, and quotation mark.  This is done so that other types of selection (e.g. find a word in a message) don't cause reply quoting selection to kick in.
Comment 81 Charles 2008-01-22 07:16:06 PST
Wow... excellent!! I'm really glad to finally see some action on this one... hopefully the patch is acceptable to the ones who decide whether or not to include it...
Comment 82 Ben Bucksch (:BenB) 2008-01-22 17:52:45 PST
Thanks for implementing this important feature. Just looked at the patch superficially, but it looks very good (small and clean).
Comment 83 David :Bienvenu 2008-01-22 18:01:35 PST
yeah, thx Jeff, it looks pretty good. I saw what looks like a tab:

+  {
+	nsString selPlain;

and the Lossy convert always raises a red flag, but it's probably ok for this. I need to try running with it, though.
Comment 84 Jeff Beckley 2008-01-22 21:35:26 PST
Oh, didn't notice the tab.  I try to keep those out, but it looks like one snuck in.

Yeah, the "lossy" bugged me at first, but I just thought users are not going to want to have anything but sentence-ending punctuation in that list.  I suppose I could have used UTF-8, but it seemed unnecessary.
Comment 85 Magnus Melin 2008-01-22 22:41:29 PST
Hmm... http://en.wikipedia.org/wiki/East_Asian_Punctuation
Comment 86 Jeff Beckley 2008-01-23 13:20:22 PST
Magnus, thanks for pointing that out.  So I'll change it to UTF-8.

Anyone know of a strcspn()-equivalent for UTF-8 strings?  That would be the cleanest, although it's not that much code to do it myself.
Comment 87 Karsten Düsterloh 2008-01-23 14:35:11 PST
While your tinkering, remember changing the IDL's UUID. ;-)
Comment 88 Jeff Beckley 2008-01-25 09:41:11 PST
Created attachment 299227 [details] [diff] [review]
New patch from comments

Here's a new whole patch that address David, Magnus, and Karsten's concerns:

- Removed tabs
- Changed character list to use UTF-8 so East Asian punctuation can be handled
- Changed the UID on nsIMsgComposeParams.idl

Thanks guys!
Comment 89 David :Bienvenu 2008-01-27 17:08:18 PST
Comment on attachment 299227 [details] [diff] [review]
New patch from comments

if you can, +  attribute string                origQuoteHTML;

can you try using an ACString instead? That's currently preferred...

can you remove 

 pref("mail.imap.check_deleted_before_expunge", false);
+pref("mail.imap.expunge_option",            0);
+pref("mail.imap.expunge_threshold_number",  20);

from your patch?

With those changes, r=bienvenu. I'm glad this approach worked out!
Comment 90 jwq 2008-01-28 02:16:42 PST
 I'm glad to see this bug addressed, but I have a concern that the behaviour is made unnecessarily complex by "mailnews.reply_quoting_selection.only_if_chars".  

 Having the whole message quoted if there is no selection, and only the selection if one exists, is simple and intuitive behaviour. Having the selection quoted only under some circumstances, which the user isn't told about at the beginning, will be confusing at best (and downright annoying, typically). The principle of least surprise argues for the simple behaviour.

 Guessing which selections the user wants or doesn't want quoted in a reply is a losing game. Who can say that the user doesn't want a selection not containing "\n.!?" for quoting in a reply? For example, the current default will not quote selections which are sub-clauses with comma or semicolon punctuation, nor text in double quotes where the final punctuation mark is placed outside the closing double quote. Who can say that the user doesn't want a selection from word search to be prevented being quoted in a reply.

 As simple "quote only the selection if it exists" behaviour is least confusing, and doesn't prevent the user from obtaining selected text quoting they might legitimately expect to obtain, I ask that "mailnews.reply_quoting_selection.only_if_chars" and associated behaviour be dropped from the fix to this bug.
Comment 91 Mattias Jiderhamn 2008-01-28 04:36:41 PST
(In reply to comment #90)
>  Having the whole message quoted if there is no selection, and only the
> selection if one exists, is simple and intuitive behaviour.

I agree with comment #90. The only exception would be if the selection contains nothing but white space (potentially at different quote levels).

I believe this is how Eudora (<=7) does it; if only a single word is selected, that would be the reply quote.
Comment 92 Ben Bucksch (:BenB) 2008-01-28 06:42:14 PST
> I'm glad to see this bug addressed, but I have a concern that the behaviour is
> made unnecessarily complex by "mailnews.reply_quoting_selection.only_if_chars".

Completely agreed. I suggest to remove the pref.

The only exception I *could* see would be whitespace only and a single word being selected, because this can happen with just a double-click and may not be intended. I would solve the Japanese problem by saying: If selection contains only whitespace or selection contains only [a-ZA-Z] (i.e. only latin characters, no umlauts, special chars or CJK chars), then don't use the selection. Otherwise use it.
Comment 93 Ben Bucksch (:BenB) 2008-01-28 06:43:47 PST
> If selection contains only whitespace or selection contains only [a-ZA-Z]

Note: These are 2 different cases. If selection contains latin and whitespace, use it.
Comment 94 Jeff Beckley 2008-01-28 12:26:34 PST
(In reply to comment #90)
> Guessing which selections the user wants or doesn't want quoted in a reply is
> a losing game.

Yes, it is a heuristic, but it's one we found that worked very well in Eudora.  It fails much less than it succeeds.  I'd like to see the pref remain.  I suppose we could default it to be empty, which would indicate all selections are quoted, and then users could change it to what they want.


(In reply to comment #92)
> The only exception I *could* see would be whitespace only and a single word
> being selected

This could be a compromise.  The most common misfire on reply quoting selection is when the user searches for a word and then replies afterward.  The algorithm could be to remove leading and trailing whitespace from the selection, and then look for only non-East Asian characters in what remains.  This could be tied to a pref as well, which would be on by default.


The user then could decide to use and combination of these prefs.  And there is the global pref to turn off the feature entirely.
Comment 95 Ben Bucksch (:BenB) 2008-01-28 17:18:43 PST
FWIW, the case that I'm thinking of that the current code (much less the default pref) does not cover is:
I do want to be able to quote just "hello world", but I don't want it to trigger for 1) whitespace only, 2) on a single word selection (double click) or 3) a part of a word (search, as you mentioned). The algorithm "trim leading/trailing whitespace, check that result contains something apart from a-z" (space => success) would achieve that. If you want to pref it, maybe allow to put a regexp in the pref? The above would be (if my regexp foo does not fail me) selection.match(/\s*[^a-zA-Z]+\s*/).
Comment 96 Ben Bucksch (:BenB) 2008-01-28 17:26:51 PST
> If you want to pref it, maybe allow to put a regexp in the pref?

Oh, you're in C++, and regexps are hard from there (bug 48179, bug 348642).
Comment 97 Jeff Beckley 2008-01-29 17:27:30 PST
Created attachment 300204 [details] [diff] [review]
Now with multi-word check

Changes based on latest comments:

- Use of ACString instead of string in IDL
- Extraneous prefs removed
- Addition of "mailnews.reply_quoting_selection.multi_word" pref
- Default value of "mailnews.reply_quoting_selection.only_if_chars" now empty

"mailnews.reply_quoting_selection.multi_word" is a boolean pref that controls whether there have to be multiple words in the selection in order to quote just it on the reply (defaults to on).

I'd appreciate comments from i18n folks on whether my technique for determining multiple words in the selection is okay (see nsMsgComposeService::GetOrigWindowSelection), which is basically remove leading and trailing spaces, and then look to see if the selection contains only non-space single and 2-byte UTF-8 chars.
Comment 98 Magnus Melin 2008-01-30 13:07:39 PST
Comment on attachment 300204 [details] [diff] [review]
Now with multi-word check

I only do /mail reviews.   Looks good to me though, unfortunately the patch has bit rot already:(

Since it needs an update anyway, remove the double empty line, put a . after the nsresult GetOrigWindowSelection documentation sentence. And I hear the ns(C)AutoString versions is preferred for local strings - someone correct me if I'm wrong.

Thanks for working on this though, it really is *the* feature I like in eudora!
Comment 99 Phil Ringnalda (:philor) 2008-02-22 09:40:21 PST
*** Bug 419063 has been marked as a duplicate of this bug. ***
Comment 100 Jeff Beckley 2008-03-06 10:47:45 PST
Created attachment 307762 [details] [diff] [review]
Un bit-rotted and fixed reply all

This new patch is now using recent file revisions for diffs, and it addresses Magnus' comments.

It also fixes a bug where Reply All wasn't getting all of the recipients in the reply.
Comment 101 Reed Loden [:reed] (use needinfo?) 2008-03-16 13:53:09 PDT
Don't you need superreview for this? bienvenu can do r+sr, but please ask him to mark it as such if you want him to... else get somebody like mscott to sr= it.
Comment 102 Reed Loden [:reed] (use needinfo?) 2008-03-16 13:53:37 PDT
Re-add checkin-needed when you have all required reviews.
Comment 103 Tuukka Tolvanen (sp3000) 2008-03-19 10:41:47 PDT
*** Bug 423904 has been marked as a duplicate of this bug. ***
Comment 104 Dan Mosedale (:dmose) 2008-03-26 17:26:00 PDT
Comment on attachment 307762 [details] [diff] [review]
Un bit-rotted and fixed reply all

>+  attribute ACString              origQuoteHTML;

This name seems non-obvious on first reading.  htmlToQuote would be a bit clearer; there might be something clearer still.

Please add doxygen commentary here describing exactly what this new attribute means and how it is used.  directory/xpcom/public and calendar/public include examples of this sort of thing.

>+    mIgnoreData = PR_FALSE;

Instead of this mIgnoreData stuff, have a look at the call to mQuote->QuoteMessage in nsMsgCompose::QuoteMessage.  Currently, it's always called with a headersOnly value of PR_FALSE.  If you can tweak that code so that it gets called with headersOnly set to PR_TRUE in the case where we've got a selection, it should be possible to get rid of mIgnoreData.

Please also file a bug about disentraining the header setup in replies from the quoting, as having to call mQuote->QuoteMessage for the header-setting side effects is bogus.
Comment 105 Jeff Beckley 2008-04-04 09:53:55 PDT
Created attachment 313632 [details] [diff] [review]
New patch addressing dmose's comments

Here's a new patch incorporating the changes that dmose suggested:
- Renamed origQuoteHTML to htmlToQuote
- Reworked the code to get rid of mIgnoreData
- Added doxygen-style comment to the new htmlToQuote attribute
Comment 106 Dan Mosedale (:dmose) 2008-05-29 13:44:45 PDT
Comment on attachment 313632 [details] [diff] [review]
New patch addressing dmose's comments

Looks good.  sr=dmose, conditional on getting input from some i18n folks on the UTF8 stuff, as well as tweaking a few nits:

>+
>+  /**
>+   * HTML-formatted content to quote in the body of the message.
>+   * Set this to get different content than what would normally
>+   * appear in the body, e.g. the original message body in a reply.
>+   */
>+  attribute ACString              htmlToQuote;
> }

htmlToQuote is encoded in UTF8, so this should be AUTF8String.  Otherwise if someone tries to access it from JS, bad things will happen.

> NS_IMETHODIMP QuotingOutputStreamListener::OnStopRequest(nsIRequest *request, nsISupports * /* ctxt */, nsresult status)
> {
>   nsresult rv = NS_OK;
>   nsAutoString aCharset;
> 
>+  if (!mHtmlToQuote.IsEmpty())
>+  {
>+    // If we had a selection in the original message to quote, we can add
>+    // it now that we are done ignoring the original body of the message
>+    nsCOMPtr<nsIInputStream> stream;
>+    rv = NS_NewCStringInputStream(getter_AddRefs(stream), mHtmlToQuote);
>+    NS_ENSURE_SUCCESS(rv, rv);
>+
>+    mHeadersOnly = PR_FALSE;
>+    rv = OnDataAvailable(nsnull, nsnull, stream, 0, mHtmlToQuote.Length());

OnDataAvailable doesn't currently make use of the first two params, but at some point it might want to.  Better to pass them through rather than handing in nsnull, I'd think.

>+nsresult
>+nsMsgComposeService::GetOrigWindowSelection(MSG_ComposeType type, nsIMsgWindow *aMsgWindow, nsACString& aSelHTML)
>+{
>+  nsresult rv;
>+
>+  // Good hygiene
>+  aSelHTML.Truncate();
>+
>+  // Get the pref to see if we even should do reply quoting selection
>+  nsCOMPtr<nsIPrefBranch> prefs(do_GetService(NS_PREFSERVICE_CONTRACTID));
>+  NS_ENSURE_TRUE(prefs, NS_ERROR_FAILURE);

Here and elsewhere, please use the two argument form of do_GetService and propagate rv up the stack.  In some situations, it will make debugging problems after this returns easier.


>+  nsCOMPtr<nsIDocShell> docShell(do_QueryInterface(childAsItem));
>+  NS_ENSURE_TRUE(docShell, NS_ERROR_FAILURE);
>+  nsCOMPtr<nsIContentViewer> contentViewer;
>+  rv = docShell->GetContentViewer(getter_AddRefs(contentViewer));
>+  NS_ENSURE_SUCCESS(rv, rv);
>+
>+  nsCOMPtr<nsIDOMDocument> domDocument;
>+  rv = contentViewer->GetDOMDocument(getter_AddRefs(domDocument));
>+  NS_ENSURE_SUCCESS(rv, rv);

Seems like the code would be slightly clearer on first reading if the above hunk were moved to just before where domDocument is actually used.

>+      for (; i != end; ++i)
>+      {
>+        char c = *i;

If you can, prefer |const char c| for optimizer hinting goodness...
Comment 107 Dan Mosedale (:dmose) 2008-05-29 13:50:06 PDT
Adding m_wada and jshin in the hopes that one of them can proofread the UTF8 stuff in GetOrigWindowSelection() in this patch.
Comment 108 Jeff Beckley 2008-06-02 22:14:24 PDT
Created attachment 323502 [details] [diff] [review]
Changes addressing dmose's latest comments

Fixes per dmose's latest comments.  Continuing his sr+.

Waiting for comments from m-wada and/or jshin.
Comment 109 Magnus Melin 2008-06-04 00:31:18 PDT
I don't think m-wada usually does code reviewing, and a quick bugzilla query on jshin doesn't show much activity within the last year. 

m-wada, jshin: please comment here if you intend to look at the patch within any reasonable time. If not I think we should get this landed. Really looking forward to it!
Comment 110 WADA 2008-06-04 00:56:14 PDT
Magnus, you are right. I have no experience of code review/patch creation at bugzilla.mozilla.org.
Comment 111 Dan Mosedale (:dmose) 2008-06-05 17:55:35 PDT
Comment on attachment 323502 [details] [diff] [review]
Changes addressing dmose's latest comments

smontagu's i18n fu is very strong.  Simon, all I'm really looking for here is review of the |if (requireMultipleWords)| clause of nsMsgComposeService::GetOrigWindowSelection()
Comment 112 Simon Montagu :smontagu 2008-06-06 08:26:07 PDT
Comment on attachment 323502 [details] [diff] [review]
Changes addressing dmose's latest comments

>+      for (; i != end; ++i)
>+      {
>+        const char c = *i;
>+        if (c == ' ' || (!UTF8traits::isASCII(c) && !UTF8traits::is2byte(c) && !UTF8traits::isInSeq(c)))
>+          break;
>+      }

Please document in code what you are doing here and why. It made no sense to me until I read back in the bug.

(In reply to comment #97)
> I'd appreciate comments from i18n folks on whether my technique for determining
> multiple words in the selection is okay (see
> nsMsgComposeService::GetOrigWindowSelection), which is basically remove leading
> and trailing spaces, and then look to see if the selection contains only
> non-space single and 2-byte UTF-8 chars.
> 

Single and 2-byte UTF-8 chars just means "unicode values < U+0800", so you could just test for that in the UTF-16 string without converting to UTF-8. But the test doesn't really do what you want. It will break in all Indic languages and with a bunch of commonly used punctuation, e.g. smart quotes. I think what you need to do here is use nsIWordBreaker::FindWord.
Comment 113 Jeff Beckley 2008-06-06 11:46:38 PDT
(In reply to comment #112)
> I think what you need to do here is use nsIWordBreaker::FindWord.

The only concrete instance I see of nsIWordBreaker in all of Mozilla code is nsSampleWordBreaker.  Is it okay to use that in production code?  Is there another concrete instance I'm missing?

Do all languages separate words by spaces?  That's true for latin-derived languages, but what about others?  Yes, there is punctuation to mark the end of a word, but before the next word there must be a space.  Could I simply iterate through a UTF-16 string looking for spaces?
Comment 114 Garth Wallace 2008-06-06 15:39:13 PDT
(In reply to comment #113)
> Do all languages separate words by spaces?

No. Japanese, for instance, typically does not use interword space at all. Word breaks are between kana and kanji, or at word separator dots between words written just in katakana.
Comment 115 Simon Montagu :smontagu 2008-06-08 04:20:20 PDT
(In reply to comment #113)
> The only concrete instance I see of nsIWordBreaker in all of Mozilla code is
> nsSampleWordBreaker.  Is it okay to use that in production code?  Is there
> another concrete instance I'm missing?

I thought that we call into platform dependent routines (Uniscribe/Pango/Carbon), but I may be wrong. Roc probably knows better than I.
Comment 116 Robert O'Callahan (:roc) (email my personal email if necessary) 2008-06-08 15:15:05 PDT
We use platform dependent calls combined with custom code to determine *line* break opportunities. *Word* breaking is used for word-based caret movement (e.g. ctrl-leftarrow).

You could use either here, really.
Comment 117 Jeff Beckley 2008-06-10 00:24:57 PDT
Created attachment 324409 [details] [diff] [review]
More robust word detection

Here's a new version that uses nsILineBreaker to detect words.

It also fixes bit-rot on a couple of files.

Continuing dmose's sr+, but waiting for r+ by smontagu.
Comment 118 Simon Montagu :smontagu 2008-06-10 22:55:48 PDT
Comment on attachment 324409 [details] [diff] [review]
More robust word detection

Looks good to me.
Comment 119 Mark Banner (:standard8) 2008-06-11 00:55:58 PDT
I had a bit of trouble with the nsIMsgComposeParams.idl change not applying, couldn't see why, so I just applied it by hand as it was simple.

Checking in mailnews/compose/public/nsIMsgComposeParams.idl;
/cvsroot/mozilla/mailnews/compose/public/nsIMsgComposeParams.idl,v  <--  nsIMsgComposeParams.idl
new revision: 1.10; previous revision: 1.9
done
Checking in mailnews/compose/src/Makefile.in;
/cvsroot/mozilla/mailnews/compose/src/Makefile.in,v  <--  Makefile.in
new revision: 1.92; previous revision: 1.91
done
Checking in mailnews/compose/src/nsMsgCompose.cpp;
/cvsroot/mozilla/mailnews/compose/src/nsMsgCompose.cpp,v  <--  nsMsgCompose.cpp
new revision: 1.568; previous revision: 1.567
done
Checking in mailnews/compose/src/nsMsgCompose.h;
/cvsroot/mozilla/mailnews/compose/src/nsMsgCompose.h,v  <--  nsMsgCompose.h
new revision: 1.120; previous revision: 1.119
done
Checking in mailnews/compose/src/nsMsgComposeParams.cpp;
/cvsroot/mozilla/mailnews/compose/src/nsMsgComposeParams.cpp,v  <--  nsMsgComposeParams.cpp
new revision: 1.12; previous revision: 1.11
done
Checking in mailnews/compose/src/nsMsgComposeParams.h;
/cvsroot/mozilla/mailnews/compose/src/nsMsgComposeParams.h,v  <--  nsMsgComposeParams.h
new revision: 1.10; previous revision: 1.9
done
Checking in mailnews/compose/src/nsMsgComposeService.cpp;
/cvsroot/mozilla/mailnews/compose/src/nsMsgComposeService.cpp,v  <--  nsMsgComposeService.cpp
new revision: 1.146; previous revision: 1.145
done
Checking in mailnews/compose/src/nsMsgComposeService.h;
/cvsroot/mozilla/mailnews/compose/src/nsMsgComposeService.h,v  <--  nsMsgComposeService.h
new revision: 1.26; previous revision: 1.25
done
Checking in mailnews/mailnews.js;
/cvsroot/mozilla/mailnews/mailnews.js,v  <--  mailnews.js
new revision: 3.315; previous revision: 3.314
done
Comment 120 Charles 2008-06-11 04:59:00 PDT
Wow, this is awesome!

Now, one question - for those of us who don't know the full dev process for TBird...

Will this new functionality now show up in the nightly builds? Will it show up in a coming 2.0.x release? Or will it be limited to the 3.0.x builds?

Much appreciation to Jeff and the others who made this happen!

Charles
Comment 121 Jeff Beckley 2008-06-11 09:57:28 PDT
(In reply to comment #120)
> Will this new functionality now show up in the nightly builds? Will it show up
> in a coming 2.0.x release? Or will it be limited to the 3.0.x builds?

This is feature is only in 3.0 at the moment.  That doesn't prevent someone from backporting to 2.0.x, but it's very unlikely you're going to see any new features go in to a 2.0.x release.

So, it will show up in nightlies and will be in the 3.0a2 release.
Comment 122 u41494 2008-06-11 11:14:48 PDT
I voted for this bug many years ago, now is fixed... Thank you Jeff :)
Comment 123 Dan Mosedale (:dmose) 2008-06-17 14:13:38 PDT
Jeff, can you file a spinoff bug as per comment 104 and link to it here?
Comment 124 Jeff Beckley 2008-06-17 16:26:41 PDT
(In reply to comment #123)
> Jeff, can you file a spinoff bug as per comment 104 and link to it here?

Bug 439758.
Comment 125 Phil Ringnalda (:philor) 2009-04-16 00:13:51 PDT
*** Bug 488642 has been marked as a duplicate of this bug. ***
Comment 126 Nathan 2009-12-17 06:32:15 PST
Is there a way to disable this feature via a preference?

When reading an email, I often select sections while reading.  Then when I reply, I only have part of the message.  I know that I can un-select before replying, but I do not see me ever wanting this behavior.

Thanks.
Comment 127 u41494 2009-12-17 07:02:48 PST
(In reply to comment #126)
> Is there a way to disable this feature via a preference?

You have to set "mailnews.reply_quoting_selection" to false in about:config
Comment 128 Bjoern Sjut 2010-01-06 13:09:24 PST
Thank you, thank you, thank you, everyone: It's great to have this feature working in TB3!
Comment 129 David :Bienvenu 2010-01-06 13:38:47 PST
I'm going to take that as "verified" :-)
Comment 130 Nathan Tuggy (:tuggyne) 2010-01-29 17:42:31 PST
*** Bug 543152 has been marked as a duplicate of this bug. ***
Comment 131 Markus Multrus 2010-06-04 02:18:48 PDT
Is there the possibility to add a GUI preference, to turn this "feature" (or should I say annoyance) off, or alternative disable this again by default? Fiddling in the about:config is not very comfortable, and it took me hours to find the good key...
Comment 132 Josh Feingold 2012-04-11 12:04:54 PDT
Totally agree.  This is a very annoying "feature".  In fact, I didn't even realize that the selected text was what was being included in the reply for a few days.  It just seemed messed up.

Note You need to log in before you can comment on or make changes to this bug.