444932 - nsMsgMessageFlags.h should be an idl file

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Rain [:rain1] (inactive)]

This bug is in the spirit of bug 436044. Even though the message flags seem to be less used than the folder flags in JS, there's really no reason not to.

JS files with MSG_FLAG_*: http://mxr.mozilla.org/mozilla/search?string=MSG_FLAG_&find=^.*\.js&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla
(there might be more instances, where the value is directly used)

Comment 1

[Rain [:rain1] (inactive)]

Attachment: step 1: create idl, remove duplicate definitions in MailNewsTypes.h

Ugh, duplicate definitions are a pain. :(

Comment 2

[Rain [:rain1] (inactive)]

Attachment: [checked in] Updated patch to fix bitrot

A bit of bitrot is fixed.

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 357114 [details] [diff] [review]
[checked in] Updated patch to fix bitrot

Checked in: http://hg.mozilla.org/comm-central/rev/c263cff9191d

Comment 4

[Rain [:rain1] (inactive)]

Attachment: Step 2: replace C++ users

Pretty large patch, but just a bunch of global search and replace operations.

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 357665 [details] [diff] [review]
Step 2: replace C++ users

My Mac build fails with:

/Users/moztest/comm/main/src/mailnews/import/applemail/src/nsEmlxHelperUtils.mm: In static member function ‘static nsresult nsEmlxHelperUtils::ConvertToMozillaStatusFlags(const char*, const char*, PRUint32*)’:
/Users/moztest/comm/main/src/mailnews/import/applemail/src/nsEmlxHelperUtils.mm:80: error: ‘MSG_FLAG_READ’ was not declared in this scope
/Users/moztest/comm/main/src/mailnews/import/applemail/src/nsEmlxHelperUtils.mm:82: error: ‘MSG_FLAG_FORWARDED’ was not declared in this scope
/Users/moztest/comm/main/src/mailnews/import/applemail/src/nsEmlxHelperUtils.mm:84: error: ‘MSG_FLAG_REPLIED’ was not declared in this scope
/Users/moztest/comm/main/src/mailnews/import/applemail/src/nsEmlxHelperUtils.mm:86: error: ‘MSG_FLAG_MARKED’ was not declared in this scope
make[4]: *** [nsEmlxHelperUtils.o] Error 1
make[3]: *** [applemail/src_libs] Error 2
make[2]: *** [import_libs] Error 2
make[1]: *** [libs_tier_app] Error 2
make: *** [tier_app] Error 2


-    if (!(flags & MSG_FLAG_READ)) unreadDelta++;
-    if (flags & MSG_FLAG_NEW) newDelta++;
+    if (!(flags & nsMsgMessageFlags::Read)) unreadDelta++;
+    if (flags & nsMsgMessageFlags::New) newDelta++;

Please put the variable increments on a new line.

-    if (unreadDelta == -1 && aOldFlags & MSG_FLAG_NEW)
+    if (unreadDelta == -1 && aOldFlags & nsMsgMessageFlags::Read)

This doesn't look right...

-    case MSG_FLAG_FORWARDED|MSG_FLAG_REPLIED:
+    case nsMsgMessageFlags::Forwarded|nsMsgMessageFlags::Replied:

nit: space either side of the | please.

   virtual void	UpdateFolderFlag(nsIMsgDBHdr *msgHdr, PRBool bSet, 
-                                  MsgFlags flag, nsIOutputStream **ppFileStream);
-  virtual PRBool SetHdrFlag(nsIMsgDBHdr *msgHdr, PRBool bSet, MsgFlags flag);
+                                 nsMsgMessageFlagType flag, nsIOutputStream **ppFileStream);

nit: nsMsgMessageFlagType should align with the start of nsIMsgDBHdr on the line above.

 // IMAP does not set local file flags, override does nothing
 void	nsImapMailDatabase::UpdateFolderFlag(nsIMsgDBHdr * /* msgHdr */, PRBool /* bSet */,
-                                              MsgFlags /* flag */, nsIOutputStream ** /* ppFileStream */)
+                                           nsMsgMessageFlagType /* flag */, nsIOutputStream ** /* ppFileStream */)

