Improve some of the string interfaces on nsISmtpServer

RESOLVED FIXED in Thunderbird 3.0b2

Status

RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: standard8, Assigned: standard8)

Tracking

Trunk
Thunderbird 3.0b2

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Created attachment 351428 [details] [diff] [review]
The fix

I'm doing these so we've got a slightly saner implementation for when I alter stuff for the password manager switch.

This changes some of the string interfaces from string to ACString. Not going with AUTF8String at this stage, as that will adjust the current functionality which I don't want to do without specific testing for l18n issues.
Attachment #351428 - Flags: superreview?(neil)
Attachment #351428 - Flags: review?(neil)

Comment 1

10 years ago
Comment on attachment 351428 [details] [diff] [review]
The fix

>-        if (*aUsername && **aUsername)
>-            return rv;
>-
>-        // empty username
>-        NS_Free(*aUsername);
>-        *aUsername = 0;
>+        if (aUsername.IsEmpty())
>+          return rv;
!aUsername.IsEmpty() surely?

>+  return mPrefBranch->SetCharPref("hostname", nsCString(aHostname).get());
Nit: please use PromiseFlatCString (2×).

>+    uri.AssignLiteral("smtp");
>+    uri.AppendLiteral("://");
AssignLiteral("smtp://");

Should this need to support smtps:// in the future, I think we can live with an if choosing between the two literals ;-)
Attachment #351428 - Flags: superreview?(neil)
Attachment #351428 - Flags: superreview+
Attachment #351428 - Flags: review?(neil)
Attachment #351428 - Flags: review+
Checked in with comments addressed. http://hg.mozilla.org/comm-central/rev/1e7d1a981aa2
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
I had to revert some of the ways we return values from the Set* functions that I altered; changeset: http://hg.mozilla.org/comm-central/rev/89b309ce721c

I also added a comment as to why.
You need to log in before you can comment on or make changes to this bug.