112099 - Make sure Namespace works well enough for Shared Folders to work

Status: VERIFIED FIXED

(MailNews Core :: Networking: IMAP, defect, P2)

Description

[scottputterman]

Need to ensure that the namespace feature works well enough for Shared Folders.
 This could end up being another meta bug for Namespace work that ends up being
done or it could end up being a Worksforme if we're finished.

Comment 1

[David :Bienvenu]

Attachment: much of the namespace code

Comment 2

[(not reading, please use [e-mail] instead)]

you are a coding machime.

here are eight nits, address them and then sr=sspitzer

1)

+  return (host == nsnull) ? NS_ERROR_ILLEGAL_VALUE : NS_OK;

why not

return host ? NS_OK : NS_ERROR_ILLEGAL_VALUE;

2)

+#include "nsPrintfCString.h"

is that needed?

3)

+    aFolderProps->SetFolderPermissions(rightsString.get());
+  //    void setFolderTypeDescription(in string folderTypeDescription);
+  //    void setFolderPermissions(in string permissions);

what's up with those two comments?

4)

+      m_aclCount--;
+#ifdef OSF1
+      /*** m_aclCount is an unsigned so this ia always true ??? ***/
+      /*** Typo?, if not, then why bother. Dropping from OSF1   ***/
+      /*** to clean up compiler warnings                        ***/
+      // XP_ASSERT(m_aclCount >= 0);
+#else
+      NS_ASSERTION(m_aclCount >= 0, "acl count can't go negative");
+#endif

m_aclCount is an PRUint32.

how about this?

NS_ASSERTION(m_aclCount, "acl count can't go negative");
m_aclCount--;

or

NS_ASSERTION(m_aclCount, "acl count can't go negative");
if (m_aclCount)
  m_aclCount--;

5)  

if (rights.Length()) rights.AppendWithConversion(", ");

do

if (rights.Length())
  rights += NS_LITERAL_STRING(", ");

6)


+  PRBool          m_folderIsNamespace;
+  nsIMAPNamespace  *m_namespace;	// Opaque pointer to the IMAP namespace 
for this folder
+										
	// Use libnet accessors for various namespace functionality
   PRBool m_folderNeedsSubscribing;
   PRBool m_folderNeedsAdded;
   PRBool m_folderNeedsACLListed;
 
+  nsMsgIMAPFolderACL *m_folderACL;
+
   nsCOMPtr<nsIMsgMailNewsUrl> mUrlToRelease;
 
   // offline imap support
   PRBool m_downloadMessageForOfflineUse;
   PRBool m_downloadingFolderForOfflineUse;
 

group those PRBools and switch to PRPackedBool?

7)

+#ifdef DEBUG_bienvenu
+    // Make sure this isn't causing us to open the database
+    PR_ASSERT(m_hierarchyDelimiter != kOnlineHierarchySeparatorUnknown);
+#endif

and

+    PR_ASSERT(m_namespace);

on linux, that is an abort().  switch to NS_ASSERTION()

Comment 3

[David :Bienvenu]

Attachment: comments incorporated and more changes

I've addressed Seth's comments (mostly), and added more code that I didn't
realize we hadn't ported from 4.x - with this patch, I can now tell that a
folder is not readable, and we won't try to select it. We also issue myrights
on folders we don't own, instead of general acl.

Comment 4

[(not reading, please use [e-mail] instead)]

Comment on attachment 65144 [details] [diff] [review]
comments incorporated and more changes

r=sspitzer

looks like there is bogus comment in there:

+  // this

Comment 5

[Scott MacGregor]

Comment on attachment 65144 [details] [diff] [review]
comments incorporated and more changes

sr=mscott

Comment 6

[David :Bienvenu]

fixes checked in.

Comment 7

[Karen Huang]

IMAP Shared Folder works for all the platforms, so this bug should be verified
and closed now.
Windows 07-24-08-1.0 verified
Linux 07-24-06-1.0 verified
Mac 9.2 07-18-05-1.0 verified
Mac OSX 07-24-05-1.0 verified