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

RESOLVED FIXED in Thunderbird 3.0b1

Status

defect
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: rain1, Assigned: rain1)

Tracking

({regression})

Trunk
Thunderbird 3.0b1
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Assignee

Description

11 years ago
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.)
Attachment #329869 - Flags: superreview?(bienvenu)
Attachment #329869 - Flags: review?(bienvenu)
Assignee

Updated

11 years ago
Summary: MsgMailboxGetURI doesn't escape the relative path as should be done for a → MsgMailboxGetURI doesn't escape the relative path as should be done for a URL

Updated

11 years ago
Attachment #329869 - Flags: superreview?(bienvenu)
Attachment #329869 - Flags: superreview+
Attachment #329869 - Flags: review?(bienvenu)
Attachment #329869 - Flags: review+
Assignee

Updated

11 years ago
Keywords: checkin-needed
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
Keywords: checkin-needed
Target Milestone: --- → mozilla1.9.1a1
Assignee

Updated

11 years ago
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Product: Core → MailNews Core
Assignee

Comment 2

11 years ago
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee

Comment 3

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

Updated

11 years ago
Attachment #343625 - Flags: superreview?(bienvenu)
Attachment #343625 - Flags: review?(bienvenu)

Comment 4

11 years ago
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?

Updated

11 years ago
Attachment #343625 - Flags: superreview?(bienvenu)
Attachment #343625 - Flags: superreview+
Attachment #343625 - Flags: review?(bienvenu)
Attachment #343625 - Flags: review+
Assignee

Comment 5

11 years ago
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.
Attachment #343625 - Attachment is obsolete: true
Attachment #344108 - Flags: superreview+
Attachment #344108 - Flags: review+
Assignee

Updated

11 years ago
Keywords: checkin-needed
Comment on attachment 344108 [details] [diff] [review]
[checked in] update addressing review comments

Checked in:
http://hg.mozilla.org/comm-central/rev/f63c0bb551a6
Attachment #344108 - Attachment description: [to checkin] update addressing review comments → [checked in] update addressing review comments
Keywords: checkin-needed
Target Milestone: mozilla1.9.1a1 → Thunderbird 3.0b1
Assignee

Updated

11 years ago
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.