Bug 749097 - Allow to search for ( and ) characters in addressbook cards by urlencoding them in the search query. r=Neil, r=mconley
authoraceman <acelists@atlas.sk>
Sun, 27 Apr 2014 07:27:00 -0400
changeset 19954 6991232407063296e14bd1bc26941b5330cc36c9
parent 19953 af30e12503ab60017b490d09d4108696e77cb2fd
child 19955 35badd55a546b137bea97066c416acb0a95bdc0a
push id1151
push usermbanner@mozilla.com
push dateMon, 09 Jun 2014 22:14:36 +0000
treeherdercomm-beta@ce127428ad7d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersNeil, mconley
bugs749097
Bug 749097 - Allow to search for ( and ) characters in addressbook cards by urlencoding them in the search query. r=Neil, r=mconley
mail/base/content/ABSearchDialog.js
mail/components/addrbook/content/abCommon.js
mail/components/addrbook/content/abContactsPanel.js
mail/components/addrbook/content/addressbook.js
mailnews/addrbook/src/nsAbAutoCompleteSearch.js
mailnews/addrbook/src/nsAbQueryStringToExpression.cpp
mailnews/addrbook/test/unit/test_nsAbAutoCompleteSearch3.js
suite/mailnews/addrbook/abCommon.js
suite/mailnews/addrbook/abSelectAddressesDialog.js
suite/mailnews/addrbook/addressbook.js
suite/mailnews/search/ABSearchDialog.js
--- a/mail/base/content/ABSearchDialog.js
+++ b/mail/base/content/ABSearchDialog.js
@@ -278,17 +278,17 @@ function onSearch()
       }
 
       // currently, we can't do "and" and "or" searches at the same time
       // (it's either all "and"s or all "or"s)
       var max_attrs = attrs.length;
 
       for (var j=0;j<max_attrs;j++) {
        // append the term(s) to the searchUri
-       searchUri += "(" + attrs[j] + "," + opStr + "," + encodeURIComponent(searchTerm.value.str) + ")";
+       searchUri += "(" + attrs[j] + "," + opStr + "," + encodeABTermValue(searchTerm.value.str) + ")";
       }
     }
 
     searchUri += ")";
     SetAbView(searchUri);
 }
 
 // used to toggle functionality for Search/Stop button.
--- a/mail/components/addrbook/content/abCommon.js
+++ b/mail/components/addrbook/content/abCommon.js
@@ -749,8 +749,18 @@ function makePhotoFile(aDir, aExtension)
   // Find a random filename for the photo that doesn't exist yet
   do {
     filename = new String(Math.random()).replace("0.", "") + "." + aExtension;
     newFile = aDir.clone();
     newFile.append(filename);
   } while (newFile.exists());
   return newFile;
 }
+
+/**
+ * Encode the string passed as value into an addressbook search term.
+ * The '(' and ')' characters are special for the addressbook
+ * search query language, but are not escaped in encodeURIComponent()
+ * so must be done manually on top of it.
+ */
+function encodeABTermValue(aString) {
+  return encodeURIComponent(aString).replace(/\(/g, "%28").replace(/\)/g, "%29");
+}
--- a/mail/components/addrbook/content/abContactsPanel.js
+++ b/mail/components/addrbook/content/abContactsPanel.js
@@ -134,12 +134,12 @@ function onEnterInSearchBar()
   if (!gQueryURIFormat)
     gQueryURIFormat = Services.prefs.getComplexValue("mail.addr_book.quicksearchquery.format",
       Components.interfaces.nsIPrefLocalizedString).data;
 
   var searchURI = GetSelectedDirectory();
   var searchInput = document.getElementById("peopleSearchInput");
 
   if (searchInput.value != "")
-    searchURI += gQueryURIFormat.replace(/@V/g, encodeURIComponent(searchInput.value));
+    searchURI += gQueryURIFormat.replace(/@V/g, encodeABTermValue(searchInput.value));
 
   SetAbView(searchURI);
 }
--- a/mail/components/addrbook/content/addressbook.js
+++ b/mail/components/addrbook/content/addressbook.js
@@ -495,17 +495,17 @@ function onEnterInSearchBar()
    XXX todo, handle the case where the LDAP url
    already has a query, like
    moz-abldapdirectory://nsdirectory.netscape.com:389/ou=People,dc=netscape,dc=com?(or(Department,=,Applications))
   */
   var searchInput = document.getElementById("peopleSearchInput");
   if (searchInput && searchInput.value != "") {
     // replace all instances of @V with the escaped version
     // of what the user typed in the quick search text input
-    searchURI += gQueryURIFormat.replace(/@V/g, encodeURIComponent(searchInput.value));
+    searchURI += gQueryURIFormat.replace(/@V/g, encodeABTermValue(searchInput.value));
   }
 
   SetAbView(searchURI);
 
   // XXX todo
   // this works for synchronous searches of local addressbooks,
   // but not for LDAP searches
   SelectFirstCard();
