466330 - "Quote Message" quotes the currently highlighted message - not the replied-to message

Status: RESOLVED WONTFIX

(Thunderbird :: Message Compose Window, enhancement)

Description

[Erez Schatz]

User-Agent:       Opera/9.62 (Windows NT 5.1; U; en) Presto/2.1.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20081006 Shredder/3.0a3

When using "quote message" menu item, the currently highlighted message is qouted, rather than a more appropriate message (if the mail is a reply-to, or forward). 

Reproducible: Always

Steps to Reproduce:
1. click "reply" on any message
2. in the main Thunderbird window, click on another message
3. at the compose window, click Options> Quote message
Actual Results:  
The highlighted message will be quoted

Expected Results:  
The replied-to message should be quoted

This is the way Thunderbird have behaved since probably always.

Comment 1

[rsx11m]

That was introduced on purpose by bug 23394 when highlighting a text of the original message and then clicking on Reply, thus the same also applies to "Quote message" from the menu.

I'd expect this to resolve INVALID or WONTFIX as the change was on purpose.

Comment 2

[Magnus Melin [:mkmelin]]

->WONTFIX

Comment 3

[Ben Bucksch (:BenB)]

I think this is a valid bug. It says that the
Computer | menu | Options | Quote message
feature is essentially broken now. It's fairly likely that I opened another mail since I hit Reply and edited my mail.

