445554 - MsgMailboxGetURI doesn't escape the relative path as should be done for a URL

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Rain [:rain1] (inactive)]

Attachment: patch to escape the string

Because of changes in bug 433697, the relative path of the folder inside the root folder isn't escaped. (getRelativeDescriptor doesn't escape the path.)

Comment 1

[Mark Banner (:standard8)]

Checking in mailnews/base/util/nsMsgUtils.cpp;
/cvsroot/mozilla/mailnews/base/util/nsMsgUtils.cpp,v  <--  nsMsgUtils.cpp
new revision: 1.151; previous revision: 1.150
done

Comment 2

[Rain [:rain1] (inactive)]

Reopened because the fix is incorrect -- it only works for local folders, not for IMAP. In fact, IMAP folders don't escape spaces, so the patch fixes the local folder case but breaks IMAP folders with spaces.

Comment 3

[Rain [:rain1] (inactive)]

Attachment: Now escapes only for a local folder

Ugly hack, but discussion over IRC led me to believe that this way will work best.

Comment 4

[David :Bienvenu]

Comment on attachment 343625 [details] [diff] [review]
Now escapes only for a local folder

I think the patch is OK, but I think this would all be a bit easier to understand if we fixed some function and var names.

MsgMailboxGetURI is a misleading name (and I bet I either wrote this method or reviewed it, so my bad)

and in nsMessengerBootstrap, the code that calls MsgMailboxGetURI uses the name msgFolder for a local file object, not an nsIMsgFolder. So I think we should change some names here, while there's only one caller.

Maybe something like MsgGetFolderUriFromProfilePath?  and maybe msgFolder should be folderProfilePath?

Comment 5

[Rain [:rain1] (inactive)]

Attachment: [checked in] update addressing review comments

After discussion on IRC, MsgMailboxGetURI was changed to FolderUriFromDirInProfile, which is unambiguous as to the purpose of the function.

In HandleIndexerResult, folderPath has been changed to folderPathStr, and msgFolder to folderPath.

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 344108 [details] [diff] [review]
[checked in] update addressing review comments

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