Bug 1866951 - Fix sort order of threads in Grouped By views. r=mkmelin

When Grouped By Date, the selected sort order is already used for the messages in each of the
groups. This patch applies this behavior to Grouped By Received as well.

For all other sort types the `mailnews.default_sort_order` or `mailnews.default_news_sort_order`
preference is now used.

Also fixes a bug where messages without `Received:` information (e.g. newsgroup messages) were
always sorted into the "Older" group.

In addition, the sort order is determined only once for each group, not for each individual
message as before.

As another simple improvement, when Grouped By Subject, "Re: " is removed from the dummy rows.

Differential Revision: https://phabricator.services.mozilla.com/D195658

--HG--
extra : amend_source : cacf5c5deb66bb8d80575969f3494053c93875c5

mailnews/base/src/nsMsgGroupThread.cpp

@@ -12,20 +12,16 @@
 
 NS_IMPL_ISUPPORTS(nsMsgGroupThread, nsIMsgThread)
 
-nsMsgGroupThread::nsMsgGroupThread() { Init(); }
-
-nsMsgGroupThread::nsMsgGroupThread(nsIMsgDatabase* db) {
-  m_db = db;
-  Init();
-}
-
-void nsMsgGroupThread::Init() {
+nsMsgGroupThread::nsMsgGroupThread(nsIMsgDatabase* db,
+                                   nsMsgViewSortOrderValue sortOrder) {
   m_threadKey = nsMsgKey_None;
   m_threadRootKey = nsMsgKey_None;
   m_numUnreadChildren = 0;
   m_flags = 0;
   m_newestMsgDate = 0;
   m_dummy = false;
+  m_db = db;
+  m_sortOrder = sortOrder;
 }
 
 nsMsgGroupThread::~nsMsgGroupThread() {}
@@ -115,27 +111,8 @@ nsMsgViewIndex nsMsgGroupThread::AddMsgHdrInDateOrder(nsIMsgDBHdr* child,
   // since we're sorted by date, we could do a binary search for the
   // insert point. Or, we could start at the end...
   if (m_keys.Length() > 0) {
-    nsMsgViewSortTypeValue sortType;
-    nsMsgViewSortOrderValue sortOrder;
-    (void)view->GetSortType(&sortType);
-    (void)view->GetSortOrder(&sortOrder);
-    // historical behavior is ascending date order unless our primary sort is
-    //  on date
-    nsMsgViewSortOrderValue threadSortOrder =
-        (sortType == nsMsgViewSortType::byDate &&
-         sortOrder == nsMsgViewSortOrder::descending)
-            ? nsMsgViewSortOrder::descending
-            : nsMsgViewSortOrder::ascending;
-    // new behavior is tricky and uses the secondary sort order if the secondary
-    //  sort is on the date
-    nsMsgViewSortTypeValue secondarySortType;
-    nsMsgViewSortOrderValue secondarySortOrder;
-    (void)view->GetSecondarySortType(&secondarySortType);
-    (void)view->GetSecondarySortOrder(&secondarySortOrder);
-    if (secondarySortType == nsMsgViewSortType::byDate)
-      threadSortOrder = secondarySortOrder;
     // sort by date within group.
-    insertIndex = GetInsertIndexFromView(view, child, threadSortOrder);
+    insertIndex = GetInsertIndexFromView(view, child, m_sortOrder);
   }
   m_keys.InsertElementAt(insertIndex, newHdrKey);
   if (!insertIndex) m_threadRootKey = newHdrKey;
@@ -654,7 +631,8 @@ NS_IMETHODIMP nsMsgGroupThread::SetNewestMsgDate(uint32_t aNewestMsgDate) {
   return NS_OK;
 }
 
-nsMsgXFGroupThread::nsMsgXFGroupThread() {}
+nsMsgXFGroupThread::nsMsgXFGroupThread(nsMsgViewSortOrderValue sortOrder)
+    : nsMsgGroupThread(nullptr, sortOrder) {}
 
 nsMsgXFGroupThread::~nsMsgXFGroupThread() {}
 

mailnews/base/src/nsMsgGroupThread.h

@@ -19,8 +19,8 @@ class nsMsgGroupThread : public nsIMsgThread {
  public:
   friend class nsMsgGroupView;
 
-  nsMsgGroupThread();
-  explicit nsMsgGroupThread(nsIMsgDatabase* db);
+  explicit nsMsgGroupThread(nsIMsgDatabase* db,
+                            nsMsgViewSortOrderValue sortOrder);
 
   NS_DECL_NSIMSGTHREAD
   NS_DECL_ISUPPORTS
@@ -28,7 +28,6 @@ class nsMsgGroupThread : public nsIMsgThread {
  protected:
   virtual ~nsMsgGroupThread();
 
-  void Init();
   nsMsgViewIndex AddChildFromGroupView(nsIMsgDBHdr* child, nsMsgDBView* view);
   nsresult RemoveChild(nsMsgKey msgKey);
   nsresult RerootThread(nsIMsgDBHdr* newParentOfOldRoot, nsIMsgDBHdr* oldRoot,
@@ -61,11 +60,12 @@ class nsMsgGroupThread : public nsIMsgThread {
   nsTArray<nsMsgKey> m_keys;
   bool m_dummy;  // top level msg is a dummy, e.g., grouped by age.
   nsCOMPtr<nsIMsgDatabase> m_db;  // should we make a weak ref or just a ptr?
+  nsMsgViewSortOrderValue m_sortOrder;
 };
 
 class nsMsgXFGroupThread : public nsMsgGroupThread {
  public:
-  nsMsgXFGroupThread();
+  explicit nsMsgXFGroupThread(nsMsgViewSortOrderValue sortOrder);
 
   NS_IMETHOD GetNumChildren(uint32_t* aNumChildren) override;
   NS_IMETHOD GetChildKeyAt(uint32_t aIndex, nsMsgKey* aResult) override;

mailnews/base/src/nsMsgGroupView.cpp

@@ -91,14 +91,16 @@ nsresult nsMsgGroupView::GetAgeBucketValue(nsIMsgDBHdr* aMsgHdr,
   NS_ENSURE_ARG_POINTER(aAgeBucket);
 
   PRTime dateOfMsg;
-  nsresult rv;
-  if (!rcvDate)
-    rv = aMsgHdr->GetDate(&dateOfMsg);
-  else {
   uint32_t rcvDateSecs;
+  nsresult rv;
+
+  // Silently return Date: instead if Received: is unavailable.
+  if (rcvDate) {
     rv = aMsgHdr->GetUint32Property("dateReceived", &rcvDateSecs);
-    Seconds2PRTime(rcvDateSecs, &dateOfMsg);
+    if (rcvDateSecs != 0) Seconds2PRTime(rcvDateSecs, &dateOfMsg);
   }
+
+  if (!rcvDate || rcvDateSecs == 0) rv = aMsgHdr->GetDate(&dateOfMsg);
   NS_ENSURE_SUCCESS(rv, rv);
 
   PRTime currentTime = PR_Now();
@@ -241,7 +243,14 @@ nsresult nsMsgGroupView::HashHdr(nsIMsgDBHdr* msgHdr, nsString& aHashKey) {
 }
 
 nsMsgGroupThread* nsMsgGroupView::CreateGroupThread(nsIMsgDatabase* db) {
-  return new nsMsgGroupThread(db);
+  nsMsgViewSortOrderValue threadSortOrder = nsMsgViewSortOrder::descending;
+  if (m_sortType == nsMsgViewSortType::byDate ||
+      m_sortType == nsMsgViewSortType::byReceived) {
+    threadSortOrder = m_sortOrder;
+  } else {
+    m_db->GetDefaultSortOrder(&threadSortOrder);
+  }
+  return new nsMsgGroupThread(db, threadSortOrder);
 }
 
 nsMsgGroupThread* nsMsgGroupView::AddHdrToThread(nsIMsgDBHdr* msgHdr,
@@ -792,7 +801,7 @@ nsMsgGroupView::CellTextForColumn(int32_t aRow, const nsAString& aColumnName,
         break;
       }
       case nsMsgViewSortType::bySubject:
-        FetchSubject(msgHdr, m_flags[aRow], aValue);
+        FetchSubject(msgHdr, m_flags[aRow] & ~nsMsgMessageFlags::HasRe, aValue);
         break;
       case nsMsgViewSortType::byAuthor:
         FetchAuthor(msgHdr, aValue);

mailnews/base/src/nsMsgSearchDBView.cpp

@@ -1274,7 +1274,17 @@ nsresult nsMsgSearchDBView::RemoveMsgFromHashTables(nsIMsgDBHdr* msgHdr) {
 
 nsMsgGroupThread* nsMsgSearchDBView::CreateGroupThread(
     nsIMsgDatabase* /* db */) {
-  return new nsMsgXFGroupThread();
+  nsMsgViewSortOrderValue threadSortOrder = nsMsgViewSortOrder::descending;
+  if (m_sortType == nsMsgViewSortType::byDate ||
+      m_sortType == nsMsgViewSortType::byReceived) {
+    threadSortOrder = m_sortOrder;
+  } else {
+    if (mozilla::Preferences::GetInt("mailnews.default_sort_order") ==
+        nsMsgViewSortOrder::ascending) {
+      threadSortOrder = nsMsgViewSortOrder::ascending;
+    }
+  }
+  return new nsMsgXFGroupThread(threadSortOrder);
 }
 
 NS_IMETHODIMP