Closed
Bug 1558726
Opened 5 years ago
Closed 5 years ago
Stop using [array] in nsISocketTransportService
Categories
(Core :: Networking, task, P3)
Core
Networking
Tracking
()
RESOLVED
FIXED
mozilla69
Tracking | Status | |
---|---|---|
firefox69 | --- | fixed |
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
(Blocks 1 open bug)
Details
(Whiteboard: [necko-triaged])
Attachments
(2 files, 1 obsolete file)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
patch
|
darktrojan
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•5 years ago
|
||
Assignee | ||
Comment 2•5 years ago
|
||
Jorg, this will need some comm-central changes.
Comment 3•5 years ago
|
||
Thanks. I see looking at bug 1509981 that there is plenty more to come. EDIT: I checked the other ones, looks like the don't affect TB unless I missed something. So please keep the heads-up coming.
This one here is for nsISocketTransportService.create{Routed}Transport.
Comment 4•5 years ago
|
||
Hi Boris, please let me know if you want me to move smaller patches like this one into a different bug and find another reviewer.
EDIT: I haven't compiled this.
Attachment #9071553 -
Flags: feedback?(bzbarsky)
Updated•5 years ago
|
Priority: -- → P3
Whiteboard: [necko-triaged]
Assignee | ||
Comment 5•5 years ago
|
||
Comment on attachment 9071553 [details] [diff] [review]
1558536-c-c-part-nsISocketTransportService.patch
>+++ b/mail/components/accountcreation/content/guessConfig.js
> var socketTypeName = null;
> if (ssl == SSL) {
> socketTypeName = "ssl";
> } else if (ssl == TLS) {
> socketTypeName = "starttls";
> }
I think this would be clearer as:
var socketTypeName = [];
if (ssl == SSL) {
socketTypeName.push("ssl");
} else if (ssl == TLS) {
socketTypeName.push("starttls");
}
and then
var transport = transportService.createTransport(socketTypeName, hostname, port, proxy);
>+++ b/mailnews/base/util/nsMsgProtocol.cpp
>+ if (connectionType)
>+ connectionTypeArray.AppendElement(nsDependentCString(connectionType));
You can just AppendElement(connectionType), fwiw.
>+++ b/mailnews/imap/src/nsImapProtocol.cpp
>+ if (connectionType)
>+ connectionTypeArray.AppendElement(nsDependentCString(connectionType));
Again, no need for nsDependentCString.
Attachment #9071553 -
Flags: feedback?(bzbarsky) → feedback+
Comment 6•5 years ago
|
||
Thanks, Boris, took your suggestions on board. Geoff, I can't r? here, so the f? is an r?
Attachment #9071553 -
Attachment is obsolete: true
Attachment #9071694 -
Flags: feedback?(geoff)
Updated•5 years ago
|
Attachment #9071694 -
Flags: feedback?(geoff) → feedback+
Comment 7•5 years ago
|
||
You need to stick on an r+ :-)
Updated•5 years ago
|
Attachment #9071694 -
Flags: feedback+ → review+
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e3bd88cde793
Stop using [array] in nsISocketTransportService. r=dragana
Comment 9•5 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
You need to log in
before you can comment on or make changes to this bug.
Description
•