--- a/mailnews/addrbook/src/nsAbAutoCompleteSearch.js
+++ b/mailnews/addrbook/src/nsAbAutoCompleteSearch.js
@@ -124,18 +124,23 @@ nsAbAutoCompleteSearch.prototype = {
    * a mailing list) then the function will add a result for each email address
    * that exists.
    *
    * @param searchQuery  The boolean search query to use.
    * @param directory    An nsIAbDirectory to search.
    * @param result       The result element to append results to.
    */
   _searchCards: function _searchCards(searchQuery, directory, result) {
-    var childCards =
-      this._abManager.getDirectory(directory.URI + searchQuery).childCards;
+    let childCards;
+    try {
+      childCards = this._abManager.getDirectory(directory.URI + searchQuery).childCards;
+    } catch (e) {
+      Components.utils.reportError("Error running addressbook query '" + searchQuery + "': " + e);
+      return;
+    }
 
     // Cache this values to save going through xpconnect each time
     var commentColumn = this._commentColumn == 1 ? directory.dirName : "";
 
     // Now iterate through all the cards.
     while (childCards.hasMoreElements()) {
       var card = childCards.getNext();
 
@@ -345,17 +350,17 @@ nsAbAutoCompleteSearch.prototype = {
       // for each word separately so that each result contains all the words
       // from the fullstring in the fields of the addressbook card
       // (see bug 558931 for explanations).
       let searchQuery = "";
       let modelQuery = "(or(DisplayName,c,@V)(FirstName,c,@V)(LastName,c,@V)" +
                        "(NickName,c,@V)(PrimaryEmail,c,@V)(SecondEmail,c,@V)" +
                        "(and(IsMailList,=,TRUE)(Notes,c,@V)))";
       for (let searchWord of searchWords) {
-        searchQuery += modelQuery.replace(/@V/g, encodeURIComponent(searchWord));
+        searchQuery += modelQuery.replace(/@V/g, encodeABTermValue(searchWord));
       }
       searchQuery = "?(and" + searchQuery + ")";
       // Now do the searching
       var allABs = this._abManager.directories;
 
       // We're not going to bother searching sub-directories, currently the
       // architecture forces all cards that are in mailing lists to be in ABs as
       // well, therefore by searching sub-directories (aka mailing lists) we're
@@ -381,12 +386,22 @@ nsAbAutoCompleteSearch.prototype = {
   },
 
   // nsISupports
 
   QueryInterface: XPCOMUtils.generateQI([Components.interfaces
                                                    .nsIAutoCompleteSearch])
 };
 
+/**
+ * Encode the string passed as value into an addressbook search term.
+ * The '(' and ')' characters are special for the addressbook
+ * search query language, but are not escaped in encodeURIComponent()
+ * so it must be done manually on top of it.
+ */
+function encodeABTermValue(aString) {
+  return encodeURIComponent(aString).replace(/\(/g, "%28").replace(/\)/g, "%29");
+}
+
 // Module
 
 var components = [nsAbAutoCompleteSearch];
 const NSGetFactory = XPCOMUtils.generateNSGetFactory(components);
--- a/mailnews/addrbook/src/nsAbQueryStringToExpression.cpp
+++ b/mailnews/addrbook/src/nsAbQueryStringToExpression.cpp
@@ -10,16 +10,30 @@
 #include "nsCOMPtr.h"
 #include "nsStringGlue.h"
 #include "nsITextToSubURI.h"
 #include "nsAbBooleanExpression.h"
 #include "nsAbBaseCID.h"
 #include "plstr.h"
 #include "nsIMutableArray.h"
 
+/**
+ * This code parses the query expression passed in as an addressbook URI.
+ * The expression takes the form:
+ * (BOOL1(FIELD1,OP1,VALUE1)..(FIELDn,OPn,VALUEn)(BOOL2(FIELD1,OP1,VALUE1)...)...)
+ *
+ * BOOLn   A boolean operator joining subsequent terms delimited by ().
+ *         For possible values see CreateBooleanExpression().
+ * FIELDn  An addressbook card data field.
+ * OPn     An operator for the search term.
+ *         For possible values see CreateBooleanConditionString().
+ * VALUEn  The value to be matched in the FIELDn via the OPn operator.
+ *         The value must be URL encoded by the caller, if it contains any special
+ *         characters including '(' and ')'.
+ */
 nsresult nsAbQueryStringToExpression::Convert (
     const nsACString &aQueryString,
     nsIAbBooleanExpression** expression)
 {
     nsresult rv;
 
     nsAutoCString q(aQueryString);
     q.StripWhitespace();
--- a/mailnews/addrbook/test/unit/test_nsAbAutoCompleteSearch3.js
+++ b/mailnews/addrbook/test/unit/test_nsAbAutoCompleteSearch3.js
@@ -14,22 +14,30 @@ const cards = [
     popularityIndex: 1, firstName: "test2", value: "abc@foo.invalid" },
   { email: "foo1@foo.invalid", displayName: "d",
     popularityIndex: 0, firstName: "first1", value: "d <foo1@foo.invalid>" },
   { email: "foo2@foo.invalid", displayName: "di",
     popularityIndex: 1, firstName: "first1", value: "di <foo2@foo.invalid>" },
   { email: "foo3@foo.invalid", displayName: "dis",
     popularityIndex: 2, firstName: "first2", value: "dis <foo3@foo.invalid>" },
   { email: "foo2@foo.invalid", displayName: "di",
-    popularityIndex: 3, firstName: "first2", value: "di <foo2@foo.invalid>" }
+    popularityIndex: 3, firstName: "first2", value: "di <foo2@foo.invalid>" },
+  // this just tests we can search for the special chars '(' and ')', bug 749097
+  { email: "bracket@not.invalid", secondEmail: "h@not.invalid", firstName: "Mr.",
+    displayName: "Mr. (Bracket)", value: "Mr. (Bracket) <bracket@not.invalid>",
+    popularityIndex: 2 },
+  { email: "mr@(bracket).not.invalid", secondEmail: "bracket@not.invalid",  firstName: "Mr.",
+    displayName: "Mr. Bracket", value: "Mr. Bracket <mr@(bracket).not.invalid>",
+    popularityIndex: 1 }
 ];
 
 const duplicates = [
   { search: "test", expected: [2, 1] },
-  { search: "first", expected: [6, 5, 3] }
+  { search: "first", expected: [6, 5, 3] },
+  { search: "(bracket)", expected: [7, 8] }
 ];
 
 
 function acObserver() {}
 
 acObserver.prototype = {
   _search: null,
   _result: null,
--- a/suite/mailnews/addrbook/abCommon.js
+++ b/suite/mailnews/addrbook/abCommon.js
@@ -751,8 +751,18 @@ function makePhotoFile(aDir, aExtension)
   // Find a random filename for the photo that doesn't exist yet
   do {
     filename = new String(Math.random()).replace("0.", "") + "." + aExtension;
     newFile = aDir.clone();
     newFile.append(filename);
   } while (newFile.exists());
   return newFile;
 }
+
+/**
+ * Encode the string passed as value into an addressbook search term.
+ * The '(' and ')' characters are special for the addressbook
+ * search query language, but are not escaped in encodeURIComponent()
+ * so must be done manually on top of it.
+ */
+function encodeABTermValue(aString) {
+  return encodeURIComponent(aString).replace(/\(/g, "%28").replace(/\)/g, "%29");
+}
--- a/suite/mailnews/addrbook/abSelectAddressesDialog.js
+++ b/suite/mailnews/addrbook/abSelectAddressesDialog.js
@@ -361,17 +361,17 @@ function onEnterInSearchBar()
 
   if (!gQueryURIFormat) {
     gQueryURIFormat = GetLocalizedStringPref("mail.addr_book.quicksearchquery.format");
   }
   
   var searchURI = selectedNode.value;
 
   if (gSearchInput.value != "") {
-    searchURI += gQueryURIFormat.replace(/@V/g, encodeURIComponent(gSearchInput.value));
+    searchURI += gQueryURIFormat.replace(/@V/g, encodeABTermValue(gSearchInput.value));
   }
 
   SetAbView(searchURI);
   
   SelectFirstCard();
 }
 
 function DirPaneSelectionChangeMenulist()
