Comment 26 for bug 85579

Revision history for this message
In , Sspitzer (sspitzer) wrote :

minor comments. over all, it is looking good.

1) cavin and I decided to move the existing bug (the "a/b" problem) out to
another bug. see #89986

so the plan is to remove that part of the fix from this patch.

2)

+ rv = GetDBFolderInfoAndDB(getter_AddRefs(folderInfo), getter_AddRefs(db));
+ // do this after GetDBFolderInfoAndDB, because it crunches
m_displayFolderName (not sure why)

any idea yet why GetDBFolderInfoAndDB() stomps on m_displayFolderName()?

3)

+ folderInfo = null_nsCOMPtr();

folderInfo is a COMPtr, so once the method returns and folderInfo goes out of
scope, it will release its ref. I don't think you have to do it manually.

4)

+NS_IMETHODIMP nsMsgLocalMailFolder::GetDisplayName(PRUnichar ** aDisplayName)
+{
+ if (!aDisplayName)
+ return NS_ERROR_NULL_POINTER;

just do NS_ENSURE_ARG_POINTER(aDisplayName);

5)

+ ReadDBFolderInfo(PR_FALSE); // update cache first.

ReadDBFolderInfo() returns a nsresult. we don't care about the return result?

if we really don't care what the return value is, we should do this:

(void)ReadDBFolderInfo(PR_FALSE);

6)

before landing, we should double check that we haven't broken any existing
functionality for users on with non latin-1 or non ASCII system charsets.

it looks like we should be ok, but it would be bad if this fix makes it so
those users who could create non-ASCII folders are now unable too.

nhotta, can you help test or help review when we've got a final patch?