[prev in list] [next in list] [prev in thread] [next in thread]
List: kmail-devel
Subject: Re: [PATCH] "Reply" button functionality in KMail (especially with
From: Matthew Blissett <matt () blissett ! me ! uk>
Date: 2007-11-24 19:05:32
Message-ID: 200711241905.42347.matt () blissett ! me ! uk
[Download RAW message or body]
[Attachment #2 (multipart/signed)]
[Attachment #4 (multipart/mixed)]
Hi
(Some deleted quotation replaced)
I've made a patch! It's my first one, so let me know what can be improved.
It applies to kdepim/kmail/kmmessage.cpp
On Friday 23 November 2007 09:35:16 David Faure wrote:
> On Thursday 22 November 2007 21:54:26 Ingo Klöcker wrote:
> > On Wednesday 21 November 2007, Matthew Blissett wrote:
> > > Hi
> > >
> > > I think this behaviour would be better:
> > >
> > > "Reply" button
> > > If "Folder holds a mailing list" is ticked for this folder
> > > - the same as "Reply to List" button
> > > Otherwise
> > > - the same as "Reply to Author" button
> >
> > I don't think this behavior is better than the current behavior. If
> > anything then I'd change "Reply" to always do a reply to the
> > Mail-Reply-To/Reply-To/From address.
With the patch "Reply" will;
if "Folder holds a mailing list" is ticked do the same as "Reply to list";
if not, do the same as "Reply to Author" button, except that it will honour a
mailing-list-munged reply-to address.
> Hmm, so after 8 years of pressing 'r' to reply on kde lists, it would
> suddenly not reply to the list (except on those lists that mangle
> Reply-To)? That would be pretty unexpected and confusing.
Where there isn't a Reply-To header KMail previously would reply to the whole
list. With the patch, it will reply to the list if the folder is set as a
mailing list. Otherwise, it will reply to the From address (or Reply-To etc)
When I forget to check the To: line and hit send, I'd prefer the safe option:
that the message goes to one person when it was intended for many, rather
than to many people when it was intended for one.
> At least if it depends on "Folder holds a mailing list" (which is not in
> the RMB for folders so it's pretty hard to find BTW), it will mean a lot of
> checkbox-clicking after upgrading, but at least one will be able to set up
> kmail so that 'r' still replies to the list (whatever the list
> configuration is). Yes, I do want to reply on the list, 99.9999% of the
> time. When I want to mail a personal reply I just do Shift+A, that's
> 0.0001% of the time...
I get messages sent to "all-final-year-students@..." etc from staff at
University, which aren't intended as discussion lists at all -- Reply-To
isn't set, and replies should go to the sender of the message.
Currently, 'r' replies to the whole list, which is annoying.
For real discussion mailing lists I can tick that box in the folder
settings :-) (a RMB item would be good).
> > > "Reply to List" button
> > > Try
> > > - add addresses in "Mail-Followup-To" header to "To"
> > > Otherwise
> > > - add address set in "Mailing list management" to "To"
> > > Otherwise
> > > - add address in "List-Post" header to "To"
> > > Otherwise
> > > - leave "To" empty
> >
> > Makes sense, but doesn't KMail work already that way?
Yes, I wrote it for completeness.
(Well, KMail almost does this. If there's no address after trying List-Post
KMail uses Reply-To since it might have been set by a mailing list. The
patch shouldn't change this.)
> > > "Reply to Author" button
> > > - Add "Mail-Reply-To" or "Reply-To" or "From" to "To"
> >
> > Yes. Additionally, KMail should try to ignore Reply-To headers set by
> > Reply-To-header-mangling mailing list managers.
Yes.
(By checking if List-Post or an address set by the user is the same as
Reply-To.)
> > > "Reply to All" button
> > > - Do "Reply to List"
> > >
> > > If "To" is empty (so it wasn't a mailing list)
> > > - Add "Mail-Reply-To" or "Reply-To" or "From" to "To"
> > > Otherwise (we have a mailing list address in "To")
> > > - Add "Mail-Reply-To" or "Reply-To" or "From" to "CC"
> > >
> > > - Add "To" and "CC" addresses to "CC"
> >
> > Not sure about this. I like the current behavior to always put the
> > Reply-To/From address in To and all other addresses in CC better.
That wasn't quite the current behaviour, but I've done it anyway.
-------------
Summary of behaviour with patch:
"Reply" button
If "Folder holds a mailing list" is ticked for this folder
- the same as "Reply to List" button
Otherwise
- the same as "Reply to Author" button EXCEPT we honour Reply-To munging
mailing lists
"Reply to List" button
Try
- add addresses in "Mail-Followup-To" header to "To"
Otherwise
- add address set in "Mailing list management" or "List Post" to "To"
Otherwise
- assume Reply-To munging list and use Reply-To
Otherwise
- leave "To" empty
"Reply to Author" button
- Add "Mail-Reply-To" or "Reply-To" or "From" to "To"
- Remove mailing list addresses if we know them (from configuration or
List-Post header)
"Reply to All" button
- Do "Reply to List"
(except don't accept mailing list reply-to munged addresses just yet)
- Add "Mail-Reply-To" or "Reply-To" or "From" to "To"
- Add "To" and "CC" addresses to "CC"
-------------
Thanks :-)
--
Matt Blissett
matt@blissett.me.uk
["kmmessage-patch.diff" (text/x-diff)]
Index: kmmessage.cpp
===================================================================
--- kmmessage.cpp (revision 739637)
+++ kmmessage.cpp (working copy)
@@ -847,186 +847,242 @@
const QString &tmpl /* = QString() */ )
{
KMMessage* msg = new KMMessage;
- QString str, replyStr, mailingListStr, replyToStr, toStr;
- QStringList mailingListAddresses;
+ QString replyStr, toStr;
+ QStringList mailingListAddresses, recipients, ccRecipients, replyToList;
QByteArray refStr, headerName;
bool replyAll = true;
+ bool smartReply = false;
msg->initFromMessage(this);
- MailingList::name(this, headerName, mailingListStr);
- replyToStr = replyTo();
-
msg->setCharset("utf-8");
- // determine the mailing list posting address
+ // use the "On ... Joe User wrote:" header by default
+ replyStr = sReplyAllStr;
+
+ /*
+ "Reply" button
+ If "Folder holds a mailing list" is ticked for this folder
+ - the same as "Reply to List" button
+ Otherwise
+ - the same as "Reply to Author" button EXCEPT we honour Reply-To munging
+ mailing lists
+
+ "Reply to List" button
+ Try
+ - add addresses in "Mail-Followup-To" header to "To"
+ Otherwise
+ - add address set in "Mailing list management" to "To"
+ Otherwise
+ - add address in "List-Post" header to "To"
+ Otherwise
+ - assume Reply-To munging list and use Reply-To
+ Otherwise
+ - leave "To" empty
+
+ "Reply to Author" button
+ - Add "Mail-Reply-To" or "Reply-To" or "From" to "To"
+ - Remove mailing list addresses if we know them (from configuration or
+ List-Post header)
+
+ "Reply to All" button
+ - Do "Reply to List"
+ (except don't accept mailing list reply-to munged addresses just yet)
+ - Add "Mail-Reply-To" or "Reply-To" or "From" to "To"
+ - Add "To" and "CC" addresses to "CC"
+ */
+
+ // Find mailing list addresses
if ( parent() && parent()->isMailingListEnabled() &&
!parent()->mailingListPostAddress().isEmpty() ) {
+ kDebug(5006) << "Folder is set as a mailing list";
mailingListAddresses << parent()->mailingListPostAddress();
}
if ( headerField("List-Post").contains( "mailto:", Qt::CaseInsensitive ) ) {
QString listPost = headerField("List-Post");
QRegExp rx( "<mailto:([^@>]+)@([^>]+)>", Qt::CaseInsensitive );
- if ( rx.indexIn( listPost, 0 ) != -1 ) // matched
+ if ( rx.indexIn( listPost, 0 ) != -1 ) { // matched
+ kDebug(5006) << "Message has useful List-Post address";
mailingListAddresses << rx.cap(1) + '@' + rx.cap(2);
+ }
}
- // use the "On ... Joe User wrote:" header by default
- replyStr = sReplyAllStr;
-
- switch( replyStrategy ) {
- case KMail::ReplySmart : {
- if ( !headerField( "Mail-Followup-To" ).isEmpty() ) {
- toStr = headerField( "Mail-Followup-To" );
+ // Set ReplySmart to be a ReplyList (if folder is set as mailing list)
+ // or ReplyAuthor (otherwise)
+ if ( replyStrategy == KMail::ReplySmart ) {
+ smartReply = true;
+ if ( parent() && parent()->isMailingListEnabled() ) {
+ // Folder holds a mailing list
+ replyStrategy = KMail::ReplyList;
+ kDebug(5006) << "Smart reply becoming List reply";
}
- else if ( !replyToStr.isEmpty() ) {
- // assume a Reply-To header mangling mailing list
- toStr = replyToStr;
- }
- else if ( !mailingListAddresses.isEmpty() ) {
- toStr = mailingListAddresses[0];
- }
else {
- // doesn't seem to be a mailing list, reply to From: address
- toStr = from();
- replyStr = sReplyStr; // reply to author, so use "On ... you wrote:"
- replyAll = false;
+ replyStrategy = KMail::ReplyAuthor;
+ kDebug(5006) << "Smart reply becoming Author reply";
}
- // strip all my addresses from the list of recipients
- QStringList recipients = KPIMUtils::splitAddressList( toStr );
- toStr = stripMyAddressesFromAddressList( recipients ).join(", ");
- // ... unless the list contains only my addresses (reply to self)
- if ( toStr.isEmpty() && !recipients.isEmpty() )
- toStr = recipients[0];
-
- break;
}
- case KMail::ReplyList : {
- if ( !headerField( "Mail-Followup-To" ).isEmpty() ) {
- toStr = headerField( "Mail-Followup-To" );
- }
- else if ( !mailingListAddresses.isEmpty() ) {
- toStr = mailingListAddresses[0];
- }
- else if ( !replyToStr.isEmpty() ) {
- // assume a Reply-To header mangling mailing list
- toStr = replyToStr;
- }
- // strip all my addresses from the list of recipients
- QStringList recipients = KPIMUtils::splitAddressList( toStr );
- toStr = stripMyAddressesFromAddressList( recipients ).join(", ");
- break;
+ // Prefer Mail-Reply-To header to Reply-To header
+ replyToList = KPIMUtils::splitAddressList(
+ KPIMUtils::normalizeAddressesAndDecodeIdn(
+ headerField("Mail-Reply-To") ) );
+ if ( replyToList.isEmpty() ) {
+ replyToList = KPIMUtils::splitAddressList( replyTo() );
}
- case KMail::ReplyAll : {
- QStringList recipients;
- QStringList ccRecipients;
- // add addresses from the Reply-To header to the list of recipients
- if( !replyToStr.isEmpty() ) {
- recipients += KPIMUtils::splitAddressList( replyToStr );
- // strip all possible mailing list addresses from the list of Reply-To
- // addresses
- for ( QStringList::const_iterator it = mailingListAddresses.begin();
- it != mailingListAddresses.end();
- ++it ) {
- recipients = stripAddressFromAddressList( *it, recipients );
+ switch ( replyStrategy ) {
+ case KMail::ReplyAll :
+ case KMail::ReplyList : {
+ if ( !headerField( "Mail-Followup-To" ).isEmpty() ) {
+ // When posting to a mailing list 'power users' can set
+ // Mail-Followup-To to include (or not) their own address, depending
+ // if they're subscribed to the list or not
+ recipients += KPIMUtils::splitAddressList( headerField( "Mail-Followup-To" ) );
}
- }
+ else if ( !mailingListAddresses.isEmpty() ) {
+ // Add mailing list addresses (configured or List-Post)
+ recipients += KPIMUtils::splitAddressList( mailingListAddresses[0] );
+ }
+ // "To" might be empty if this doesn't seem to be a mailing list.
- if ( !mailingListAddresses.isEmpty() ) {
- // this is a mailing list message
- if ( recipients.isEmpty() && !from().isEmpty() ) {
- // The sender didn't set a Reply-to address, so we add the From
- // address to the list of CC recipients.
- ccRecipients += from();
- kDebug(5006) <<"Added" << from() <<"to the list of CC recipients";
+ if ( replyStrategy == KMail::ReplyList ) {
+ if ( recipients.isEmpty() ) {
+ // Assume reply-to munging mailing list that doesn't set a List-Post
+ // header
+ recipients += replyToList;
+ }
}
- // if it is a mailing list, add the posting address
- recipients.prepend( mailingListAddresses[0] );
- }
- else {
- // this is a normal message
- if ( recipients.isEmpty() && !from().isEmpty() ) {
- // in case of replying to a normal message only then add the From
- // address to the list of recipients if there was no Reply-to address
- recipients += from();
- kDebug(5006) <<"Added" << from() <<"to the list of recipients";
- }
- }
+ else {
+ // ReplyAll
- // strip all my addresses from the list of recipients
- toStr = stripMyAddressesFromAddressList( recipients ).join(", ");
+ QStringList potentialTo, potentialCC;
- // merge To header and CC header into a list of CC recipients
- if( !cc().isEmpty() || !to().isEmpty() ) {
- QStringList list;
- if (!to().isEmpty())
- list += KPIMUtils::splitAddressList(to());
- if (!cc().isEmpty())
- list += KPIMUtils::splitAddressList(cc());
- for( QStringList::Iterator it = list.begin(); it != list.end(); ++it ) {
- if( !addressIsInAddressList( *it, recipients )
- && !addressIsInAddressList( *it, ccRecipients ) ) {
- ccRecipients += *it;
- kDebug(5006) <<"Added" << *it <<"to the list of CC recipients";
+ // Add Reply-To addresses to To
+ switch ( replyToList.size() ) {
+ case 0: {
+ // No Reply-to addresses: fall back to From
+ potentialTo += from();
+ break;
+ }
+ case 1: {
+ if ( addressIsInAddressList( replyToList[0], mailingListAddresses ) ) {
+ // Reply to address is mailing list address:
+ // add From address too (since this is reply ALL)
+ potentialTo += from();
+ }
+ else {
+ potentialTo += replyToList;
+ }
+ }
+ default: {
+ potentialTo += replyToList;
+ }
}
- }
- }
- if ( !ccRecipients.isEmpty() ) {
- // strip all my addresses from the list of CC recipients
- ccRecipients = stripMyAddressesFromAddressList( ccRecipients );
+ for ( QStringList::Iterator it = potentialTo.begin();
+ it != potentialTo.end();
+ ++it ) {
+ if ( !addressIsInAddressList( *it, recipients ) ) {
+ recipients += *it;
+ kDebug(5006) << "Added" << *it << "to the list of recipients";
+ }
+ }
- // in case of a reply to self toStr might be empty. if that's the case
- // then propagate a cc recipient to To: (if there is any).
- if ( toStr.isEmpty() && !ccRecipients.isEmpty() ) {
- toStr = ccRecipients[0];
- ccRecipients.pop_front();
+ // also merge To, CC headers into a list of CC recipients
+ if( !cc().isEmpty() || !to().isEmpty() ) {
+ if ( !to().isEmpty() ) {
+ potentialCC += KPIMUtils::splitAddressList( to() );
+ }
+ if ( !cc().isEmpty() ) {
+ potentialCC += KPIMUtils::splitAddressList( cc() );
+ }
+ }
+
+ if ( !potentialCC.isEmpty() ) {
+ for ( QStringList::Iterator it = potentialCC.begin();
+ it != potentialCC.end();
+ ++it ) {
+ if ( !addressIsInAddressList( *it, recipients )
+ && !addressIsInAddressList( *it, ccRecipients ) ) {
+ ccRecipients += *it;
+ kDebug(5006) << "Added" << *it << "to the list of CC recipients";
+ }
+ }
+ }
+
}
- msg->setCc( ccRecipients.join(", ") );
+ break;
}
- if ( toStr.isEmpty() && !recipients.isEmpty() ) {
- // reply to self without other recipients
- toStr = recipients[0];
- }
- break;
- }
- case KMail::ReplyAuthor : {
- if ( !replyToStr.isEmpty() ) {
- QStringList recipients = KPIMUtils::splitAddressList( replyToStr );
- // strip the mailing list post address from the list of Reply-To
- // addresses since we want to reply in private
- for ( QStringList::const_iterator it = mailingListAddresses.begin();
- it != mailingListAddresses.end();
- ++it ) {
- recipients = stripAddressFromAddressList( *it, recipients );
+ case KMail::ReplyAuthor : {
+ // Adds "Mail-Reply-To" or "Reply-To" or "From" to "To"
+ // Try and strip mailing list addresses from this
+
+ if ( !replyToList.isEmpty() ) {
+ // Use Reply-To if set
+ recipients = replyToList;
+
+ if ( !smartReply ) {
+ // ReplyAuthor should avoid replying to a mailing lists
+ // (but ReplySmart should allow a munged Reply-To)
+
+ // Remove the mailing list post address from the list of Reply-To
+ // addresses since we want to reply in private
+ for ( QStringList::const_iterator it = mailingListAddresses.begin();
+ it != mailingListAddresses.end();
+ ++it ) {
+ recipients = stripAddressFromAddressList( *it, recipients );
+ }
+ }
+
+ if ( recipients.isEmpty() ) {
+ // there was only the mailing list post address in the Reply-To
+ // header, so use the From address instead
+ recipients += from();
+ }
}
- if ( !recipients.isEmpty() ) {
- toStr = recipients.join(", ");
+ else if ( !from().isEmpty() ) {
+ recipients += from();
}
- else {
- // there was only the mailing list post address in the Reply-To header,
- // so use the From address instead
- toStr = from();
- }
+
+ replyStr = sReplyStr; // reply to author, so use "On ... you wrote:"
+ replyAll = false;
+
+ break;
}
- else if ( !from().isEmpty() ) {
- toStr = from();
+ default : {
+ break;
}
- replyStr = sReplyStr; // reply to author, so use "On ... you wrote:"
- replyAll = false;
- break;
}
- case KMail::ReplyNone : {
- // the addressees will be set by the caller
+
+ // strip all my addresses from the list of recipients
+ toStr = stripMyAddressesFromAddressList( recipients ).join( ", " );
+
+ if ( !ccRecipients.isEmpty() ) {
+ // strip all my addresses from the list of CC recipients
+ ccRecipients = stripMyAddressesFromAddressList( ccRecipients );
+
+ // in case of a reply to self toStr might be empty. If that's the case
+ // then move a CC recipient to To (if there are any).
+ if ( toStr.isEmpty() && !ccRecipients.isEmpty() ) {
+ toStr = ccRecipients[0];
+ ccRecipients.pop_front();
+ }
}
+
+ if ( toStr.isEmpty() && !recipients.isEmpty() ) {
+ // reply to self without other recipients
+ kDebug(5006) << "Replied to self";
+ toStr = recipients[0];
}
- msg->setTo(toStr);
+ msg->setTo( toStr );
+ msg->setCc( ccRecipients.join(", ") );
+ kDebug (5006) << "to() at end is " << to() << endl;
+ kDebug (5006) << "cc() at end is " << cc() << endl;
+
refStr = getRefStr();
if (!refStr.isEmpty())
msg->setReferences(refStr);
["signature.asc" (application/pgp-signature)]
_______________________________________________
KMail developers mailing list
KMail-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmail-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic