Bug 1596371 - Fix regression introduced by bug 583677. Improve handling of tags on shared folders. r=mkmelin
authorGene Smith <gds@chartertn.net>
Tue, 10 Dec 2019 23:03:01 +0100
changeset 37833 047a04a5198d3e0d1862279a25b19e549c67782d
parent 37832 0c56d5bd0e5d0b73e18327ec58ba10c36c5eecac
child 37834 ebbfde7f5adde2b8e22f5366cb2cf3569e112f07
push id397
push userclokep@gmail.com
push dateMon, 10 Feb 2020 21:16:13 +0000
reviewersmkmelin
bugs1596371, 583677
Bug 1596371 - Fix regression introduced by bug 583677. Improve handling of tags on shared folders. r=mkmelin
mailnews/imap/public/nsIImapFlagAndUidState.idl
mailnews/imap/src/nsImapFlagAndUidState.cpp
mailnews/imap/src/nsImapFlagAndUidState.h
mailnews/imap/src/nsImapMailFolder.cpp
mailnews/imap/src/nsImapMailFolder.h
mailnews/imap/src/nsImapServerResponseParser.cpp
--- a/mailnews/imap/public/nsIImapFlagAndUidState.idl
+++ b/mailnews/imap/public/nsIImapFlagAndUidState.idl
@@ -65,12 +65,10 @@ interface nsIImapFlagAndUidState : nsISu
    * Gets the custom attributes from the hash table where they were stored earlier
    * them.
    * @param aUid   UID of the associated msg
    * @param aCustomAttributeName   Name of the custom attribute value
    * @param aCustomAttributeValue   Value of the attribute,
    */
   ACString getCustomAttribute(in unsigned long aUid,
                               in ACString aCustomAttributeName);
-  void setOtherKeywords(in unsigned short index, in AUTF8String otherKeyword);
-  AUTF8String getOtherKeywords(in unsigned short index);
 };
 
--- a/mailnews/imap/src/nsImapFlagAndUidState.cpp
+++ b/mailnews/imap/src/nsImapFlagAndUidState.cpp
@@ -94,35 +94,16 @@ nsImapFlagAndUidState::OrSupportedUserFl
 
 NS_IMETHODIMP
 nsImapFlagAndUidState::GetSupportedUserFlags(uint16_t *aFlags) {
   NS_ENSURE_ARG_POINTER(aFlags);
   *aFlags = fSupportedUserFlags;
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsImapFlagAndUidState::SetOtherKeywords(uint16_t index,
-                                        const nsACString &otherKeyword) {
-  if (index == 0) fOtherKeywords.Clear();
-  nsAutoCString flag(otherKeyword);
-  ToLowerCase(flag);
-  fOtherKeywords.AppendElement(flag);
-  return NS_OK;
-}
-
-NS_IMETHODIMP
-nsImapFlagAndUidState::GetOtherKeywords(uint16_t index, nsACString &aKeyword) {
-  if (index < fOtherKeywords.Length())
-    aKeyword = fOtherKeywords[index];
-  else
-    aKeyword = EmptyCString();
-  return NS_OK;
-}
-
 // we need to reset our flags, (re-read all) but chances are the memory
 // allocation needed will be very close to what we were already using
 
 NS_IMETHODIMP nsImapFlagAndUidState::Reset() {
   PR_CEnterMonitor(this);
   fNumberDeleted = 0;
   m_customFlagsHash.Clear();
   fUids.Clear();
--- a/mailnews/imap/src/nsImapFlagAndUidState.h
+++ b/mailnews/imap/src/nsImapFlagAndUidState.h
@@ -46,14 +46,12 @@ class nsImapFlagAndUidState : public nsI
   nsDataHashtable<nsUint32HashKey, nsCString> m_customFlagsHash;
   // Hash table, mapping UID+customAttributeName to customAttributeValue.
   nsDataHashtable<nsCStringHashKey, nsCString> m_customAttributesHash;
   uint16_t fSupportedUserFlags;
   int32_t fNumberDeleted;
   bool fPartialUIDFetch;
   uint32_t fNumAdded;
   bool fStartCapture;
-  // Keywords (aka, tags) in FLAGS response to SELECT defined by other clients
-  nsTArray<nsCString> fOtherKeywords;
   mozilla::Mutex mLock;
 };
 
 #endif
--- a/mailnews/imap/src/nsImapMailFolder.cpp
+++ b/mailnews/imap/src/nsImapMailFolder.cpp
@@ -4145,18 +4145,17 @@ void nsImapMailFolder::TweakHeaderFlags(
       if (imap_flags & kImapMsgLabelFlags) {
         // we need to set label attribute on header because the dbview code
         // does msgHdr->GetLabel when asked to paint a row
         tweakMe->SetLabel((imap_flags & kImapMsgLabelFlags) >> 9);
         newFlags |= (imap_flags & kImapMsgLabelFlags) << 16;
       }
       if (newFlags) tweakMe->OrFlags(newFlags, &dbHdrFlags);
       if (!customFlags.IsEmpty())
-        (void)HandleCustomFlags(m_curMsgUid, tweakMe, userFlags, customFlags,
-                                nullptr);
+        (void)HandleCustomFlags(m_curMsgUid, tweakMe, userFlags, customFlags);
     }
   }
 }
 
 NS_IMETHODIMP
 nsImapMailFolder::SetupMsgWriteStream(nsIFile *aFile, bool addDummyEnvelope) {
   nsresult rv;
   aFile->Remove(false);
@@ -4423,19 +4422,20 @@ NS_IMETHODIMP
 nsImapMailFolder::ReleaseUrlCacheEntry(nsIMsgMailNewsUrl *aUrl) {
   NS_ENSURE_ARG_POINTER(aUrl);
   return aUrl->SetMemCacheEntry(nullptr);
 }
 
 NS_IMETHODIMP
 nsImapMailFolder::BeginMessageUpload() { return NS_ERROR_FAILURE; }
 
-nsresult nsImapMailFolder::HandleCustomFlags(
-    nsMsgKey uidOfMessage, nsIMsgDBHdr *dbHdr, uint16_t userFlags,
-    nsCString &keywords, nsIImapFlagAndUidState *flagState) {
+nsresult nsImapMailFolder::HandleCustomFlags(nsMsgKey uidOfMessage,
+                                             nsIMsgDBHdr *dbHdr,
+                                             uint16_t userFlags,
+                                             nsCString &keywords) {
   nsresult rv = GetDatabase();
   NS_ENSURE_SUCCESS(rv, rv);
 
   ToLowerCase(keywords);
   bool messageClassified = true;
   // Mac Mail uses "NotJunk"
   if (keywords.Find("NonJunk", /* ignoreCase = */ true) != kNotFound ||
       keywords.Find("NotJunk", /* ignoreCase = */ true) != kNotFound) {
@@ -4457,69 +4457,69 @@ nsresult nsImapMailFolder::HandleCustomF
     // only set the junkscore origin if it wasn't set before.
     nsCString existingProperty;
     dbHdr->GetStringProperty("junkscoreorigin",
                              getter_Copies(existingProperty));
     if (existingProperty.IsEmpty())
       dbHdr->SetStringProperty("junkscoreorigin", "imapflag");
   }
 
-  if (flagState && !(userFlags & kImapMsgSupportUserFlag)) {
+  if (!(userFlags & kImapMsgSupportUserFlag)) {
     nsCString localKeywords;
-    if (!(userFlags & kImapMsgSupportUserFlag)) {
-      dbHdr->GetStringProperty("keywords", getter_Copies(localKeywords));
-      MOZ_LOG(IMAP_KW, mozilla::LogLevel::Debug,
-              ("UID=%" PRIu32 ", localKeywords=|%s| rcvdKeyword=|%s|",
-               uidOfMessage, localKeywords.get(), keywords.get()));
-    }
+    nsCString prevKeywords;
+    dbHdr->GetStringProperty("keywords", getter_Copies(localKeywords));
+    dbHdr->GetStringProperty("prevkeywords", getter_Copies(prevKeywords));
+    // localKeywords: tags currently stored in database for the message.
+    // keywords: tags stored in server and obtained when flags for the message
+    //           were last fetched. (Parameter of this function.)
+    // prevKeywords: saved keywords from previous call of this function.
+    // clang-format off
+    MOZ_LOG(IMAP_KW, mozilla::LogLevel::Debug,
+            ("UID=%" PRIu32 ", localKeywords=|%s| keywords=|%s|, prevKeywords=|%s|",
+             uidOfMessage, localKeywords.get(), keywords.get(), prevKeywords.get()));
+    // clang-format on
+
+    // Store keywords to detect changes on next call of this function.
+    dbHdr->SetStringProperty("prevkeywords", keywords.get());
+
+    // Parse the space separated strings into arrays.
     nsTArray<nsCString> localKeywordArray;
-    nsTArray<nsCString> rcvdKeywordArray;
+    nsTArray<nsCString> keywordArray;
+    nsTArray<nsCString> prevKeywordArray;
     ParseString(localKeywords, ' ', localKeywordArray);
-    ParseString(keywords, ' ', rcvdKeywordArray);
-
-    nsAutoCString mozLogDefinedKWs;
-    if (MOZ_LOG_TEST(IMAP_KW, mozilla::LogLevel::Debug))
-      mozLogDefinedKWs.AppendLiteral("Defined keywords = |");
-    uint32_t i = 0;
-    while (true) {
-      nsAutoCString definedKeyword;
-      flagState->GetOtherKeywords(i++, definedKeyword);
-      if (definedKeyword.IsEmpty()) {
-        if (MOZ_LOG_TEST(IMAP_KW, mozilla::LogLevel::Debug)) {
-          mozLogDefinedKWs.Append('|');
-          MOZ_LOG(IMAP_KW, mozilla::LogLevel::Debug,
-                  ("%s", mozLogDefinedKWs.get()));
-        }
-        break;
-      }
-
-      if (MOZ_LOG_TEST(IMAP_KW, mozilla::LogLevel::Debug)) {
-        mozLogDefinedKWs.Append(definedKeyword.get());
-        mozLogDefinedKWs.Append(' ');
-      }
-
-      bool inLocal = localKeywordArray.Contains(definedKeyword);
-      bool inRcvd = rcvdKeywordArray.Contains(definedKeyword);
-      if (inLocal && inRcvd) rcvdKeywordArray.RemoveElement(definedKeyword);
-      if (inLocal && !inRcvd) localKeywordArray.RemoveElement(definedKeyword);
-    }
+    ParseString(keywords, ' ', keywordArray);
+    ParseString(prevKeywords, ' ', prevKeywordArray);
+
+    // If keyword not received now but was the last time, remove it from
+    // the localKeywords. This means the keyword was removed by another user
+    // sharing the folder.
+    for (uint32_t i = 0; i < prevKeywordArray.Length(); i++) {
+      bool inRcvd = keywordArray.Contains(prevKeywordArray[i]);
+      bool inLocal = localKeywordArray.Contains(prevKeywordArray[i]);
+      if (!inRcvd && inLocal)
+        localKeywordArray.RemoveElement(prevKeywordArray[i]);
+    }
+
     // Combine local and rcvd keyword arrays into a single string
     // so it can be passed to SetStringProperty(). If element of
     // local already in rcvd, avoid duplicates in combined string.
     nsAutoCString combinedKeywords;
-    for (i = 0; i < localKeywordArray.Length(); i++) {
-      if (!rcvdKeywordArray.Contains(localKeywordArray[i])) {
+    for (uint32_t i = 0; i < localKeywordArray.Length(); i++) {
+      if (!keywordArray.Contains(localKeywordArray[i])) {
         combinedKeywords.Append(localKeywordArray[i]);
         combinedKeywords.Append(' ');
       }
     }
-    for (i = 0; i < rcvdKeywordArray.Length(); i++) {
-      combinedKeywords.Append(rcvdKeywordArray[i]);
+    for (uint32_t i = 0; i < keywordArray.Length(); i++) {
+      combinedKeywords.Append(keywordArray[i]);
       combinedKeywords.Append(' ');
     }
+    MOZ_LOG(IMAP_KW, mozilla::LogLevel::Debug,
+            ("combinedKeywords stored = |%s|", combinedKeywords.get()));
+    // combinedKeywords are tags being stored in database for the message.
     return dbHdr->SetStringProperty("keywords", combinedKeywords.get());
   }
   return (userFlags & kImapMsgSupportUserFlag)
              ? dbHdr->SetStringProperty("keywords", keywords.get())
              : NS_OK;
 }
 
 // synchronize the message flags in the database with the server flags
@@ -4556,18 +4556,17 @@ nsresult nsImapMailFolder::SyncFlags(nsI
 
     rv = mDatabase->GetMsgHdrForKey(uidOfMessage, getter_AddRefs(dbHdr));
     if (NS_SUCCEEDED(dbHdr->GetMessageSize(&messageSize)))
       newFolderSize += messageSize;
 
     nsCString keywords;
     if (NS_SUCCEEDED(
             flagState->GetCustomFlags(uidOfMessage, getter_Copies(keywords))))
-      HandleCustomFlags(uidOfMessage, dbHdr, supportedUserFlags, keywords,
-                        flagState);
+      HandleCustomFlags(uidOfMessage, dbHdr, supportedUserFlags, keywords);
 
     NotifyMessageFlagsFromHdr(dbHdr, uidOfMessage, flags);
   }
   if (!partialUIDFetch && newFolderSize != mFolderSize) {
     int64_t oldFolderSize = mFolderSize;
     mFolderSize = newFolderSize;
     NotifyIntPropertyChanged(kFolderSize, oldFolderSize, mFolderSize);
   }
@@ -4658,17 +4657,17 @@ nsImapMailFolder::NotifyMessageFlags(uin
     // GetMsgHdrForKey will create the header if it doesn't exist.
     if (NS_FAILED(rv) || !containsKey) return rv;
     rv = mDatabase->GetMsgHdrForKey(aMsgKey, getter_AddRefs(dbHdr));
     if (NS_SUCCEEDED(rv) && dbHdr) {
       uint32_t supportedUserFlags;
       GetSupportedUserFlags(&supportedUserFlags);
       NotifyMessageFlagsFromHdr(dbHdr, aMsgKey, aFlags);
       nsCString keywords(aKeywords);
-      HandleCustomFlags(aMsgKey, dbHdr, supportedUserFlags, keywords, nullptr);
+      HandleCustomFlags(aMsgKey, dbHdr, supportedUserFlags, keywords);
     }
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsImapMailFolder::NotifyMessageDeleted(const char *onlineFolderName,
                                        bool deleteAllMsgs,
--- a/mailnews/imap/src/nsImapMailFolder.h
+++ b/mailnews/imap/src/nsImapMailFolder.h
@@ -384,18 +384,17 @@ class nsImapMailFolder : public nsMsgDBF
   void FindKeysToDelete(const nsTArray<nsMsgKey> &existingKeys,
                         nsTArray<nsMsgKey> &keysToFetch,
                         nsIImapFlagAndUidState *flagState, uint32_t boxFlags);
   void PrepareToAddHeadersToMailDB(nsIImapProtocol *aProtocol);
   void TweakHeaderFlags(nsIImapProtocol *aProtocol, nsIMsgDBHdr *tweakMe);
 
   nsresult SyncFlags(nsIImapFlagAndUidState *flagState);
   nsresult HandleCustomFlags(nsMsgKey uidOfMessage, nsIMsgDBHdr *dbHdr,
-                             uint16_t userFlags, nsCString &keywords,
-                             nsIImapFlagAndUidState *flagState);
+                             uint16_t userFlags, nsCString &keywords);
   nsresult NotifyMessageFlagsFromHdr(nsIMsgDBHdr *dbHdr, nsMsgKey msgKey,
                                      uint32_t flags);
 
   nsresult SetupHeaderParseStream(uint32_t size, const nsACString &content_type,
                                   nsIMailboxSpec *boxSpec);
   nsresult ParseAdoptedHeaderLine(const char *messageLine, nsMsgKey msgKey);
   nsresult NormalEndHeaderParseStream(nsIImapProtocol *aProtocol,
                                       nsIImapUrl *imapUrl);
--- a/mailnews/imap/src/nsImapServerResponseParser.cpp
+++ b/mailnews/imap/src/nsImapServerResponseParser.cpp
@@ -1591,18 +1591,16 @@ void nsImapServerResponseParser::text_mi
  text            ::= 1*TEXT_CHAR
 
 */
 void nsImapServerResponseParser::text() { skip_to_CRLF(); }
 
 void nsImapServerResponseParser::parse_folder_flags(bool calledForFlags) {
   uint16_t labelFlags = 0;
   uint16_t junkNotJunkFlags = 0;
-  bool storeUserFlags = calledForFlags && fFlagState;
-  uint16_t numOtherKeywords = 0;
 
   do {
     AdvanceToNextToken();
     if (*fNextToken == '(') fNextToken++;
     if (!PL_strncasecmp(fNextToken, "\\Seen", 5))
       fSettablePermanentFlags |= kImapMsgSeenFlag;
     else if (!PL_strncasecmp(fNextToken, "\\Answered", 9))
       fSettablePermanentFlags |= kImapMsgAnsweredFlag;
@@ -1614,64 +1612,44 @@ void nsImapServerResponseParser::parse_f
       fSettablePermanentFlags |= kImapMsgDraftFlag;
     else if (!PL_strncasecmp(fNextToken, "\\*", 2)) {
       // User defined and special keywords (tags) can be defined and set for
       // mailbox. Should only occur in PERMANENTFLAGS response.
       fSupportsUserDefinedFlags |= kImapMsgSupportUserFlag;
       fSupportsUserDefinedFlags |= kImapMsgSupportForwardedFlag;
       fSupportsUserDefinedFlags |= kImapMsgSupportMDNSentFlag;
       fSupportsUserDefinedFlags |= kImapMsgLabelFlags;
-    } else {
-      // Treat special and built-in $LabelX's as user defined if a
-      // store occurs below. Include $Junk/$NotJunk in this too.
-      if (!PL_strncasecmp(fNextToken, "$MDNSent", 8))
-        fSupportsUserDefinedFlags |= kImapMsgSupportMDNSentFlag;
-      else if (!PL_strncasecmp(fNextToken, "$Forwarded", 10))
-        fSupportsUserDefinedFlags |= kImapMsgSupportForwardedFlag;
-      else if (!PL_strncasecmp(fNextToken, "$Label1", 7))
-        labelFlags |= 1;
-      else if (!PL_strncasecmp(fNextToken, "$Label2", 7))
-        labelFlags |= 2;
-      else if (!PL_strncasecmp(fNextToken, "$Label3", 7))
-        labelFlags |= 4;
-      else if (!PL_strncasecmp(fNextToken, "$Label4", 7))
-        labelFlags |= 8;
-      else if (!PL_strncasecmp(fNextToken, "$Label5", 7))
-        labelFlags |= 16;
-      else if (!PL_strncasecmp(fNextToken, "$Junk", 5))
-        junkNotJunkFlags |= 1;
-      else if (!PL_strncasecmp(fNextToken, "$NotJunk", 8))
-        junkNotJunkFlags |= 2;
-
-      // Store user keywords defined for mailbox, usually by other clients.
-      // But only do this for FLAGS response, not PERMANENTFLAGS response.
-      // This is only needed if '\*' does not appear in a PERMANENTFLAGS
-      // response indicating the user defined keywords are not allowed. But this
-      // is not known until this function is called for PERMANENTFLAGS which
-      // typically occurs after FLAGS, so must store them regardless.
-      if (storeUserFlags && *fNextToken != '\r') {
-        if (*(fNextToken + strlen(fNextToken) - 1) != ')') {
-          // Token doesn't end in ')' so save it as is.
-          fFlagState->SetOtherKeywords(numOtherKeywords++,
-                                       nsDependentCString(fNextToken));
-        } else {
-          // Token ends in ')' so end of list. Save all but ending ')'.
-          fFlagState->SetOtherKeywords(
-              numOtherKeywords++,
-              nsDependentCSubstring(fNextToken, strlen(fNextToken) - 1));
-        }
-      }
     }
+    // Treat special and built-in $LabelX's as user defined and include
+    // $Junk/$NotJunk too.
+    else if (!PL_strncasecmp(fNextToken, "$MDNSent", 8))
+      fSupportsUserDefinedFlags |= kImapMsgSupportMDNSentFlag;
+    else if (!PL_strncasecmp(fNextToken, "$Forwarded", 10))
+      fSupportsUserDefinedFlags |= kImapMsgSupportForwardedFlag;
+    else if (!PL_strncasecmp(fNextToken, "$Label1", 7))
+      labelFlags |= 1;
+    else if (!PL_strncasecmp(fNextToken, "$Label2", 7))
+      labelFlags |= 2;
+    else if (!PL_strncasecmp(fNextToken, "$Label3", 7))
+      labelFlags |= 4;
+    else if (!PL_strncasecmp(fNextToken, "$Label4", 7))
+      labelFlags |= 8;
+    else if (!PL_strncasecmp(fNextToken, "$Label5", 7))
+      labelFlags |= 16;
+    else if (!PL_strncasecmp(fNextToken, "$Junk", 5))
+      junkNotJunkFlags |= 1;
+    else if (!PL_strncasecmp(fNextToken, "$NotJunk", 8))
+      junkNotJunkFlags |= 2;
   } while (!fAtEndOfLine && ContinueParse());
 
   if (labelFlags == 31) fSupportsUserDefinedFlags |= kImapMsgLabelFlags;
 
   if (fFlagState) fFlagState->OrSupportedUserFlags(fSupportsUserDefinedFlags);
 
-  if (storeUserFlags) {
+  if (calledForFlags) {
     // Set true if both "$Junk" and "$NotJunk" appear in FLAGS.
     fStdJunkNotJunkUseOk = (junkNotJunkFlags == 3);
   }
 }
 /*
   resp_text_code  ::= ("ALERT" / "PARSE" /
                               "PERMANENTFLAGS" SPACE "(" #(flag / "\*") ")" /
                               "READ-ONLY" / "READ-WRITE" / "TRYCREATE" /