diff --git a/messagecomposer/autotests/messagefactoryngtest.cpp b/messagecomposer/autotests/messagefactoryngtest.cpp --- a/messagecomposer/autotests/messagefactoryngtest.cpp +++ b/messagecomposer/autotests/messagefactoryngtest.cpp @@ -204,7 +204,6 @@ MessageFactoryNG::MessageReply reply = spy.at(0).at(0).value(); reply.replyAll = true; - //qDebug() << reply.msg->body(); QDateTime date = msg->date()->dateTime(); QString datetime = QLocale::system().toString(date.date(), QLocale::LongFormat); @@ -217,8 +216,8 @@ QString ba = QString::fromLatin1("From: foo1 \n" "X-KMail-Identity: %1\n" "Date: %2\n" - "Cc: blo , bli , blu , bly , Bla \n" - "To: Bla \n" + "Cc: blu , bly \n" + "To: blo , bli \n" "Subject: Re: Plain Message Test\n" "Content-Type: text/plain; charset=\"US-ASCII\"\n" "Content-Transfer-Encoding: 8Bit\nMIME-Version: 1.0\n" @@ -253,7 +252,6 @@ MessageFactoryNG::MessageReply reply = spy.at(0).at(0).value(); reply.replyAll = true; - //qDebug() << reply.msg->body(); QDateTime date = msg->date()->dateTime(); QString datetime = QLocale::system().toString(date.date(), QLocale::LongFormat); @@ -265,8 +263,8 @@ QString dateStr = reply.msg->date()->asUnicodeString(); QString ba = QString::fromLatin1("From: another \n" "Date: %1\n" - "Cc: blo , bli , blu , bly \n" - "To: Bla \n" + "Cc: blu , bly \n" + "To: blo , bli \n" "Subject: Re: Plain Message Test\n" "Content-Type: text/plain; charset=\"US-ASCII\"\n" "Content-Transfer-Encoding: 8Bit\nMIME-Version: 1.0\n" @@ -301,7 +299,6 @@ MessageFactoryNG::MessageReply reply = spy.at(0).at(0).value(); reply.replyAll = true; - //qDebug() << reply.msg->body(); QDateTime date = msg->date()->dateTime(); QString datetime = QLocale::system().toString(date.date(), QLocale::LongFormat); @@ -313,8 +310,8 @@ QString dateStr = reply.msg->date()->asUnicodeString(); QString ba = QString::fromLatin1("From: another \n" "Date: %1\n" - "Cc: blo , bli , blu , bly \n" - "To: Bla \n" + "Cc: blu , bly \n" + "To: blo , bli , Bla \n" "Subject: Re: Plain Message Test\n" "Content-Type: text/plain; charset=\"US-ASCII\"\n" "Content-Transfer-Encoding: 8Bit\nMIME-Version: 1.0\n" @@ -379,7 +376,6 @@ MessageFactoryNG::MessageReply reply = spy.at(0).at(0).value(); reply.replyAll = true; - //qDebug() << reply.msg->body(); QDateTime date = msg->date()->dateTime(); QString datetime = QLocale::system().toString(date.date(), QLocale::LongFormat); @@ -424,7 +420,6 @@ MessageFactoryNG::MessageReply reply = spy.at(0).at(0).value(); reply.replyAll = true; - //qDebug() << reply.msg->body(); QDateTime date = msg->date()->dateTime(); QString datetime = QLocale::system().toString(date.date(), QLocale::LongFormat); @@ -439,8 +434,8 @@ QString ba = QString::fromLatin1("From: another \n" "Date: %1\n" "X-KMail-Transport: 0\n" - "Cc: you@you.you, you2@you.you, cc@cc.cc, cc2@cc.cc\n" - "To: me@me.me\n" + "Cc: cc@cc.cc, cc2@cc.cc\n" + "To: you@you.you, you2@you.you, me@me.me\n" "References: %3\n" "In-Reply-To: %2\n" "Subject: Re: Test Email Subject\nContent-Type: text/plain; charset=\"US-ASCII\"\n" @@ -463,7 +458,6 @@ QCOMPARE(spy.count(), 1); MessageFactoryNG::MessageReply reply = spy.at(0).at(0).value(); reply.replyAll = true; - //qDebug() << reply.msg->body(); QDateTime date = msg->date()->dateTime(); QString datetime = QLocale::system().toString(date.date(), QLocale::LongFormat); @@ -492,7 +486,6 @@ QCOMPARE(spy.count(), 1); MessageFactoryNG::MessageReply reply = spy.at(0).at(0).value(); reply.replyAll = true; - //qDebug() << "html reply" << reply.msg->encodedContent(); QDateTime date = msg->date()->dateTime().toLocalTime(); QString datetime = QLocale().toString(date.date(), QLocale::LongFormat); @@ -523,7 +516,6 @@ KMime::Message::Ptr msg = loadMessageFromFile(QStringLiteral("plain_utf16.mbox")); TemplateParser::TemplateParserSettings::self()->setReplyUsingHtml(true); -// qDebug() << "plain base64 msg message:" << msg->encodedContent(); MessageFactoryNG factory(msg, 0); factory.setIdentityManager(mIdentMan); @@ -536,7 +528,6 @@ QCOMPARE(spy.count(), 1); MessageFactoryNG::MessageReply reply = spy.at(0).at(0).value(); reply.replyAll = true; -// qDebug() << "html reply" << reply.msg->encodedContent(); QDateTime date = msg->date()->dateTime().toLocalTime(); QString datetime = QLocale().toString(date.date(), QLocale::LongFormat); @@ -586,7 +577,6 @@ "-----------------------------------------"); fwdMsg = fwdMsg.arg(datetime).arg(fw->date()->asUnicodeString()); -// qDebug() << "got:" << fw->encodedContent() << "against" << fwdMsg.toLatin1(); QCOMPARE(fw->subject()->asUnicodeString(), QStringLiteral("Fwd: Test Email Subject")); QCOMPARE_OR_DIFF(fw->encodedContent(), fwdMsg.toLatin1()); } @@ -631,7 +621,6 @@ "-----------------------------------------"); fwdMsg = fwdMsg.arg(datetime).arg(fw->date()->asUnicodeString()); -// qDebug() << "got:" << fw->encodedContent() << "against" << fwdMsg.toLatin1(); QCOMPARE(fw->subject()->asUnicodeString(), QStringLiteral("Fwd: Test Email Subject")); QCOMPARE_OR_DIFF(fw->encodedContent(), fwdMsg.toLatin1()); } @@ -650,14 +639,11 @@ QString datetime = rdir->date()->asUnicodeString(); -// qDebug() << rdir->encodedContent(); - QRegExp rx(QLatin1String("Resent-Message-ID: ([^\n]*)")); rx.indexIn(QString::fromLatin1(rdir->head())); QRegExp rxmessageid(QLatin1String("Message-ID: ([^\n]+)")); rxmessageid.indexIn(QString::fromLatin1(rdir->head())); - //qWarning() << "messageid:" << rxmessageid.cap(1) << "(" << rdir->head() << ")"; QString baseline = QString::fromLatin1("From: me@me.me\n" "Cc: cc@cc.cc\n" "Bcc: bcc@bcc.bcc\n" @@ -681,10 +667,6 @@ "All happy families are alike; each unhappy family is unhappy in its own way."); baseline = baseline.arg(datetime).arg(rxmessageid.cap(1)).arg(rx.cap(1)).arg(datetime).arg(QStringLiteral("another ")); -// qDebug() << baseline.toLatin1(); -// qDebug() << "instead:" << rdir->encodedContent(); - -// QString fwdStr = QString::fromLatin1( "On " + datetime.toLatin1() + " you wrote:\n> All happy families are alike; each unhappy family is unhappy in its own way.\n" ); QCOMPARE(rdir->subject()->asUnicodeString(), QStringLiteral("Test Email Subject")); QCOMPARE_OR_DIFF(rdir->encodedContent(), baseline.toLatin1()); } @@ -702,8 +684,6 @@ QString datetime = rdir->date()->asUnicodeString(); -// qDebug() << rdir->encodedContent(); - QString msgId = MessageCore::StringUtil::generateMessageId(msg->sender()->asUnicodeString(), QString()); QRegExp rx(QLatin1String("Resent-Message-ID: ([^\n]*)")); @@ -734,10 +714,6 @@ "All happy families are alike; each unhappy family is unhappy in its own way."); baseline = baseline.arg(datetime).arg(rxmessageid.cap(1)).arg(rx.cap(1)).arg(datetime).arg(QStringLiteral("another ")); -// qDebug() << baseline.toLatin1(); -// qDebug() << "instead:" << rdir->encodedContent(); - -// QString fwdStr = QString::fromLatin1( "On " + datetime.toLatin1() + " you wrote:\n> All happy families are alike; each unhappy family is unhappy in its own way.\n" ); QCOMPARE(rdir->subject()->asUnicodeString(), QStringLiteral("Test Email Subject")); QCOMPARE_OR_DIFF(rdir->encodedContent(), baseline.toLatin1()); } @@ -754,14 +730,11 @@ QString datetime = rdir->date()->asUnicodeString(); -// qDebug() << rdir->encodedContent(); - QRegExp rx(QLatin1String("Resent-Message-ID: ([^\n]*)")); rx.indexIn(QString::fromLatin1(rdir->head())); QRegExp rxmessageid(QLatin1String("Message-ID: ([^\n]+)")); rxmessageid.indexIn(QString::fromLatin1(rdir->head())); - //qWarning() << "messageid:" << rxmessageid.cap(1) << "(" << rdir->head() << ")"; QString baseline = QString::fromLatin1("From: me@me.me\n" "Cc: cc@cc.cc\n" "Bcc: bcc@bcc.bcc\n" @@ -783,10 +756,6 @@ "All happy families are alike; each unhappy family is unhappy in its own way."); baseline = baseline.arg(datetime).arg(rxmessageid.cap(1)).arg(rx.cap(1)).arg(datetime).arg(QStringLiteral("another ")); -// qDebug() << baseline.toLatin1(); -// qDebug() << "instead:" << rdir->encodedContent(); - -// QString fwdStr = QString::fromLatin1( "On " + datetime.toLatin1() + " you wrote:\n> All happy families are alike; each unhappy family is unhappy in its own way.\n" ); QCOMPARE(rdir->subject()->asUnicodeString(), QStringLiteral("Test Email Subject")); QCOMPARE_OR_DIFF(rdir->encodedContent(), baseline.toLatin1()); } @@ -802,8 +771,6 @@ QString datetime = rdir->date()->asUnicodeString(); -// qDebug() << msg->encodedContent(); - QRegExp rx(QLatin1String("Resent-Message-ID: ([^\n]*)")); rx.indexIn(QString::fromLatin1(rdir->head())); @@ -826,10 +793,6 @@ "All happy families are alike; each unhappy family is unhappy in its own way."); baseline = baseline.arg(msg->to()->asUnicodeString()).arg(datetime).arg(rxmessageid.cap(1)); - //qDebug() << baseline.toLatin1(); - //qDebug() << "instead:" << rdir->encodedContent(); - -// QString fwdStr = QString::fromLatin1( "On " + datetime.toLatin1() + " you wrote:\n> All happy families are alike; each unhappy family is unhappy in its own way.\n" ); QCOMPARE(rdir->subject()->asUnicodeString(), QStringLiteral("Test Email Subject")); QCOMPARE_OR_DIFF(rdir->encodedContent(), baseline.toLatin1()); } @@ -845,15 +808,12 @@ KMime::Message::Ptr mdn = factory.createMDN(KMime::MDN::AutomaticAction, KMime::MDN::Displayed, KMime::MDN::SentAutomatically); QVERIFY(mdn.data()); - //qDebug() << "mdn" << mdn->encodedContent(); QString mdnContent = QString::fromLatin1("The message sent on %1 to %2 with subject \"%3\" has been displayed. " "This is no guarantee that the message has been read or understood."); mdnContent = mdnContent.arg(KMime::DateFormatter::formatDate(KMime::DateFormatter::Localized, msg->date()->dateTime().toSecsSinceEpoch())) .arg(msg->to()->asUnicodeString()).arg(msg->subject()->asUnicodeString()); - //qDebug() << "comparing with:" << mdnContent; - QCOMPARE_OR_DIFF(Util::findTypeInMessage(mdn.data(), "multipart", "report")->contents().at(0)->body(), mdnContent.toLatin1()); } diff --git a/messagecomposer/autotests/replystrategytest.cpp b/messagecomposer/autotests/replystrategytest.cpp --- a/messagecomposer/autotests/replystrategytest.cpp +++ b/messagecomposer/autotests/replystrategytest.cpp @@ -261,27 +261,70 @@ QTest::newRow("ReplyAll, with multiple To addresses in original") << friend1Address << both(friend2Address, nondefaultAddress) << only(defaultAddress) << nobody << nobody << QString() << nobody - << (int)ReplyAll << nondefaultAddress << only(friend1Address) << only(friend2Address); + << (int)ReplyAll << nondefaultAddress << both(friend1Address, friend2Address) << nobody; QTest::newRow("ReplyAll, with Reply-To in original") << friend1Address << only(defaultAddress) << only(friend2Address) << only(replyAddress) << nobody << QString() << nobody << (int)ReplyAll << defaultAddress << only(replyAddress) << only(friend2Address); + QTest::newRow("ReplyAll, with Mail-Reply-To in original") + << friend1Address << only(defaultAddress) << only(friend2Address) + << only(replyAddress) << nobody << QString() << only(mailReplyAddress) + << (int)ReplyAll << defaultAddress << only(mailReplyAddress) << only(friend2Address); + + // If the original message was _from_ the user _to_ another person (the + // reverse of the usual direction), reply to all goes to the other person. + // Therefore Mail-Reply-To and Reply-To are ignored. + // The reply is assumed to add to the original message. + QTest::newRow("ReplyAll, from default identity to someone") + << defaultAddress << only(friend1Address) << only(friend2Address) + << only(replyAddress) << nobody << QString() << only(mailReplyAddress) + << (int)ReplyAll << defaultAddress << only(friend1Address) << only(friend2Address); + + // If the original message was from one of the user's identities to another + // identity (i.e., between two of the user's mail accounts), reply to all + // goes back to the sending identity. + QTest::newRow("ReplyAll, between identities") + << defaultAddress << only(nondefaultAddress) << only(friend2Address) + << nobody << nobody << QString() << nobody + << (int)ReplyAll << nondefaultAddress << only(defaultAddress) << only(friend2Address); // If the original passed through a mailing list, ReplyAll replies to the - // list, preferring Mail-Followup-To over List-Post as the list address. + // list. // It CCs the author, using Mail-Reply-To, Reply-To, or From (in that order). QTest::newRow("ReplyAll, from list with List-Post") << friend1Address << only(nondefaultAddress) << only(friend2Address) << nobody << nobody << listAddress << nobody << (int)ReplyAll << nondefaultAddress << only(listAddress) << both(friend1Address, friend2Address); + QTest::newRow("ReplyAll, from list with Reply-To") + << friend1Address << only(defaultAddress) << only(friend2Address) + << only(replyAddress) << nobody << listAddress << nobody + << (int)ReplyAll << defaultAddress << only(listAddress) << both(replyAddress, friend2Address); + QTest::newRow("ReplyAll, from list with Mail-Reply-To") + << friend1Address << only(defaultAddress) << only(friend2Address) + << only(replyAddress) << nobody << listAddress << only(mailReplyAddress) + << (int)ReplyAll << defaultAddress << only(listAddress) << both(mailReplyAddress, friend2Address); // If Reply-To is the same as List-Post, ReplyAll ignores it and uses - // From, because the mailing list munged Reply-To. + // From for the author's address, because the mailing list munged Reply-To. QTest::newRow("ReplyAll, from list that munges Reply-To") << friend1Address << only(defaultAddress) << nobody << only(listAddress) << nobody << listAddress << nobody << (int)ReplyAll << defaultAddress << only(listAddress) << only(friend1Address); + // If Reply-To contains List-Post, ReplyAll uses the other reply + // addresses, because the mailing list didn't completely munge Reply-To. + QTest::newRow("ReplyAll, from list that lightly munges Reply-To") + << friend1Address << only(defaultAddress) << nobody + << both(replyAddress, listAddress) << nobody << listAddress << nobody + << (int)ReplyAll << defaultAddress << only(listAddress) << only(replyAddress); + + // If Mail-Followup-To header is present, use it for To and ignore other + // headers. Cc is empty. + QTest::newRow("ReplyAll, from list with Reply-To and Mail-Followup-To") + << friend1Address << only(defaultAddress) << only(friend2Address) + << only(replyAddress) << only(followupAddress) << listAddress << only(mailReplyAddress) + << (int)ReplyAll << defaultAddress << only(followupAddress) << nobody; + // Reply to Author // --------------- // ReplyAuthor ignores Cc, and replies to the Mail-Reply-To, Reply-To, or diff --git a/messagecomposer/src/helper/messagefactoryng.cpp b/messagecomposer/src/helper/messagefactoryng.cpp --- a/messagecomposer/src/helper/messagefactoryng.cpp +++ b/messagecomposer/src/helper/messagefactoryng.cpp @@ -225,106 +225,49 @@ break; } case MessageComposer::ReplyAll: - { - KMime::Types::Mailbox::List recipients; - KMime::Types::Mailbox::List ccRecipients; - - QString sender; - if (auto hrd = m_origMsg->sender(false)) { - sender = hrd->asUnicodeString(); - } - // add addresses from the Reply-To header to the list of recipients - if (!replyToList.isEmpty()) { - recipients = replyToList; - - // strip all possible mailing list addresses from the list of Reply-To addresses - for (const KMime::Types::Mailbox &mailbox : qAsConst(m_mailingListAddresses)) { - foreach (const KMime::Types::Mailbox &recipient, recipients) { //Don't use for(...:...) - if (mailbox == recipient) { - recipients.removeAll(recipient); - } - } - } - } - bool stripMyAddresses = true; - if (!m_mailingListAddresses.isEmpty()) { - // this is a mailing list message - if (recipients.isEmpty() && !m_origMsg->from()->asUnicodeString().isEmpty()) { - // The sender didn't set a Reply-to address, so we add the From - // address to the list of CC recipients. - ccRecipients += m_origMsg->from()->mailboxes(); - qCDebug(MESSAGECOMPOSER_LOG) << "Added" << m_origMsg->from()->asUnicodeString() << "to the list of CC recipients"; - } - - // if it is a mailing list, add the posting address - recipients.prepend(m_mailingListAddresses[ 0 ]); + if (auto hdr = m_origMsg->headerByType("Mail-Followup-To")) { + toList = KMime::Types::Mailbox::listFrom7BitString(hdr->as7BitString(false)); } else { - const QString fromAddress = m_origMsg->from()->asUnicodeString(); - if (!fromAddress.isEmpty()) { - if (!sender.isEmpty() && m_identityManager->thatIsMe(fromAddress)) { - // strip all my addresses from the list of recipients - toList = recipients; - toList += m_origMsg->from()->mailboxes(); - stripMyAddresses = false; - } else { - // this is a normal message - if (recipients.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 += m_origMsg->from()->mailboxes(); - qCDebug(MESSAGECOMPOSER_LOG) << "Added" << m_origMsg->from()->asUnicodeString() << "to the list of recipients"; + auto ccList = stripMyAddressesFromAddressList(m_origMsg->cc(false)->mailboxes(), m_identityManager); + + if (!m_mailingListAddresses.isEmpty()) { + toList = stripMyAddressesFromAddressList(m_origMsg->to()->mailboxes(), m_identityManager); + bool addMailingList = true; + for (const KMime::Types::Mailbox &m : m_mailingListAddresses) { + if (toList.contains(m)) { + addMailingList = false; + break; } } - } - } - if (stripMyAddresses) { - // strip all my addresses from the list of recipients - toList = stripMyAddressesFromAddressList(recipients, m_identityManager); - } - - // merge To header and CC header into a list of CC recipients - if (!m_origMsg->cc()->asUnicodeString().isEmpty() || !m_origMsg->to()->asUnicodeString().isEmpty()) { - KMime::Types::Mailbox::List list; - if (!m_origMsg->to()->asUnicodeString().isEmpty()) { - list += m_origMsg->to()->mailboxes(); - } - if (!m_origMsg->cc()->asUnicodeString().isEmpty()) { - list += m_origMsg->cc()->mailboxes(); - } - - for (const KMime::Types::Mailbox &mailbox : qAsConst(list)) { - if (!recipients.contains(mailbox) - && !ccRecipients.contains(mailbox)) { - ccRecipients += mailbox; - qCDebug(MESSAGECOMPOSER_LOG) << "Added" << mailbox.prettyAddress() << "to the list of CC recipients"; + if (addMailingList) { + toList += m_mailingListAddresses.front(); } - } - } - - if (!ccRecipients.isEmpty()) { - // strip all my addresses from the list of CC recipients - if (stripMyAddresses) { - ccRecipients = stripMyAddressesFromAddressList(ccRecipients, m_identityManager); - } - // in case of a reply to self, toList might be empty. if that's the case - // then propagate a cc recipient to To: (if there is any). - if (toList.isEmpty() && !ccRecipients.isEmpty()) { - toList << ccRecipients.at(0); - ccRecipients.pop_front(); + ccList += authorMailboxes(m_origMsg, m_mailingListAddresses); + } else { + // Doesn't seem to be a mailing list. + auto originalFromList = m_origMsg->from()->mailboxes(); + auto originalToList = m_origMsg->to()->mailboxes(); + + if (m_identityManager->thatIsMe(KMime::Types::Mailbox::listToUnicodeString(originalFromList)) + && !m_identityManager->thatIsMe(KMime::Types::Mailbox::listToUnicodeString(originalToList)) + ) { + // Sender seems to be one of our own identities and recipient is not, + // so we assume that this is a reply to a "sent" mail where the user + // wants to add additional information for the recipient. + toList = originalToList; + } else { + // "Normal" case: reply to sender. + toList = stripMyAddressesFromAddressList(m_origMsg->to()->mailboxes(), m_identityManager); + toList += authorMailboxes(m_origMsg, m_mailingListAddresses); + } } - for (const KMime::Types::Mailbox &mailbox : qAsConst(ccRecipients)) { + for (const KMime::Types::Mailbox &mailbox : ccList) { msg->cc()->addAddress(mailbox); } } - - if (toList.isEmpty() && !recipients.isEmpty()) { - // reply to self without other recipients - toList << recipients.at(0); - } break; - } case MessageComposer::ReplyAuthor: toList = authorMailboxes(m_origMsg, m_mailingListAddresses); replyAll = false;