Closed
Bug 89019
Opened 23 years ago
Closed 23 years ago
LDAP does not work on Mac OSX
Categories
(MailNews Core :: LDAP Integration, defect, P1)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.9.4
People
(Reporter: svbegg, Assigned: dmosedale)
References
Details
(Whiteboard: OSX+)
Attachments
(4 files, 4 obsolete files)
52.25 KB,
application/octet-stream
|
Details | |
18.47 KB,
patch
|
Details | Diff | Splinter Review | |
9.01 KB,
application/octet-stream
|
Details | |
31.36 KB,
patch
|
Details | Diff | Splinter Review |
Trying to setup an LDAP directory for email address resolution will cause Fizilla to terminate. This occurs with Mozilla 0.9.2 (Mozilla/5.0 (Macintosh; U; PPC; en-US; rv:0.9.2) Gecko/20010702, Build ID: 0000000000), running on Mac OS X 10.0.4, PowerBook G3 (Lombard) 320 MBytes RAM, 12 Gbytes spare disk. The problem is reproducible. Go to the edit menu, select preferences. Go to 'Mail and Newsgroups', then to addressing. Check the box for 'Directory Server', and click the button labled 'Edit Directories...'. Fill in the name of the directory and the host name. Click the 'Advanced' tab, then click the button labeled 'OK'. The window does not go away. Clicking the tabs in the window will swap the views. Clicking the 'OK' button again will crash Mozilla. The same thing happened with Mozilla 0.9.1 on the same platform.
Comment 1•23 years ago
|
||
->ldap. not sure if this is limited to Mac OS X...
Assignee: sgehani → srilatha
Severity: normal → critical
Component: Preferences → LDAP Mail/News Integration
Keywords: crash
Product: Browser → MailNews
QA Contact: sairuh → yulian
Comment 2•23 years ago
|
||
I can't crash the application but OK button in the Directory Server Properties dialod doesn't function. Checked under Mac OS X 10.0.4 and Fizzilla July 3rd build. This problem doesn't occur under Mac OS 9 (2001070503) or Windows ME (2001070503) branch builds.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 3•23 years ago
|
||
yeah, i dont see the crash either. removing keyword crash. Changing summary. over to sdagley since this is specific mac OSX.
Comment 4•23 years ago
|
||
The problem is that the LDAP SDK is not Carbonated and fails to load under Mac OS X. Apple has supposedly done some work on the SDK so we're trying to get info from them before starting our own Carbon version.
I'm running MacosX build 2001070515 on a Powerbook G3 (Pismo) and I can duplicate both the inactive OK button and that hitting it twice will crash Mozilla. I would not take out the crash keyword just yet ;)
sdagley - should you mark this OS X? If LDAP won't be supported, we have to change the UI to remove LDAP.
Assignee | ||
Comment 7•23 years ago
|
||
peterv might have some thoughts on this as well; adding him to the CC list.
Comment 8•23 years ago
|
||
I have found the changes Apple made to the LDAP code but they date back to 1999 so I'm not certain if they're going to help with making it work on OS X (I haven't had a chance to test them yet).
Comment 9•23 years ago
|
||
We cannot remove LDAP UI because it is all in xul and js and we can ifdef the UI for OSX.
Assignee | ||
Comment 10•23 years ago
|
||
Srilatha: isn't there a separate bug to fix exactly this problem.. ie make --disable-ldap builds do the right thing with the various UI stuff?
Comment 11•23 years ago
|
||
Comment 12•23 years ago
|
||
Found (and attached) the Apple changes but they seem to be based on a verison of the LDAK SDK somewhat different than what's in the current Mozilla tree.
Comment 13•23 years ago
|
||
I was unable to determine exactly what date and/or branch the Apple changes were based on, but they are not TOO far off the tarball from ftp://ftp.mozilla.org/pub/directory/c-sdk/ldap/ldapsdk_12311998.tar.gz
Comment 14•23 years ago
|
||
Comment 15•23 years ago
|
||
The attachment I just created may be useful... but I don't see how these changes will fix the MacOS X problem. Does anyone know WHY libldap won't load on MacOS X? Are OpenTransport calls supported in the Carbon environment?
Comment 16•23 years ago
|
||
Thanks for the patch Mark, I'll try it was soon as I get a chance (that'd be Monday 7/30 since I'm going to be travelling for a few days). As for what errors there are with the current libldap build - direct linkage to InterfaceLib and the OT libs rather than InterfaceStubs, use of Toolbox calls obsoleted in Carbon (SystemTask() for one) and possibly some other OT related issues. I'll post more after I take a look Monday.
Comment 17•23 years ago
|
||
And no, the Apple changes won't solve our Carbon problem but their changes to avoid calling SystemTask() should be a good start
Comment 18•23 years ago
|
||
I do not like the idea of introducing a Mac-specific option to the libldap API which is on the standards track within the IETF (see Apple's LDAP_OPT_MAC_YIELDTOTHREAD related changes in ldap.h, getoption.c, setoption.c, ). I am a few years out of touch with the MacOS world, but can we always call YieldToThread()? Or always call it in the Carbonized version? Also, it would be better to leave NET_SSL and LDAP_SSLIO_HOOKS defined because Mozilla will needs those once LDAP over SSL support is added back into the client (see Apple's changes in LDAPClientDebugDefs.h) I am definitely not qualified to review the OT related changes on tcp.c, but using asynch mode seems like a good thing.
Comment 19•23 years ago
|
||
Well I couldn't sleep on the flight so I did some hacking to get the LDAPClient project building under Carbon. This involved excision of the MacTCP support as that doesn't compile under Carbon. Since it's been at least 5 years since Apple shipped MacTCP with Mac OS I didn't think it was any great loss. Note that this doesn't incluse the async mode change from Apple's changes. I'll look at that next. I'm attaching a patch with the current state of the changes and a .sit file with the revised project (linkage changed).
Comment 20•23 years ago
|
||
Comment 21•23 years ago
|
||
Comment 22•23 years ago
|
||
Steve, you have my (module owner) permission to proceed with your changes (very nice clean up). Another Mac expert should review though.
Assignee | ||
Comment 23•23 years ago
|
||
Steve: one thing worth noting is that in the relatively near future (next several months, I think), the client will move to using the 5.0 or later version of the LDAP C SDK, and the 4.0 branch that we're using now will be obsoleted. Where have you been doing your changes?
Comment 24•23 years ago
|
||
My changes are in the Mozilla tree and pretty specific to the Mac networking code. I don't think there'll be any problem moving my changes over to the main LDAP SDK version as I don't think that code will be changing much. I'm sure mcs will let us know if there's any problems there :-)
Comment 25•23 years ago
|
||
I don't think any work has been done recently on the MacOS-specific LDAP C SDK code (except what Steve has done in the context of this bug). Life should get better with the 5.0 or later LDAP C SDK code because when it is integrated into the Mozilla client we will very likely arrange at run time to ALWAYS use the NSPR networking code (which is better than maintaining two sets of code). The 5.0 LDAP C SDK provides a prldap library that makes it easy to tie libldap to NSPR (also used to tie it to NSS for LDAP over SSL support). Note that the MacOS-specific networking code does still exist in the 5.0 SDK in case someone wants to use the SDK without NSPR, so Steve's changes will still be useful.
Updated•23 years ago
|
Whiteboard: OSX+
Comment 26•23 years ago
|
||
Comment 27•23 years ago
|
||
Posted a more up to date patch that yields a version that works under Mac OS 9.1 with the Carbon build. Unfortunately it still doesn't work under OS X. srilatha, could you please try the patch on the non-Carbon builds and verify I haven't broken anything there (all my trees are Carbon)? You don't need to replace the LDAPClient.mcp, just apply the patch I created on 08/03. Thanks.
Status: NEW → ASSIGNED
Comment 28•23 years ago
|
||
Steve, works fine in the non-carbon build with the patch
Comment 29•23 years ago
|
||
I'm using Moz 0.93 and indeed LDAP does not work: as stated, you can't update your servers, OK does not work. Of course, Netscape 6.1 beta for OS X doesn't work either. The other platform I care about is OpenVMS. On the VMS version (build 2001080116, I think) OK does work, but address completion does not work and the addressbook does not allow you to lookup the directory you have defined. It is no help to define a directory server you can't access!! <soapbox> I read a long submission from a person whose company had "invested" heavily in Netscape 4.7.7 as the preferred e-mail client. We have too. Despite its drawbacks, the ability to use an LDAP directory, in an intuitive way, is absolutely key and is the feature that caused us to choose the product. The lack of this feature in Netscape 6.0 caused us to instruct students and faculty here NOT TO USE IT. Please, getting LDAP directories working is make-or-break for us, and others too you'll find. It is a value-differentiator. </soapbox>
Comment 30•23 years ago
|
||
I see I hadn't set the priority or milestone apparently leading some people to belive this wasn't a critical bug so adjusting accordingly. Don't worry, we know LDAP support is critical.
Priority: -- → P1
Target Milestone: --- → mozilla0.9.4
Updated•23 years ago
|
Keywords: nsenterprise+
Comment 31•23 years ago
|
||
Comment 32•23 years ago
|
||
Looks like the 3rd try is the charm - new patch works on 9x and OS X. Now to get it reviewed and checked in :-)
Comment 33•23 years ago
|
||
I reviewed the patch, but my OT skills are rusty and my carbon knowledge is very limited. It looks okay from the libldap point of view. My only complaint is that the contextPtr that gets passed to the EventHandler() callback is sometimes a tcpstream * and sometimes an int * (which is confusing). Maybe there should be a TCPEventHandler() and a DNREventHandler().
Comment 34•23 years ago
|
||
As Mark points out there's some bad assumptions in the EventHandler() completion proc regarding the contextPtr passed in. Let me make another pass at this before going for a r/sr.
Comment 35•23 years ago
|
||
Oops, meant to add sfraser to the cc: list so he'd see my comment that I'll be submitting a different patch for r/sr
Comment 36•23 years ago
|
||
*** Bug 84210 has been marked as a duplicate of this bug. ***
Comment 37•23 years ago
|
||
Comment 38•23 years ago
|
||
Revised patch with different completion procs per mcs and WaitNextEvent() calls rather than YieldToThread() per sfraser
Comment 39•23 years ago
|
||
Comments: #include "IDE_Options.h" // #define NO_USERINTERFACE // #define LDAP_DEBUG + +/* Read generated build options. */ +#include "DefinesOptions.h" // written at build time Why aren't the LDAP projects using the standard config files, MacPrefix.h and MacPrefix_debug.h? RCS file: /cvsroot/mozilla/directory/c-sdk/ldap/libraries/macintosh/macos-ip.c,v +#include <Threads.h> No longer needed. RCS file: /cvsroot/mozilla/directory/c-sdk/ldap/libraries/macintosh/tcp-univhdrs/ tcp.c,v +#include <Threads.h> and again RCS file: /cvsroot/mozilla/directory/c-sdk/ldap/libraries/macintosh/tcp-univhdrs/ tcp.c,v + OTClientContextPtr OTContext; ... +#if TARGET_CARBON + gHaveOT = ( InitOpenTransportInContext(kInitOTForApplicationMask, &OTContext) == noErr ); +#else /* TARGET_CARBON */ ... +#if TARGET_CARBON + tsp->tcps_ep = OTOpenEndpointInContext( OTCreateConfiguration( kTCPName ), 0, &info, &err, NULL ); +#else /* TARGET_CARBON */ Shouldn't you pass the OTContext that you got from InitOpenTransportInContext() to OTOpenEndpointInContext() ? + GetDateTime( &time ); + end_time = time + timeout; + while(( timeout <= 0 || end_time > time ) && !tsp->tcps_connected && + !tsp->tcps_terminated ) { + GetDateTime( &time ); ... Why use GetDateTime() for timeouts? TickCount() is cheaper, and would do just as well. Same goes for the DNS loops. +#if TARGET_CARBON + //YieldToAnyThread(); + (void)WaitNextEvent(nullEvent, &dummyEvent, 1, NULL); +#else /* TARGET_CARBON */ Delay( 2L, &ticks ); SystemTask(); +#endif /* TARGET_CARBON */ } This Delay() is pointless. It's better to just call SystemTask() in the loop, because then at least you're not wasting CPU cycles. If you take out the Delay(), don't forget to update 'ticks' using TickCount(). Why not WaitNextEvent here too? Or would that introduce too much latency? The OT notifier routines probably need to deal with a few more event types, to deal with dropped connections, hosts that are full and disconnect etc. I haven't looked at the rest of the code to determine if we disconnect politely.
Comment 40•23 years ago
|
||
In the carbon case, this loop will never time out: while (( rc = tcpreadready( tsp )) == 0 && ( timeout == NULL || ticks < endticks )) { +#if TARGET_CARBON + //YieldToAnyThread(); + (void)WaitNextEvent(nullEvent, &dummyEvent, 1, NULL); +#else /* TARGET_CARBON */ Delay( 2L, &ticks ); SystemTask(); +#endif /* TARGET_CARBON */ } sr with these changes
a=dbaron (on behalf of drivers), once you incorporate all of Simon's changes
Comment 42•23 years ago
|
||
Comment 43•23 years ago
|
||
Fix checked in
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 45•23 years ago
|
||
Comment on attachment 43664 [details] [diff] [review] First cut at changes to build Carbon compatible LDAPClient marking obsolete patches as such
Attachment #43664 -
Attachment is obsolete: true
Comment 46•23 years ago
|
||
Comment on attachment 44672 [details] [diff] [review] Second pass at LDAP changes for OS X marking obsolete patches as such
Attachment #44672 -
Attachment is obsolete: true
Comment 47•23 years ago
|
||
Comment on attachment 46816 [details] [diff] [review] Patch for LDAP that works on 9.x and X marking obsolete patches as such
Attachment #46816 -
Attachment is obsolete: true
Comment 48•23 years ago
|
||
Comment on attachment 47424 [details] [diff] [review] Cleaned up patch incorporating suggestions from mcs and sfraser marking obsolete patches as such
Attachment #47424 -
Attachment is obsolete: true
Assignee | ||
Comment 49•23 years ago
|
||
Reopening for landing on LDAP C SDK 5.0 branch.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 51•23 years ago
|
||
Checked in to ldapcsdk_branch_50.
Status: NEW → RESOLVED
Closed: 23 years ago → 23 years ago
Resolution: --- → FIXED
Updated•20 years ago
|
Product: MailNews → Core
Updated•16 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•