Can you drop the tab in the UpdateFolderFlag line so that your new line aligns properly please.

         PRUint32 msgFlags = 0;
         qhdr->GetFlags(&msgFlags);
-        if ( msgFlags & MSG_FLAG_OFFLINE )
+        if ( msgFlags & nsMsgMessageFlags::Offline )
           continue;

nit: drop the extra spaces please.

-      PRUint32 mask = MSG_FLAG_READ | MSG_FLAG_REPLIED | MSG_FLAG_MARKED | MSG_FLAG_IMAP_DELETED | MSG_FLAG_LABELS;
+      PRUint32 mask = nsMsgMessageFlags::Read | nsMsgMessageFlags::Replied | nsMsgMessageFlags::Marked | nsMsgMessageFlags::IMAPDeleted | nsMsgMessageFlags::Labels;

nit: this line needs wrapping.

Comment 6

[Rain [:rain1] (inactive)]

Attachment: Step 2, revision 2

Nits fixed, and bustage hopefully fixed as well.

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 358258 [details] [diff] [review]
Step 2, revision 2

> nsresult nsMsgDBView::FetchStatus(PRUint32 aFlags, nsAString &aStatusString)
> {
>-  if(aFlags & MSG_FLAG_REPLIED)
>+  if(aFlags & nsMsgMessageFlags::Replied)
>     aStatusString = kRepliedString;
>-  else if(aFlags & MSG_FLAG_FORWARDED)
>+  else if(aFlags & nsMsgMessageFlags::Forwarded)
>     aStatusString = kForwardedString;
>-  else if(aFlags & MSG_FLAG_NEW)
>+  else if(aFlags & nsMsgMessageFlags::New)
>     aStatusString = kNewString;
>-  else if(aFlags & MSG_FLAG_READ)
>+  else if(aFlags & nsMsgMessageFlags::Read)
>     aStatusString = kReadString;

nit: please can you insert spaces after the if.

>-  if (flags & MSG_FLAG_ELIDED || !(m_viewFlags & nsMsgViewFlagsType::kThreadedDisplay) || !(flags & MSG_VIEW_FLAG_HASCHILDREN))
>-    return NS_OK;
>-  flags  |= MSG_FLAG_ELIDED;
>+  if (flags & nsMsgMessageFlags::Elided || !(m_viewFlags & nsMsgViewFlagsType::kThreadedDisplay) || !(flags & MSG_VIEW_FLAG_HASCHILDREN))
>+    return NS_OK;
>+  flags  |= nsMsgMessageFlags::Elided;

nit: just one space after flags please.

>   PRInt32 numChildren = CountExpandedThread(index);
> 
>-  *expansionDelta = (flags & MSG_FLAG_ELIDED) ? 
>+  *expansionDelta = (flags & nsMsgMessageFlags::Elided) ? 

nit: please drop the extra space from the end of the line

Comment 8

[Rain [:rain1] (inactive)]

Attachment: step 2, revision 3

more nits fixed

Comment 9

[Rain [:rain1] (inactive)]

Attachment: [checked in] step 2, revision 4

oops, missed one if(

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 358837 [details] [diff] [review]
[checked in] step 2, revision 4

Checked in: http://hg.mozilla.org/comm-central/rev/0e339aad00cf

Comment 11

[Phil Ringnalda (:philor)]

Attachment: JS uses

Surprising that it would be this easy after the variety of subtle ways we hid what we were using with nsMsgFolderFlags, but these are all I can find.

Comment 12

[[e-mail]]

Comment on attachment 368735 [details] [diff] [review]
JS uses

> function SelectedMessagesAreDeleted()
> {
>-    try {
>-        return gDBView.hdrForFirstSelectedMessage.flags & MSG_FLAG_IMAP_DELETED;
>-    }
>-    catch (ex) {
>-        return 0;
>-    }
>+    return gDBView && gDBView.numSelected &&
>+           (gDBView.hdrForFirstSelectedMessage.flags &
>+            Components.interfaces.nsMsgMessageFlags.IMAPDeleted);
> }
Thanks ;-)

Comment 13

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/comm-central/rev/ceb88f1ecea4