I think it should either be fixed to quote the replied-to message (as it was intended and always did), or entirely be removed.
Note that we already have a
Edit | Paste as quotation
for quoting other mails and sources (it just won't insert the "Fred wrote:"), so the "Options | Quote message" as-is provides no useful functionality anymore, unless it's fixed.

Comment 4

[Jeff Beckley]

This actually has nothing to do with Reply Quoting Selection.  This bug was
there in TB 2.0, before the RQS code has added.

If you look at the code in
mailnews/compose/resources/content/MsgComposeCommands.js,
QuoteSelectedMessage() calls GetSelectedMessages(), which gets the selected
message from the most recent mailbox window.  What needs to happen is that the
original message used to create the reply should be used in this case.  I don't
think that original message URI is saved around, and it might be complicated to
save on a permanent basis (i.e. when the reply message is saved and closed and
then reopened), but saving it in memory would be easy.

I'll take a look at this.

Comment 5

[Jeff Beckley]

Attachment: Fix

Here's changes for both Thunderbird and SeaMonkey.  When the response message is created, the original message URI is saved.  That URI is used to as the basis for quoting.  If the original message URI is not present, then it falls back on the old behavior of using the selected message of the topmost mailbox window.  This is to preserve old functionality, like creating a new message and then quoting several selected messages in a mailbox (a kind of mass forward in one message).

Comment 6

[Jeff Beckley]

Comment on attachment 367280 [details] [diff] [review]
Fix

So the question is whether we want the old functionality (quoting always quotes the selected messages) or the patch behavior (quoting quotes the original message if a reply/forward, but selected messages if not).  Adding clarkbw as referee.

Comment 7

[Phil Ringnalda (:philor)]

The answer to my lunchtime question is that this would be a duplicate of bug 181028 (or rather, what it morphed into), except that it's not a core issue: as your patch makes clear, the core is quite capable of supporting any sort of UI that either application wants, and I'm not at all sure that they'll wind up wanting the same thing. Reasonable SeaMonkey UI would be a "Quote Original Message" menuitem that's disabled/hidden when you aren't replying, and a "Quote Selected Message(s)" menuitem that's disabled/hidden when nothing's selected; I wouldn't be terribly surprised if Thunderbird UI winds up being "eh, it's too confusing and unpredictable and power-user-oriented, rip the whole thing out and make someone write an extension to replace it."

Comment 8

[Ben Bucksch (:BenB)]

As said in comment 3, 'quote selected message' functionality would have too much overlay with "Edit | Paste as quotation" to warrant being an extra function, therefore I think your patch is desirable, and thanks for taking the time to write it.

Comment 9

[Phil Ringnalda (:philor)]

The problem with comment 3 is that you started out from the wrong place: bug 70478 intentionally implemented exactly what we've had for almost seven years, a "quote selected messages," and did so because it was 4xp.

Argue that a feature which some people have been using happily for ten years or more shouldn't exist because they can do the same thing worse with much more effort another way, that's fine, but don't argue that it's a regression that was accidentally introduced sometime recently.

Comment 10

[Jeff Beckley]

Paste as quotation is different in that it works with whatever text is on the
clipboard.  Since it doesn't know if that clipboard text is from a message or
not, or if it was which message it was from, then it can't add an attribution
line ("On <date/time>, <sender> wrote:").

In ruminating about this some more, I think we should leave the old behavior
and change the tooltip to "Quote currently selected message(s)".  Having it
quote the original message in a reply/forward is fairly useless as almost all
of the time the original message is already quoted in there.  If we really
wanted to we could create a new menu item to quote the original message, but I
don't think it's common enough to warrant it.  You can always get that behavior
by selecting the original message.

Comment 11

[Phil Ringnalda (:philor)]

Comment on attachment 367280 [details] [diff] [review]
Fix

I like the sound of what you're thinking now much better, but just in case we get UI marching orders that lead us back here, something in one of the other bugs about this made me try:

1. Click reply
2. Delete the replied-to message
3. "Quote Message"

which with this patch fails rather unprettily by inserting a "On 12/31/69 4:00 PM,  wrote:"

Comment 12

[Bryan Clark (DevTools PM) [:clarkbw]]

(In reply to comment #10)
> In ruminating about this some more, I think we should leave the old behavior
> and change the tooltip to "Quote currently selected message(s)".  Having it
> quote the original message in a reply/forward is fairly useless as almost all
> of the time the original message is already quoted in there.  If we really
> wanted to we could create a new menu item to quote the original message, but I
> don't think it's common enough to warrant it.  You can always get that behavior
> by selecting the original message.

This sounds like the right path to me.  And it should be a much simpler patch.

The text "Quote currently selected message(s)" is really long and awkward, but I don't have any better suggestions for the situation.

Comment 13

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 367280 [details] [diff] [review]
Fix

removing myself from this as well.

Comment 14

[Jens Müller (:tessarakt)]

(In reply to comment #10)
> Paste as quotation is different in that it works with whatever text is on the
> clipboard.  Since it doesn't know if that clipboard text is from a message or
> not, or if it was which message it was from, then it can't add an attribution
> line ("On <date/time>, <sender> wrote:").
> 
> In ruminating about this some more, I think we should leave the old behavior
> and change the tooltip to "Quote currently selected message(s)". 

Maybe you should, when copying text from a message to the clipboard, also save it with a special MIME type (or however clipboard content is typed), and remember it came from a message.

If so, you could offer a separate Paste as ... entry, or add attribution on "Paste as quotation", or whatever.

Comment 15

[Magnus Melin [:mkmelin]]

"Quote Message" must be one of the strangest features we have. And it's under Options while not an option. I think we should consider dropping it.

Comment 16

[Ben Bucksch (:BenB)]

The purpose of the feature was for people who don't quote by default on reply, but want to do it on demand.
You can do that (another way) by opening the to-be-quoted message, select text, copy, go back to editor and select Edit | Paste as quotation. Surely much more cumbersome, but works.

Comment 17

[Ben Bucksch (:BenB)]

Ooops, nevermind, I already said that in comment 3 and in comment 8. Duh.

Comment 18

[Magnus Melin [:mkmelin]]

At the very least it should move to the Edit menu.

Comment 19

[Ben Bucksch (:BenB)]

I don't think anybody would understand it in Edit, if we had both this and "Paste as Quotation".

I think this is indeed something we can remove from the menu.

Comment 20

[Wayne Mery (:wsmwk)]

Thomas, can you untangle or finish this?

Comment 21

[Thomas D. (:thomas8)]

(In reply to Wayne Mery (:wsmwk) from comment #20)

Thomas, can you untangle or finish this?

Sure, my pleasure!

TL;DR:
I think Magnus (comment 2) was right: The correct resolution here is WONTFIX.

Reasons:
This bug (per current summary) wants to reduce the current functionality of Quote (selected) message(s) to just Quote the original message replied-to. Net effects:

• substantial loss of flexibility:
  • no more convenient mass-quoting of multiple selected messages
  • no more quoting of selected messages in new compositions which are not replies
• very little benefit:
  • typically, the original message will be selected anyway when you reply, so you can easily use the current feature to reply to the original (selected) message. (Quote-on-reply is ON by default, so if you're having regular challenges with reply-quoting OFF, well, just switch it back on!)

Way forward:
I think we should just move the menuitem to a better place and polish some rough corners including the wrong tooltip on the Quote button (available from toolbar customization).

I have created a clean bug for that and provided a patch:
Bug 1689803 - Move "Quote [selected] Message" out of "Options" menu and improve menu label, button tooltip and enabled behaviour

The small print - in support of this resolution:
Cross-section of previous comments

(In reply to Phil Ringnalda (:philor) from comment #9)

bug 70478 intentionally implemented exactly what we've had for almost seven years, a "quote selected messages," and did so because it was 4xp.

Bug 70478 (10 duplicates) explicitly restored this feature as still seen in TB 78, because that's what users were asking for.
On the other hand, this bug 466330 (filed 7 years after the alleged "regression") does not have a single duplicate nor votes, and mostly skeptical comments here. As a matter of fact, it's actually an RFE.

(In reply to Jeff Beckley from comment #10)

I think we should leave the [current] behavior and change the [button] tooltip to "Quote currently selected message(s)".

+1.

Having it quote the original message in a reply/forward [as requested by this bug 466330] is fairly useless as almost all of the time the original message is already quoted in there. [....] You can always get that behavior by selecting the original message.

(In reply to Bryan Clark (then TB UX) [:clarkbw] from comment #12)

This [keeping the feature as it is now, quote any selected messages] sounds like the right path to me.

+1

(In reply to Magnus Melin [:mkmelin] from comment #18)

At the very least it [the "Quote Message" menuitem] should move to the Edit menu.

Yes, Options is definitely the wrong place for this command (undiscoverable), as all the other options are about settings for the message. I think the choices are Edit or Insert; imho the latter fits even better...

(In reply to rsx11m from comment #1)

I'd expect this to resolve INVALID or WONTFIX as the change was on purpose.

(In reply to Magnus Melin [:mkmelin] from comment #2)

->WONTFIX

+1

q.e.d.