If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Implement nsILDAPURL interface

VERIFIED FIXED

Status

Directory
LDAP XPCOM SDK
P1
enhancement
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: Leif Hedstrom, Assigned: Leif Hedstrom)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

(Assignee)

Description

17 years ago
Create a new object for handling LDAP URIs. This is part of the LDAP xpcom
wrappers package.
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

17 years ago
Priority: -- → P1
(Assignee)

Comment 1

17 years ago
Created attachment 26648 [details] [diff] [review]
This is a first cut on some changes to the interface

Comment 2

17 years ago
Created attachment 26773 [details]
mozilla/directory/xpcom/base/public/nsILDAPErrors.idl

Comment 3

17 years ago
Urf; attached this to the wrong bug.  Please disregard.

Updated

17 years ago
QA Contact: esther → yulian
(Assignee)

Updated

17 years ago
Component: Address Book → LDAP XPCOM SDK
Product: MailNews → Directory
(Assignee)

Comment 4

17 years ago
Created attachment 29080 [details] [diff] [review]
Rewrote code to support set/get properly
(Assignee)

Comment 5

17 years ago
Dmose, can you please review this? I've tested it on Unix and Win32 only, still
waiting for a Mac machine. :(

Thanks.

Comment 6

17 years ago
Nitpicky comments:

* Why use an nsCString _and_ the mOptions to represent secure bit? 

nsLDAPURL::nsLDAPURL()

	* use C++-style initializers here?

nsLDAPURL::SetScheme()

	* do you need to set the option string here as well (but see above)?
	
	* is it possible to do error checking on mScheme.ToLowerCase?

nsLDAPURL::Get/SetPort()

	* according to nsIURI.idl, the "default setting" for port is
	  supposed to be represented by -1.  You'll probably need to
	  translate between the nsIURI default (-1) and the LDAP C SDK
	  default (0, I think) in at least one of these functions, 
	  perhaps in the constructor as well.

Other than those minor things, it looks good.


(Assignee)

Comment 7

17 years ago
Created attachment 29490 [details] [diff] [review]
2nd patch, changes are per review

Comment 8

17 years ago
r=dmose@netscape.com, pending working Mac and Win32 builds
+    if (!mPort) {
+        *_retval = (-1);
     } else {
-        *aPort = -1;
+        *_retval = mPort;
     }

Why the parens in |(-1)|, and below in:

+    if (aPort == (-1)) {
+        mPort = 0;

but not around the |0| in:

+    } else if (aPort >= 0) {
+        mPort = aPort;

?  I think the parens are just noise, but I really just care about consistency.

+        (aScope != SCOPE_ONELEVEL) &&
+        (aScope != SCOPE_SUBTREE)) {
+
+        return NS_ERROR_MALFORMED_URI;
+    }

Bo[gn]us whitespace!

Tidy those up, and sr=shaver (I don't need to see another patch if that's all
you change.)
Oh, and sorry for the wait.  I'm a loser.
(Assignee)

Comment 11

17 years ago
Created attachment 30508 [details] [diff] [review]
Changes as per shavers sr=
Patch checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 13

17 years ago
reassign to Olga as QA contact
QA Contact: yulian → olgac

Comment 14

17 years ago
verified:  nsILDAPURL interface is implemented
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.