Created attachment 55813 [details] N900 Screenshot showing messed up group sorting of Kontact Touch Mail Package: kdepim-mobile, N900 Version: 4:4.6~.20110106.1209.gitfde48d5-1maemo1.1211475 Package: libqt4-experimental-gui Version: 4.7.0~git20101111-0maemo1 Selected my INBOX this morning (Monday) after touchung it the last time on Friday where I left it with four read emails. After the explicit sync from the menu and wachting the emails coming in, I had several groups for the same day. See attached screenshot. (For demonstration purposes I collapsed the groups, this does not change the order.) INBOX: takes sync options from account Account: no offline mode; no intervall check This should stay keyword komo3 at least until we have clarified that this does not happen on the HTC Touch Pro2.
Could also reproduce the issue when searching with N900, got Dezember 2010, Mittwoch, Dezember 2010
Reproducible on Linux/Desktop... will debug it
commit b984b66bd8e090d96bcd451a9d19e4af0c5d86ca branch master Author: Tobias Koenig <tokoe@kde.org> Date: Wed Jan 12 13:51:40 2011 +0100 Fix grouping of messages in 'most recent' mode Let ThreadGrouperComparator handle the grouper string, since it knows what are the thread leader and the most recent items in a thread. BUG: 262735 diff --git a/mobile/lib/threadgroupermodel.cpp b/mobile/lib/threadgroupermodel.cpp index 8a00c09..cedb635 100644 --- a/mobile/lib/threadgroupermodel.cpp +++ b/mobile/lib/threadgroupermodel.cpp @@ -60,6 +60,11 @@ ThreadGrouperComparator::~ThreadGrouperComparator() { } +QString ThreadGrouperComparator::grouperString( const Akonadi::Item& ) const +{ + return QString(); +} + Akonadi::Item ThreadGrouperComparator::threadItem( const Akonadi::Item &item ) const { Q_ASSERT( m_grouper ); @@ -186,6 +191,9 @@ QVariant ThreadGrouperModel::data( const QModelIndex &index, int role ) const if ( role == ThreadIdRole ) return d->threadRoot( index ).id(); + else if ( role == GrouperRole ) { + return d->m_comparator->grouperString( index.data( Akonadi::EntityTreeModel::ItemRole ).value<Akonadi::Item>() ); + } return QSortFilterProxyModel::data( index, role ); } @@ -209,6 +217,10 @@ void ThreadGrouperModel::setSourceModel( QAbstractItemModel *sourceModel ) if ( d->m_dynamicModelRepopulation ) connect( sourceModel, SIGNAL( dataChanged( const QModelIndex&, const QModelIndex& ) ), this, SLOT( populateThreadGrouperModel() ) ); + + QHash<int, QByteArray> names = roleNames(); + names.insert( GrouperRole, "grouperString" ); + setRoleNames( names ); } bool ThreadGrouperModel::lessThan( const QModelIndex &left, const QModelIndex &right ) const @@ -228,6 +240,7 @@ void ThreadGrouperModel::setThreadingEnabled( bool enabled ) d->m_threadingEnabled = enabled; invalidate(); + reset(); } bool ThreadGrouperModel::threadingEnabled() const diff --git a/mobile/lib/threadgroupermodel.h b/mobile/lib/threadgroupermodel.h index ba43ea0..0e05171 100644 --- a/mobile/lib/threadgroupermodel.h +++ b/mobile/lib/threadgroupermodel.h @@ -62,6 +62,11 @@ class MOBILEUI_EXPORT ThreadGrouperComparator */ virtual bool lessThan( const Akonadi::Item &left, const Akonadi::Item &right ) const = 0; + /** + * Returns the grouper string for the given @p item. + */ + virtual QString grouperString( const Akonadi::Item &item ) const; + protected: /** * Returns the thread item for @p item. @@ -102,7 +107,8 @@ class MOBILEUI_EXPORT ThreadGrouperModel : public QSortFilterProxyModel public: enum CustomRoles { // FIXME Fix custom role handling in proxies. - ThreadIdRole = Akonadi::EntityTreeModel::UserRole + 30 + ThreadIdRole = Akonadi::EntityTreeModel::UserRole + 30, + GrouperRole }; /** diff --git a/mobile/mail/HeaderView.qml b/mobile/mail/HeaderView.qml index 7cabc62..3e99446 100644 --- a/mobile/mail/HeaderView.qml +++ b/mobile/mail/HeaderView.qml @@ -29,7 +29,6 @@ KPIM.ItemListView { property variant checkModel property string collapsedSections property bool showSections : true - property string groupingRole : "dateGroup" delegate: [ KPIM.ItemListViewDelegate { @@ -37,7 +36,7 @@ KPIM.ItemListView { showCheckBox : _top.showCheckBox checkModel : _top.checkModel navigationModel : _top.navigationModel - height : (_top.collapsedSections.indexOf(model[groupingRole]) >= 0) ? 0 : (itemListView.height / 7) + height : (_top.collapsedSections.indexOf(model.grouperString) >= 0) ? 0 : (itemListView.height / 7) clip: true summaryContent : [ QML.Text { @@ -208,7 +207,7 @@ KPIM.ItemListView { } ] - section.property: showSections ? _top.groupingRole : "" + section.property: showSections ? "grouperString" : "" section.criteria: QML.ViewSection.FullString section.delegate: QML.Item { id: sectionDelegate diff --git a/mobile/mail/kmail-mobile.qml b/mobile/mail/kmail-mobile.qml index 6c77807..66fb2fd 100644 --- a/mobile/mail/kmail-mobile.qml +++ b/mobile/mail/kmail-mobile.qml @@ -296,7 +296,6 @@ KPIM.MainView { navigationModel : _threadSelector showDeleteButton : false // too easy to accidentally hit it, although very useful... opacity : threadContentsViewContainer.opacity == 0 ? 1 : 0 - groupingRole : messageListSettings.groupingRole showSections : messageListSettings.groupingRole != "" } QML.Connections { diff --git a/mobile/mail/mailthreadgroupercomparator.cpp b/mobile/mail/mailthreadgroupercomparator.cpp index 35dec0f..c931509 100644 --- a/mobile/mail/mailthreadgroupercomparator.cpp +++ b/mobile/mail/mailthreadgroupercomparator.cpp @@ -24,6 +24,10 @@ #include <akonadi/kmime/messageflags.h> #include <messagecore/stringutil.h> +#include <klocale.h> +#include <kglobal.h> +#include <kcalendarsystem.h> + MailThreadGrouperComparator::MailThreadGrouperComparator() : mSortingOption( SortByDateTimeMostRecent ), mIsOutboundCollection( false ) @@ -87,8 +91,8 @@ bool MailThreadGrouperComparator::lessThan( const Akonadi::Item &leftItem, const break; case SortByDateTimeMostRecent: { - const KDateTime leftNewest = mostRecentUpdate( leftThreadRootMessage, leftThreadRootItem.id() ); - const KDateTime rightNewest = mostRecentUpdate( rightThreadRootMessage, rightThreadRootItem.id() ); + const KDateTime leftNewest = mostRecentDateTimeInThread( leftThreadRootMessage, leftThreadRootItem.id() ); + const KDateTime rightNewest = mostRecentDateTimeInThread( rightThreadRootMessage, rightThreadRootItem.id() ); if ( leftNewest != rightNewest ) { return leftNewest > rightNewest; @@ -183,6 +187,18 @@ MailThreadGrouperComparator::SortingOption MailThreadGrouperComparator::sortingO return mSortingOption; } +void MailThreadGrouperComparator::setGroupingOption( GroupingOption option ) +{ + mGroupingOption = option; + + invalidate(); +} + +MailThreadGrouperComparator::GroupingOption MailThreadGrouperComparator::groupingOption() const +{ + return mGroupingOption; +} + void MailThreadGrouperComparator::setIsOutboundCollection( bool outbound ) { mIsOutboundCollection = outbound; @@ -190,6 +206,72 @@ void MailThreadGrouperComparator::setIsOutboundCollection( bool outbound ) invalidate(); } +QString MailThreadGrouperComparator::grouperString( const Akonadi::Item &item ) const +{ + KMime::Message::Ptr msg; + + if ( mSortingOption == SortByDateTimeMostRecent ) { + const Akonadi::Item::Id newestItem = mostRecentIdInThread( messageForItem( item ), item.id() ); + msg = messageForItem( Akonadi::Item( newestItem ) ); + } else { + const Akonadi::Item rootItem = threadItem( item ); + msg = messageForItem( rootItem ); + } + + if ( mGroupingOption == GroupByDate ) { + // simplified version taken from libmessagelist + const KDateTime& dt = msg->date()->dateTime(); + const QDate dDate = dt.date(); + const KCalendarSystem *calendar = KGlobal::locale()->calendar(); + int daysAgo = -1; + if ( calendar->isValid( dDate ) && calendar->isValid( QDate::currentDate() ) ) { + daysAgo = dDate.daysTo( QDate::currentDate() ); + } + + if ( daysAgo < 0 || !dt.isValid() ) // In the future or invalid + return i18n( "Unknown" ); + else if( daysAgo == 0 ) // Today + return i18n( "Today" ); + else if ( daysAgo == 1 ) // Yesterday + return i18n( "Yesterday" ); + else if ( daysAgo > 1 && daysAgo < calendar->daysInWeek( QDate::currentDate() ) ) // Within last seven days + return KGlobal::locale()->calendar()->weekDayName( dDate ); + else if( calendar->month( dDate ) == calendar->month( QDate::currentDate() ) && calendar->year( dDate ) == calendar->year( QDate::currentDate() ) ) { // within this month + const int startOfWeekDaysAgo = ( calendar->daysInWeek( QDate::currentDate() ) + calendar->dayOfWeek( QDate::currentDate() ) - + KGlobal::locale()->weekStartDay() ) % calendar->daysInWeek( QDate::currentDate() ); + const int weeksAgo = ( ( daysAgo - startOfWeekDaysAgo ) / calendar->daysInWeek( QDate::currentDate() ) ) + 1; + if ( weeksAgo == 0 ) + return KGlobal::locale()->calendar()->weekDayName( dDate ); + else + return i18np( "One Week Ago", "%1 Weeks Ago", weeksAgo ); + } else if ( calendar->year( dDate ) == calendar->year( QDate::currentDate() ) ) { // within this year + return calendar->monthName( dDate ); + } else { // in previous years + static QHash<int, QString> yearNameHash; + + QString yearName; + if ( yearNameHash.contains( dDate.year() ) ) { + yearName = yearNameHash.value( dDate.year() ); + } else { + yearName = calendar->yearString( dDate ); + yearNameHash.insert( dDate.year(), yearName ); + } + return i18nc( "Message Aggregation Group Header: Month name and Year number", "%1 %2", calendar->monthName( dDate ), yearName ); + } + } else if ( mGroupingOption == GroupBySenderReceiver ) { + QStringList l; + foreach ( const KMime::Types::Mailbox &mbox, msg->from()->mailboxes() ) { + if ( mbox.hasName() ) + l.append( mbox.name() ); + else + l.append( mbox.addrSpec().asPrettyString() ); + } + return l.join( ", " ); + } else { + return QLatin1String( "dummy" ); + } +} + void MailThreadGrouperComparator::resetCaches() { mMessageCache.clear(); @@ -205,18 +287,22 @@ QByteArray MailThreadGrouperComparator::identifierForMessage( const KMime::Messa return identifier; } -KDateTime MailThreadGrouperComparator::mostRecentUpdate( const KMime::Message::Ptr &threadRoot, Akonadi::Item::Id itemId ) const +KDateTime MailThreadGrouperComparator::mostRecentDateTimeInThread( const KMime::Message::Ptr &threadRoot, Akonadi::Item::Id itemId ) const { - const QHash<Akonadi::Item::Id, KDateTime>::const_iterator it = mMostRecentCache.constFind( itemId ); + const QHash<Akonadi::Item::Id, MostRecentEntry>::const_iterator it = mMostRecentCache.constFind( itemId ); if ( it != mMostRecentCache.constEnd() ) - return *it; + return (*it).dateTime; const QSet<QByteArray> messageIds = threadDescendants( identifierForMessage( threadRoot, itemId ) ); KDateTime newest = threadRoot->date()->dateTime(); + Akonadi::Item::Id newestId = itemId; if ( messageIds.isEmpty() ) { - mMostRecentCache.insert( itemId, newest ); + MostRecentEntry entry; + entry.id = newestId; + entry.dateTime = newest; + mMostRecentCache.insert( itemId, entry ); return newest; } @@ -227,14 +313,59 @@ KDateTime MailThreadGrouperComparator::mostRecentUpdate( const KMime::Message::P const KMime::Message::Ptr message = messageForItem( item ); const KDateTime messageDateTime = message->date()->dateTime(); - if ( messageDateTime > newest ) + if ( messageDateTime > newest ) { newest = messageDateTime; + newestId = item.id(); + } } - mMostRecentCache.insert( itemId, newest ); + MostRecentEntry entry; + entry.id = newestId; + entry.dateTime = newest; + + mMostRecentCache.insert( itemId, entry ); return newest; } +Akonadi::Item::Id MailThreadGrouperComparator::mostRecentIdInThread( const KMime::Message::Ptr &threadRoot, Akonadi::Item::Id itemId ) const +{ + const QHash<Akonadi::Item::Id, MostRecentEntry>::const_iterator it = mMostRecentCache.constFind( itemId ); + if ( it != mMostRecentCache.constEnd() ) + return (*it).id; + + const QSet<QByteArray> messageIds = threadDescendants( identifierForMessage( threadRoot, itemId ) ); + + KDateTime newest = threadRoot->date()->dateTime(); + Akonadi::Item::Id newestId = itemId; + + if ( messageIds.isEmpty() ) { + MostRecentEntry entry; + entry.id = newestId; + entry.dateTime = newest; + mMostRecentCache.insert( itemId, entry ); + return itemId; + } + + foreach ( const QByteArray &messageId, messageIds ) { + const Akonadi::Item item = itemForIdentifier( messageId ); + Q_ASSERT( item.isValid() ); + Q_ASSERT( item.hasPayload<KMime::Message::Ptr>() ); + + const KMime::Message::Ptr message = messageForItem( item ); + const KDateTime messageDateTime = message->date()->dateTime(); + if ( messageDateTime > newest ) + newest = messageDateTime; + newestId = item.id(); + } + + MostRecentEntry entry; + entry.id = newestId; + entry.dateTime = newest; + + mMostRecentCache.insert( itemId, entry ); + return itemId; +} + KMime::Message::Ptr MailThreadGrouperComparator::messageForItem( const Akonadi::Item &item ) const { const QHash<Akonadi::Item::Id, KMime::Message::Ptr>::const_iterator it = mMessageCache.constFind( item.id() ); diff --git a/mobile/mail/mailthreadgroupercomparator.h b/mobile/mail/mailthreadgroupercomparator.h index 5c3d696..4fa41e4 100644 --- a/mobile/mail/mailthreadgroupercomparator.h +++ b/mobile/mail/mailthreadgroupercomparator.h @@ -39,6 +39,13 @@ class MailThreadGrouperComparator : public ThreadGrouperComparator SortByActionItem }; + enum GroupingOption + { + GroupByNone, + GroupByDate, + GroupBySenderReceiver + }; + /** * Creates a new mail thread grouper comparator. */ @@ -67,24 +74,38 @@ class MailThreadGrouperComparator : public ThreadGrouperComparator void setSortingOption( SortingOption option ); SortingOption sortingOption() const; + void setGroupingOption( GroupingOption option ); + GroupingOption groupingOption() const; + /** * Sets whether the currently compared items come from an outbound mail collection * (e.g. outbox, sent or drafts). */ void setIsOutboundCollection( bool outbound ); + virtual QString grouperString( const Akonadi::Item &item ) const; + protected: virtual void resetCaches(); private: QByteArray identifierForMessage( const KMime::Message::Ptr&, Akonadi::Item::Id ) const; - KDateTime mostRecentUpdate( const KMime::Message::Ptr&, Akonadi::Item::Id ) const; + KDateTime mostRecentDateTimeInThread( const KMime::Message::Ptr&, Akonadi::Item::Id ) const; + Akonadi::Item::Id mostRecentIdInThread( const KMime::Message::Ptr&, Akonadi::Item::Id ) const; KMime::Message::Ptr messageForItem( const Akonadi::Item &item ) const; SortingOption mSortingOption; + GroupingOption mGroupingOption; bool mIsOutboundCollection; mutable QHash<Akonadi::Item::Id, KMime::Message::Ptr> mMessageCache; - mutable QHash<Akonadi::Item::Id, KDateTime> mMostRecentCache; + + struct MostRecentEntry + { + Akonadi::Item::Id id; + KDateTime dateTime; + }; + + mutable QHash<Akonadi::Item::Id, MostRecentEntry> mMostRecentCache; }; #endif diff --git a/mobile/mail/mainview.cpp b/mobile/mail/mainview.cpp index 35f0d14..cdfae0e 100644 --- a/mobile/mail/mainview.cpp +++ b/mobile/mail/mainview.cpp @@ -1753,6 +1753,18 @@ void MainView::messageListSettingsChanged( const MessageListSettings &settings ) break; } + switch ( settings.groupingOption() ) { + case MessageListSettings::GroupByDate: + m_grouperComparator->setGroupingOption( MailThreadGrouperComparator::GroupByDate ); + break; + case MessageListSettings::GroupBySenderReceiver: + m_grouperComparator->setGroupingOption( MailThreadGrouperComparator::GroupBySenderReceiver ); + break; + case MessageListSettings::GroupByNone: + m_grouperComparator->setGroupingOption( MailThreadGrouperComparator::GroupByNone ); + break; + } + m_threadGrouperModel->setThreadingEnabled( settings.useThreading() ); m_threadGrouperModel->sort( 0, settings.sortingOrder() );
It does not seem to be fixed completely with windows-ce/Kontact-Mobile/testing/2011-01-17-02-23/ I still get groups like: "Mittwoch" "Vor 2 Wochen" "Dezember 2010" Vor 2 Wochen" "Dezember 2010"
Reproduced with a freshly configured account, just selected INBOX and stayed in there until it was synced.
Maillist view settings: Sorting: "nach neuster nachricht in der diskussion" "aufsteigend" [ ] not checked "Ordner verwendet immer diese Sortierung" Grouping: "nach neuster nachricht in der diskussion" [X] checked "Diskussionen anzeigen"
Second time I've started the account, the grouping order was fine, so I _guess_ it only happens when (several) emails come in. A situation which can happen during any sync.
Git commit f947dd12a27fe1a79a20416a085592073988bbd0 by Tobias Koenig. Committed on 02/02/11 at 17:54. Pushed by tokoe into branch 'komo3'. Fix sorting/grouping of incoming messages We have to resort the message list whenever new messages are inserted to make it work in 'most-recent-message-in-thread' mode. BUG: 262735 M +9 -0 mobile/lib/threadgroupermodel.cpp M +1 -0 mobile/lib/threadgroupermodel.h M +2 -1 mobile/mail/mailthreadgroupercomparator.cpp http://commits.kde.org/kdepim/f947dd12a27fe1a79a20416a085592073988bbd0
Git commit 5a808eb3b87641f372f0ee4ebb336e38d3fb8521 by Tobias Koenig. Committed on 02/02/11 at 17:54. Pushed by tokoe into branch 'master'. Fix sorting/grouping of incoming messages We have to resort the message list whenever new messages are inserted to make it work in 'most-recent-message-in-thread' mode. BUG: 262735 M +9 -0 mobile/lib/threadgroupermodel.cpp M +1 -0 mobile/lib/threadgroupermodel.h M +2 -1 mobile/mail/mailthreadgroupercomparator.cpp http://commits.kde.org/kdepim/5a808eb3b87641f372f0ee4ebb336e38d3fb8521
Seems to be fixed in 4:4.6~.20110214.1233.gitb901f2e-1maemo1.1219908 on maemo