104927 - Implement a mechanism for getLDAPAttributes() functionality

Status: VERIFIED DUPLICATE of bug 89137

(Core :: Preferences: Backend, defect)

Description

[Mitesh Shah]

Craeted a new bug to track the progress of getLDAPAttributes() feature on the 
trunk (original bug 75955)

Comment 1

[Mitesh Shah]

Attachment: JS version of getLDAPAttributes

Comment 2

[Mitesh Shah]

The attached JS file will be evaluated as a part of the fix for bug 89137 in the 
new AutoConfig JS context

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Should this bug now be resolved as a dup of 89137, since there appears to be a
newer version of the patch there?

Comment 4

[Mitesh Shah]

let's keep this open since bug 89137 is more of a new JS context for autoconfig 
while this bug is a JS version of getLDAPAttributes() which will be evaluated in 
that context. I will post the newer version of getldap.js here.

Comment 5

[Mitesh Shah]

Attachment: latest version of getldap.js

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Review comments:

a) LDAPClass.prototype.QueryInterface: the comment is not correct,
only a single interface is being implemented here.  Because of this,
XPConnect can take care of the QI for you; you should be able to
throw out the entire QueryInterface method.

b) .onLDAPMessage: the indenting and spacing in the switch clause make
it hard to read; a more standard indentation strategy of 

    switch (aMessage.type) {

    case aMessage.RES_BIND:
        this.onLDAPBind(aMessage);
        break;

    case aMessage.RES_SEARCH_ENTRY:
        ...

would be easier, I think.

c) the event queue service and the current event queue appear to be
gotten in a couple of different places.  Can you declare them in the
global scope so that they only need to be gotten once?  This should
even work through callbacks, since JS functions are lexically closed.

d) in all the error-handling, the code quietly returns nothing if
there's a problem, which will make it difficult for admins to debug
the scripts they are writing.  Can we come up with a better strategy?  Maybe
throw up an nsIPrompt window with error info?

Comment 7

[Mitesh Shah]

New patch with review comments addressed

Comment 8

[Mitesh Shah]

Attachment: updated version

Comment 9

[Mitesh Shah]

The new patch on bug 89137 will address this together with the other issues.

*** This bug has been marked as a duplicate of 89137 ***

Comment 10

[Alec Flett]

yes, I agree :)