--- a/suite/mailnews/addrbook/addressbook.js
+++ b/suite/mailnews/addrbook/addressbook.js
@@ -359,17 +359,17 @@ function onEnterInSearchBar()
   /*
    XXX todo, handle the case where the LDAP url
    already has a query, like 
    moz-abldapdirectory://nsdirectory.netscape.com:389/ou=People,dc=netscape,dc=com?(or(Department,=,Applications))
   */
   if (gSearchInput.value != "") {
     // replace all instances of @V with the escaped version
     // of what the user typed in the quick search text input
-    searchURI += gQueryURIFormat.replace(/@V/g, encodeURIComponent(gSearchInput.value));
+    searchURI += gQueryURIFormat.replace(/@V/g, encodeABTermValue(gSearchInput.value));
   }
 
   SetAbView(searchURI);
   
   // XXX todo 
   // this works for synchronous searches of local addressbooks, 
   // but not for LDAP searches
   SelectFirstCard();
--- a/suite/mailnews/search/ABSearchDialog.js
+++ b/suite/mailnews/search/ABSearchDialog.js
@@ -272,17 +272,17 @@ function onSearch()
       }
 
       // currently, we can't do "and" and "or" searches at the same time
       // (it's either all "and"s or all "or"s)
       var max_attrs = attrs.length;
 
       for (var j=0;j<max_attrs;j++) {
        // append the term(s) to the searchUri
-       searchUri += "(" + attrs[j] + "," + opStr + "," + encodeURIComponent(searchTerm.value.str) + ")";
+       searchUri += "(" + attrs[j] + "," + opStr + "," + encodeABTermValue(searchTerm.value.str) + ")";
       }
     }
 
     searchUri += ")";
     SetAbView(searchUri);
 }
 
 // used to toggle functionality for Search/Stop button.