359226 - Need Redirect option that allows editing before sending

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[Matt Dudziak]

Penelope needs a Redirect option and a resend (Send Again) option that works like the current Eudora and insert the correct 'resent' headers.

Comment 1

[Elliotte Rusty Harold]

This is related too and really a duplicate of 12916 
https://bugzilla.mozilla.org/show_bug.cgi?id=12916 

Comment 2

[David]

In addition, a Bounce option as elm has is needed as well.

Comment 3

[Cefe López]

It also needs to keep track of to whom a given msg was forwarded or redirected

Comment 4

[Mark Charlebois]

(In reply to comment #1)
> This is related too and really a duplicate of 12916 
> https://bugzilla.mozilla.org/show_bug.cgi?id=12916 
> 

That bug has not had comments on it since June 06. There has been an extension available since July 2004 but no new code since then. I looked at the extension and I did not find the implementation particularly easy to follow. The XUL is simple enough, but I couldn't figure out the purpose of some of the js files from a cursory read.

Comment 5

[David]

(In reply to comment #2)
> In addition, a Bounce option as elm has is needed as well.

 

I think <http://mailredirect.mozdev.org/index.html> may mimic the elm behaviour...

Comment 6

[Alan Rogers]

Forgive the cross-post, I submitted this on Bug 11034 which may not have been the most appropriate place.

Can anyone advise please where the development of bounce/resend is and whether
or not it is compatible with IMAP? I've used Eudora for years, because it does
redirect, and am now watching Penelope since Qualcomm announced the closure of
development/support on Eurdora.

Comment 7

[Elliotte Rusty Harold]

FYI, you probably already know this; but just in case you don't, the Tools/Redirect menu item is a no-op on 8.0b1 on the Mac.

Comment 8

[Matt Dudziak]

Yes, we know. Thanks. This bug is there to remind us to put the feature in. The menu item will likely be removed if we fail to implement this feature before the final release.

Thanks, Matt

Comment 9

[Jeff Beckley]

Attachment: Redirect implementation

Redirect feature implemented in a way very similar to Classic Eudora.  The message is basically performed as a Thunderbird Edit As New (which is the equivalent of Classic Eudora's Send Again), and the Reply-To: header is set to be the sender of the original message so that any replies to the redirected message get sent to the original sender.  This allows the user to edit the redirected message before sending, e.g. add additional comments.

This is different from the previously mentioned Mail Redirect extension, which does a resend that is compliant with RFC 2822.  That doesn't allow the message to be changed in any way (except the addition of a couple Resent-* headers), so additional comments aren't possible with it.

Comment 10

[David :Bienvenu]

I'll look at this as soon as I can...

Comment 11

[Magnus Melin [:mkmelin]]

Any reason not to implement it rfc 2822 compliant, except for compat with old eudora? If I read the description correctly, this patch is just a fancy forward + set reply-to fix. Not sure how much value that gives...

(BTW, this bug summary says proper resent headers...)

Comment 12

[Jeff Beckley]

(In reply to comment #11)
> Any reason not to implement it rfc 2822 compliant, except for compat with old
> eudora? If I read the description correctly, this patch is just a fancy forward
> + set reply-to fix. Not sure how much value that gives...
> 
> (BTW, this bug summary says proper resent headers...)

My bad for not editing the bug summary after it was decided we wanted something slightly different (now I have).  What is stated in the original description is actually impossible.  The RFC 2822 compliant way does not allow for additional comments/edits to be made by the redirector, which is what Classic Eudora has always done and users have expressed something that's wanted.

Like I mention in comment 9, the Mail Redirect add-on provides the RFC 2822 solution, for those that want that behavior.

Magnus, could you take a look at the patch to see if you see anything wrong with it, given what it's trying to implement?  Unfortunately, bugs in the Penelope component don't have the ability to assign reviewers.  Thanks!

Comment 13

[Jeff Beckley]

Attachment: Fix bit-rot

Comment 14

[Jeff Beckley]

Moving to product Thunderbird so it can get reviewed for that.

This is similar, but different to bug 12916.  This implementation of redirect allows the user to edit the body of the message before sending, whereas the RFC 2822 redirect doesn't allow editing.

Note that there are only backend changes to Thunderbird/SeaMonkey.  The UI changes are only to Penelope.

Comment 15

[Dan Mosedale (:dmosedale, :dmose)]

I'm slightly skeptical that the fact that this is a slight superset of what RFC 2822 somehow makes this a different bug, but I haven't actually read the RFC.

Why not do the UI changes in Thunderbird instead of Penelope?  I'd argue that it's very much worth having some permutation of this in Tb.

Comment 16

[Jeff Beckley]

(In reply to comment #15)
> I'm slightly skeptical that the fact that this is a slight superset of what RFC
> 2822 somehow makes this a different bug, but I haven't actually read the RFC.

The big difference is that the RFC, <http://tools.ietf.org/html/rfc2822#section-3.6.6>, requires that the redirected message be left identical to the original message, except for the addition of a few Resent-* headers, whereas Classic Eudora's implementation showed the original message in the body of the composition window and allowed the user to edit it (and didn't add Resent-* headers, as that would be a violation of the RFC).

The Mail Redirect extension, <https://addons.mozilla.org/en-US/thunderbird/addon/550>, already provides the RFC 2822 behavior, and so users wanting that can install the extension.


> Why not do the UI changes in Thunderbird instead of Penelope?  I'd argue that
> it's very much worth having some permutation of this in Tb.

Seemed like more folks in bug 12916 were wanting the RFC behavior, and so I took that as not wanting to be done in TB.  This patch will put the capability in to Thunderbird, and if someone decides that TB should actually expose some UI for it, then that can be done in a separate patch.

Comment 17

[Jeff Beckley]

Comment on attachment 324939 [details] [diff] [review]
Fix bit-rot

After discussion in #maildev, the question now is whether or not we need two menu items that have similar behavior.

There currently exists the Edit Message As New function, which allows the user to resend any message (received or sent) while changing only the From: to the account which is doing the sending.  All other headers and the body remain the same, although a compose window is opened up giving the user the ability to edit any of the contents.

This new Redirect function implemented in the patch is similar to Edit Message as New, but it adds a Reply-To: header with the address of the original sender and empties out the contents of the To: and Cc: headers.  The idea behind a Redirect is that the user is resending the message to someone else, but wants replies to go to the original sender.  In other words, it gets the person performing the redirect "out of the loop".  In doing the Redirect, it can be helpful for the redirector to edit the body of the message, for example to remove parts of the message that are thought not to be important for the new recipient to see.

Classic Eudora also has another feature related to this, which is Redirect To (affectionately called "Turbo Redirect").  Redirect To is a hierarchical menu that lists potential recipients.  The user selects Redirect To and then one of the recipients in the popup menu.  The Redirect feature as mentioned above is done, but the To: header is filled in with the selected recipient.  The composition message window is not shown to the user, and instead the message is sent right away.  This gives a method to easily redirect messages to a common recipient, and is a favorite of tech support providers.  It is planned that Redirect To will be implemented in Penelope, and can be added to Thunderbird as well, if so desired.

So do we need both Edit Message As New and Redirect menu items?  It is true that you can get the behavior of Redirect by using Edit Message As New, but it is much more time consuming.  You would have to copy the name and email address of the sender in the original message (no easy task itself, try it), select Message->Edit Message As New, add a Reply-To: header and paste in the original sender's name and address, and then clear out the To: and Cc: headers.

One possibility would be to do the separate functions from one menu item depending on what type of message is being acted upon.  In general you would only want to use the Edit Message As New functionality on outgoing messages (Sent/Drafts), and the Redirect functionality only on received messages.  I'm sure there are some users that could create use cases for the opposites, though.  It might be strange that you get different results from the same menu item, even though that's probably desired once you really think about it.

I vote for having the additional menu item for Redirect.  It gives the most flexibility and the least amount of surprise.  It's downside is obviously having two menu items instead of just one.


Opinions from others are welcome and encouraged.  In particular, I would love to get some feedback from clarkbw on this, so I added him for ui-review.

Comment 18

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

Ok, I've been looking into this for a little while now; sorry this has taken a bit longer than expected.

In some random user testing I get the sense your average person is not going to be able to understand the differences between the Edit Message as New, Forward, and Redirect menu items and their corresponding functional differences.  This is a bit of a problem.  Though I did have a problem finding a large enough number of people (2) who have used redirect before to be a relevant statistic (8). 

However I do believe that every person can much more easily recognize the format a message should be in when looking at how it is going to be sent.  That is to say if there were only one "forward" option and then in the composer window a person could toggle how they wanted the message forwarded (in terms of redirect, etc.) our success rate would be much higher.  ! Do not take take this the wrong way, that I'm suggestion we only have one forward option !  But in terms of getting the right thing done for the most amount of people I think a toggle in the compose window is the best change.

For another audience who already know they want a redirect (and even the turbo redirect) it seems like we'll want to have some kind of convenience method for getting to the redirect quickly.

<side note>
Just to throw this out there.  We will shortly be looking into using Ubiquity in Thunderbird to handle the type of turbo redirect functionality you're talking about.  Enabling people to act on a mail in a type-ahead system for converting to calendar appointments or redirecting to specific people.  Take a look at this post: http://www.toolness.com/wp/?p=54
</side note>

I don't like adding another menu item for such a similar functionality.  And the menu/toolbar is already filled with junk.  (i.e. the Forward as [ Inline / Attachment ] / Reply in [ HTML / Plaintext ])  These kind of options are format questions that should be answered when the default format was guessed incorrectly.  Good UI shouldn't require you to answer so many questions up front.

I'd love to hear comments on how to move forward (via irc, mail, or bugzilla) as I'm not inclined to go for this kind of change in the format it's currently in.  Is there a plan to assign a key combo to this menu item? (I'm not sure if any good ones are even available)

Comment 19

[Jeff Beckley]

Comment on attachment 324939 [details] [diff] [review]
Fix bit-rot

So after discussing this on #maildev, it was decided that we will forego the UI for Redirect in  Thunderbird for the time being.  Bryan gave some hints in his comment above about what the UI may be in the future.

This patch can now be reviewed and submitted since it has no UI change to Thunderbird (just the Penelope extension).  It includes the backend capability to perform Redirect, which can be exposed at a later time.

Comment 20

[Magnus Melin [:mkmelin]]

If it's not going to be an rfc compliant "redirect", should it be called something else, like "send again" or "re-send" in the code?

Comment 21

[Jeff Beckley]

(In reply to comment #20)
> If it's not going to be an rfc compliant "redirect", should it be called
> something else, like "send again" or "re-send" in the code?

It's OK as "Redirect".  The RFC, <http://tools.ietf.org/html/rfc2822#section-3.6.6>, doesn't call it anything in particular.  The headers are named Resent-*.

Comment 22

[Magnus Melin [:mkmelin]]

Ah, yes. We could use Resend for the RFC version then, when someone implements it. 

Comment 23

[Mark Banner (:standard8)]

Comment on attachment 324939 [details] [diff] [review]
Fix bit-rot

>Index: mailnews/compose/public/nsIMsgComposeParams.idl

>+    const long Redirect                 = 14;

You need to change the uuid for nsIMsgCompType as you're changing the interface.

Can you add a little bit of documentation for this option please? I think it would be worth noting what it does and that it is different from the rfc2822 case.

>+        case nsIMsgCompType::Redirect:
>+          {
>+            // For a redirect, set the Reply-To: header to what was in the original From: header...
>+            nsCAutoString author;
>+            msgHdr->GetAuthor(getter_Copies(author));
>+            m_compFields->SetReplyTo(author.get());
>+
>+            // ... and empty out the To: and Cc: headers
>+            m_compFields->SetTo("");
>+            m_compFields->SetCc("");

nit: The arguments should be EmptyString()

>Index: mailnews/mime/src/mimedrft.cpp

>       if (mdd->format_out == nsMimeOutput::nsMimeMessageEditorTemplate)
>       {
>         fields->SetDraftId(mdd->url_name);
>-        CreateTheComposeWindow(fields, newAttachData, nsIMsgCompType::Template, composeFormat, mdd->identity, mdd->originalMsgURI);
>+        MSG_ComposeType msgComposeType = PL_strstr(mdd->url_name, "redirect=true")? nsIMsgCompType::Redirect : nsIMsgCompType::Template;
>+        CreateTheComposeWindow(fields, newAttachData, msgComposeType, composeFormat, mdd->identity, mdd->originalMsgURI);

nit: need a space before ? I think these lines could also do with wrapping as you're touching them.

I've not reviewed any of the penelope files.

r=me with those fixed (bugzilla is being strange, so hopefully setting these flags will work properly).

Comment 24

[Jeff Beckley]

Attachment: Addresses Standard8's comments

Here's a new patch based on the comm-central repository (so it no longer has Penelope changes in it).  It also addresses Standard8's review comments:
- New UID for nsIMsgCompType
- Comments about new Redirect compose type
- EmptyString() argument to SetTo() and SetCc()
- Wrapped some long lines

Continuing r=standard8

Comment 25

[[e-mail]]

Comment on attachment 332000 [details] [diff] [review]
Addresses Standard8's comments

>+            // For a redirect, set the Reply-To: header to what was in the original From: header...
>+            nsCAutoString author;
>+            msgHdr->GetAuthor(getter_Copies(author));
>+            m_compFields->SetReplyTo(author.get());
Just author, no .get(), surely?

>+            // ... and empty out the To: and Cc: headers
>+            m_compFields->SetTo(EmptyString());
>+            m_compFields->SetCc(EmptyString());
Bcc? Newsgroups? Followup-to?

>+                                                   "redirect=true") ?
"&redirect=true"

Comment 26

[[e-mail]]

(In reply to comment #23)
>(From update of attachment 324939 [details] [diff] [review])
>>Index: mailnews/compose/public/nsIMsgComposeParams.idl
>>+    const long Redirect                 = 14;
>You need to change the uuid for nsIMsgCompType as you're changing the interface.
Actually this makes no sense for adding a constant because it doesn't change binary compatibility, and the interface has no methods anyway.

Comment 27

[Jeff Beckley]

Attachment: Final patch

Final patch that fixes Neil's points.  I changed nsIMsgCompFields.followupTo to be like most of the rest of the header attributes: an AString instead of a string.  So there's a couple of extra files modified.

Continuing r=Standard8, sr=Neil.

Comment 28

[Jeff Beckley]

Moved this is to MailNews Core:Composition as it has no UI.

Comment 29

[Mark Banner (:standard8)]

(In reply to comment #27)
> Created an attachment (id=335594) [details]
> Final patch
> 
> Final patch that fixes Neil's points.  I changed nsIMsgCompFields.followupTo to
> be like most of the rest of the header attributes: an AString instead of a
> string.  So there's a couple of extra files modified.
> 
> Continuing r=Standard8, sr=Neil.

This patch has been bitrotted by Bug 366482, Jeff, could you update it please?

Comment 30

[Jeff Beckley]

Attachment: Fix bit-rot

Continuing r=Standard8, sr=Neil.

Comment 31

[Mark Banner (:standard8)]

Thanks for updating Jeff.

Checked in, changeset id: 212:f483e0bf894c

I believe that this bug is fixed now.

Comment 32

[Rimas Kudelis [use needinfo to ping]]

(In reply to comment #17)
> So do we need both Edit Message As New and Redirect menu items?  It is true
> that you can get the behavior of Redirect by using Edit Message As New, but it
> is much more time consuming.  You would have to copy the name and email address
> of the sender in the original message (no easy task itself, try it), select
> Message->Edit Message As New, add a Reply-To: header and paste in the original
> sender's name and address, and then clear out the To: and Cc: headers.
> 
> One possibility would be to do the separate functions from one menu item
> depending on what type of message is being acted upon.  In general you would
> only want to use the Edit Message As New functionality on outgoing messages
> (Sent/Drafts), and the Redirect functionality only on received messages.  I'm
> sure there are some users that could create use cases for the opposites,
> though.  It might be strange that you get different results from the same menu
> item, even though that's probably desired once you really think about it.

I don't want to file a new bug for Tb just yet, but PERHAPS the "Edit as New" item should open a window with not just all recipients listed (as it does now), but also with a Reply-To header set in cases, when the message being edited was sent from someone else? The user would then have to choose which headers to keep, and which ones to delete, and this is much faster and easier than adding any of them manually. This would also save you from having an additional menu entry that nearly duplicates the other entry's action.

There's only one thing that may be ineffective in this case – and it is removing the headers (in case there is a lot of them). But I don't think that editing a mass-mailed message is a common enough scenario.

Comment 33

[David]

If this is what's in 8.0.0B4; how do I use it? 

I don't find a menu entry for it.

Comment 34

[Jeff Beckley]

(In reply to comment #32)
> There's only one thing that may be ineffective in this case – and it is
> removing the headers (in case there is a lot of them). But I don't think that
> editing a mass-mailed message is a common enough scenario.

For someone who this happens frequently to it could be a big pain, and even small amounts of recipients would be a hassle.  This would also be a problem for Turbo Redirect, where you don't get a chance to edit the message before its sent.  I think the the extra menu item is a pretty small penalty to pay.


(In reply to comment #33)
> If this is what's in 8.0.0B4; how do I use it? 
> 
> I don't find a menu entry for it.

In the Message menu, 6th menu item from the top.