Closed Bug 78932 Opened 24 years ago Closed 23 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: 23 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.