121332 - AutoConfig: ProcessLDAPvalues doesn't seem to be implemented

Status: VERIFIED FIXED

(Core :: Preferences: Backend, defect)

Description

[lrg]

At this point i am not sure if this is a simple documentation error, or if the
code wasn't put in.  I believe that we are able to issue a call from inside of
autoconfig to the LDAP servers, however it seems as though the data that is
being returned isn't being used.  Previously this was handled with the
processLDAPvalues function, however, nothing seems to work now.

Comment 1

[lrg]

Reassigning to myself as QA

Comment 2

[Brian Nesse (gone)]

The documentation (refering to the draft of the Factory documentation pg. 9-10)
indicates that the following functions as defined (in prefcalls.js):
 - getLDAPAttributes(host, base, filter, attribs)
 - getLDAPValue(str, key)

This documentation offers up some sample code, including a function named
processLDAPValues(), which the user is expected to supply as a callback function.

Looking at the code, it appears that the actual function being called by
getLDAPAttributes() may be getQueryResults(url), not processLDAPValues(url).
Please try supplying a callback with that name instead.

Comment 3

[lrg]

I have tried testing it using this code:

function getQueryResults(aresult) {
  var homepage = getLDAPValue(aresult, "webpage");
  lockPref("browser.startup.homepage",homepage);
  var uid = getLDAPValue(aresult,"uid");
  lockPref("aim.session.screenname", uid);
}

But it isn't working.  Can you see anything wrong with this code?  If not then i
don't think we have found the solution we need.

Comment 4

[Brian Nesse (gone)]

Then something is probably missing. It would be helpful if you could attach a
testcase that worked before the AutoConfig module breakout to the bug.

Comment 5

[lrg]

Attachment: A jsc file with an LDAP call that worked in old version

This attachment is a .jsc file which worked in the older implementation of
autoconfig.

Comment 6

[lrg]

The old implementation can be seen in the 20011019 branch build (among others.)

Comment 7

[lrg]

This feature was functional in the 6.2 release, but doesn't seem to be working now.

Comment 8

[lrg]

Has there been any change here?

Comment 9

[Jerry]

Stefanie meet the same question with netscape 622 solaris version when she want
to deploy netscape6 internal.

so my question is
does netscape 6 has passed enterprise deploy test?

Comment 10

[Margaret Chan]

Is this feature working on the existing 6.2.2 release on the Window platform?  
If so, is the .jsc file in attachment 66837 [details] a working version for the 6.2.2 
release?  Thanks.

Comment 11

[lrg]

I have just re-verified that the 6.2.2RTM build has the LDAP functionality
implemented.  (The jsc file that is attached to this bug functions as we expect
it to.)

I hope that helps

Comment 12

[Margaret Chan]

Hi Jerry:

Can you do a test with the exact same jsc file (with needed modification) using
the Solaris 6.2.2 candidate?  You may have done that before, but if it works for
the window platform according to [e-mail], then I would expect it to
work on ours.  Thanks.

Comment 13

[Jerry]

It works for me with netscape 6.2.2 for solaris and for windows.

Comment 14

[Niranjani Raya]

http://bugzilla.mozilla.org/show_bug.cgi?id=75955 Another bug which matches the 
description.

Comment 15

[Dan Mosedale (:dmosedale, :dmose)]

nraya: 75955 was the original implementation bug that tracked the implementation
of this feature.  This appears to be a regression of that bug.

Comment 16

[Rob Jaworski]

keyword back to nsbeta1 for nomination for rtm.

Comment 17

[tao]

Put this bug on the rtm watch radar.

Comment 18

[tao]

Hi, Brian: do we have a clue of what regressed this and how hard it is to fix
this bug? This feature used to work and now it doesn't :-\

Comment 19

[Margaret Chan]

This call is important for enterprise deployment.  We do need this regression to
be fixed by rtm.   Thanks.

Comment 20

[Brian Nesse (gone)]

Tao, in short, no. The basic code to support it seems to be there. Why it
doesn't work is anybody's guess.

Comment 21

[Mitch Green]

This is a showstopper for SBS.

Comment 22

[Margaret Chan]

Changing the target milestone to 1.0.1.  This is a regression which we will need
for enterprise deployment.

Comment 23

[Brian Nesse (gone)]

Attachment: Patch

Patch to re-enable LDAPQuery and make AutoConfig errors localizable.

Comment 24

[Brian Nesse (gone)]

Dan, can you r= this? Alec, can you sr?

This is a RTM Blocker. Thanks.

Comment 25

[Alec Flett]

Comment on attachment 86835 [details] [diff] [review]
Patch

you should be using bundle.formatStringFromName()  rather than msg.replace and
so forth.

also, what happens if the user hasn't defined processLDAPValues()?

Comment 26

[Brian Nesse (gone)]

Attachment: Better patch.

Patch which appropriately uses formatStringFromName (sorry...my bad). As
discussed with Alec, if the user doesn't supply the callback, it will throw a
JavaScript exception which will cause the error to be reported to the user...
which is what we would want to happen.

To summarize long discussion with Alec. This bug is a regression of bug 75955
(the original implementation), caused by the checkin for bug 89137 (which
seperated AutoConfig out into it's own module). The regression occurred during
the translation of the LDAP code from C++, in the original implementation, to
the current JavaScript implementation.

This checkin will in no way negatively affect users of 6.2. It works in 6.2 it
will work in 7 (with this patch). It just didn't work in between...

Comment 27

[Alec Flett]

Comment on attachment 86856 [details] [diff] [review]
Better patch.

sr=alecf

Comment 28

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 86856 [details] [diff] [review]
Better patch.

r=[e-mail]

Comment 29

[scottputterman]

adding various keywords to get it on the rtm radar.  Nominating adt1.0.1 because
it looks like it has a r=/sr= patch.  If it still needs work then please remove
the adt1.0.1.  Thanks for fixing this so quickly

Comment 30

[scottputterman]

adding adt1.0.1+.  Please get drivers approval before checking into the branch.

Comment 31

[Brian Nesse (gone)]

Checked in on the trunk.

Comment 32

[Brian Nesse (gone)]

Closing as fixed on trunk.

Comment 33

[Margaret Chan]

Thank you for the fix, Brian.  Would this fix go in to the 1.0.1 branch? 

Comment 34

[Brian Nesse (gone)]

Margaret, I am currently waiting for drivers approval to land on the branch.

Comment 35

[Margaret Chan]

Thanks for the update, Brian.

Comment 36

[Judson Valeski]

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.

Comment 37

[lrg]

I have been able to verify this fix in the trunk using the 06/17 build

Comment 38

[Brian Nesse (gone)]

Patch checked in to the 1.0 branch.

Comment 39

[lrg]

Verified on WindowsXP, MacOSX, and Red Hat Linux 7