Closed Bug 78932 Opened 24 years ago Closed 24 years ago

Additional code changes relating to multiple datasources for the addressbook

Categories

(MailNews Core :: LDAP Integration, defect, P1)

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.4

People

(Reporter: martin.maher, Assigned: john.marmion)

References

Details

There are further code changes necessary to include multiple datasources in the addressbook in including * Factory instantiation of address books * Query interfaces * Linear Query implementation
Blocks: 78933
Marking NEW.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Status: NEW → ASSIGNED
Also to make the query consistent need to make personal addressbooks searchable
Severity: normal → major
Priority: -- → P1
Target Milestone: --- → mozilla0.9.1
Target Milestone: mozilla0.9.1 → mozilla0.9.2
The patch for this is in the parent bug. However, according some mail from Martin, the only stuff that needs to be reviewed for this patch to be satisfied are the diffs to these files: addrbook/src/Makefile.in addrbook/src/makefile.win addrbook/src/nsDirPrefs.cpp addrbook/src/nsDirPrefs.h addrbook/src/nsAddressBook.cpp addrbook/src/nsAddressBook.h addrbook/src/nsAbBSDirectory.cpp addrbook/src/nsAbBSDirectory.h addrbook/src/nsAbCardProperty.cpp addrbook/src/nsAbCardProperty.h addrbook/src/nsAbDirProperty.cpp addrbook/src/nsAbMDBCardProperty.cpp addrbook/src/nsAbMDBCardProperty.h addrbook/src/nsAbRDFDataSource.cpp addrbook/src/nsAbRDFDataSource.h addrbook/src/nsAbMDBDirectory.cpp addrbook/src/nsAbMDBDirectory.h addrbook/src/nsAbMDBDirProperty.cpp addrbook/src/nsAbMDBDirProperty.h addrbook/src/nsAbMDBCard.h addrbook/build/nsAbBaseCID.h addrbook/build/nsAbFactory.cpp addrbook/public/Makefile.in addrbook/public/makefile.win addrbook/public/nsIAddressBook.idl addrbook/public/nsIAbDirectory.idl addrbook/public/nsIAbMDBCard.idl addrbook/public/nsIAbMDBDirectory.idl addrbook/resources/content/addressbook.js import/src/nsImportAddressBooks.cpp and the following new files: addrbook/src/nsAbDirFactoryService.cpp addrbook/src/nsAbDirFactoryService.h addrbook/src/nsAbMDBDirFactory.cpp addrbook/src/nsAbMDBDirFactory.h addrbook/src/nsAbDirectoryQuery.cpp addrbook/src/nsAbDirectoryQuery.h addrbook/src/nsAbDirectoryQueryProxy.cpp addrbook/src/nsAbDirectoryQueryProxy.h addrbook/src/nsBooleanExpression.cpp addrbook/src/nsBooleanExpression.h addrbook/src/nsAbUtils.cpp addrbook/src/nsAbUtils.h addrbook/src/nsAbQueryStringToExpression.cpp addrbook/src/nsAbQueryStringToExpression.h addrbook/src/nsAbDirectoryRDFResource.cpp addrbook/src/nsAbDirectoryRDFResource.h addrbook/src/nsAbDirSearchListener.cpp addrbook/src/nsAbDirSearchListener.h addrbook/public/nsIAbDirectoryQuery.idl addrbook/public/nsIAbDirectoryQueryProxy.idl addrbook/public/nsIAbDirFactory.idl addrbook/public/nsIAbDirFactoryService.idl addrbook/public/nsIBooleanExpression.idl addrbook/public/nsIAbDirectorySearch.idl chuang, can you start by reviewing these?
Reassigning bugs related to LDAP/MAPI addressbook change landing to John Marmion, as he'll be driving this landing going forward.
Assignee: dmose → srilatha
Status: ASSIGNED → NEW
Component: Address Book → LDAP Mail/News Integration
QA Contact: esther → yulian
2nd try at reassignment
Assignee: srilatha → john.marmion
updated the patch file at #78933: Updated the license boilerplate for all new source files submitted with this patch. I have also updated the experimental builds at <http://abzilla.mozdev.org> with this patch against today's head for Linux, Windows and the Mac. Note to enable the LDAP Address Book the uri entry in the preferences file must be set to "moz-abldapdirectory://...." as requested.
I needed to take a dip into some of this code in order to understand the LDAP stuff, so here's review comments on several of the new files: nsAbUtils.h ----------- * Comments about the purposes of the various classes, please! * Do are the *PtrArrayGuard classes only exist to ensure that arrays being built are freed correctly in all conditions? If so, you can just use NS_FREE_XPCOM_ALLOCATED_POINTER_ARRAY or one of its friends from xpcom/base/nsISupportsUtils.h. If you really want to keep this class around for its automatic property, at least use one of those macros inside the Free function. nsAbUtils.cpp ------------- * Comments about the various classes, please. Especially in need is the closure/enumeration stuff. * Still a few C-style casts left here. * CStringArrayToCharPtrArray::Convert(): Why is the "element" temporary necessary? Instead of |nsCRT::strdup(element->get())|, use |element->ToNewCString()|. Similar comments apply to StringArrayToCharPtrArray::Convert(). nsIAbDirectoryQuery.idl ----------------------- * Why is expression an nsISupports rather than an nsIBooleanExpression? * Why the "a" on the front of "agetResult"? As far as I know, in Mozilla parlance, a leading "a" is generally just used to indicate an argument. * In the comments for stopQuery, can you state whether the abort is guaranteed to be completed by the time stopQuery() returns, or if it may have only been initiated (ie callbacks may continue for a short period)?
I've looked on the diffs, not much on the new files. r=chuang provided no regression found on QA's end for existing features.
Target Milestone: mozilla0.9.2 → mozilla0.9.3
Missed 0.9.3.
Target Milestone: mozilla0.9.3 → mozilla0.9.4
Code checkedin.
Status: NEW → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
reassign Olga as QA Contact
QA Contact: yulian → olgac
Verified with 20010815 trunk build
Status: RESOLVED → VERIFIED
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.