Closed Bug 496970 Opened 16 years ago Closed 14 years ago

can't search address book for contact that includes apostrophe

Categories

(MailNews Core :: Address Book, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wsmwk, Assigned: pi)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

can't search (quick or advanced) address book for contact that includes apostrophe. quote and accent works. 1. create AB entry for O'Dork 2. in quick search field type O'Dork - O, ', D, ... results: as soon as you type ' the search results end expected results: finds the dork :)
I get the same result in Linux x86_64 comm-central trunk. The JavaScript function encodeURIComponent is called on the text entered but apostrophes are not encoded--along with letters, numbers, and - _ . ! ~ * ' ( and ). I'm not sure if that is related to the problem. abCommon.js calls nsIRDFService.GetResource with the following search URI: "moz-abmdbdirectory://abook.mab?(or(PrimaryEmail,c,O'D)(DisplayName,c,O'D)(FirstName,c,O'D)(LastName,c,O'D))" Usually this would reach nsAbDirectoryQuery::matchCardCondition several times but that doesn't seem to happen with a ' in the search string. Next time I have a chance to compile w/ debugging enabled I'll take a look at why this happens.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows Vista → All
Hardware: x86 → All
Attached patch Possible fix (obsolete) — Splinter Review
nsAbDirectoryRDFResource::Init usually only gets called once with the full query URI. If there is an apostrophe in the query URI, then it gets called twice (once for the full query URI and once for an incorrect URI) because the apostrophe isn't being encoded (encodeURIComponent does not escape it). I'm not sure if the fix should go in the search function onEnterInSearchBar (where encodeURIComponent is called) or in the lower-level GetDirectoryFromURI function. This patch does the latter and works for me. Normal: Breakpoint 1, nsAbDirectoryRDFResource::Init (this=0xaf77a300, aURI=0xadaee198 "moz-abmdbdirectory://abook.mab?(or(PrimaryEmail,c,O)(DisplayName,c,O)(FirstName,c,O)(LastName,c,O))") at /home/josh/comm-central/mailnews/addrbook/src/nsAbDirectoryRDFResource.cpp:61 Bad: Breakpoint 1, nsAbDirectoryRDFResource::Init (this=0xaf77ce20, aURI=0xab871508 "moz-abmdbdirectory://abook.mab?(or(PrimaryEmail,c,O'D)(DisplayName,c,O'D)(FirstName,c,O'D)(LastName,c,O'D))") at /home/josh/comm-central/mailnews/addrbook/src/nsAbDirectoryRDFResource.cpp:61 Breakpoint 1, nsAbDirectoryRDFResource::Init (this=0xaf77a300, aURI=0xaf5b8488 "moz-abmdbdirectory://a") at /home/josh/comm-central/mailnews/addrbook/src/nsAbDirectoryRDFResource.cpp:61 Note the incorrect URI in the second call.
Assignee: nobody → joshgeenen+bugzilla
Status: NEW → ASSIGNED
Parentheses do not appear to work, either. MDC says that encodeURIComponent "escapes all characters except the following: alphabetic, decimal digits, - _ . ! ~ * ' ( )" [0]. All of those characters except ' ( and ) work. Search URI with '(' entered in the search box: "moz-abmdbdirectory://abook.mab?(or(PrimaryEmail,c,()(DisplayName,c,()(FirstName,c,()(LastName,c,())" [0] https://developer.mozilla.org/en/Core_JavaScript_1.5_Reference/Global_Functions/encodeURIComponent
This patch should allow searches with apostrophes and parentheses in TB and Seamonkey. Let me know if you would like a unit test for the encodeDirectoryQuery function or basic queries in this patch.
Attachment #385690 - Attachment is obsolete: true
Attachment #386737 - Flags: review?(bugzilla)
Comment on attachment 386737 [details] [diff] [review] The fix (encodes apostrophes and parentheses) I guess there's no other easier way around this. Neil would most likely know. There's a little bit of rot caused by the removal of the resource/ directory. Otherwise r=Standard8 if Neil is happy.
Attachment #386737 - Flags: superreview?(neil)
Attachment #386737 - Flags: review?(bugzilla)
Attachment #386737 - Flags: review+
Comment on attachment 386737 [details] [diff] [review] The fix (encodes apostrophes and parentheses) Don't forget nsAbAutoCompleteSearch.js!
Attachment #386737 - Flags: superreview?(neil) → superreview+
Sadly .replace(/['()]/g, escape) fails because of a hidden parameter :-(
Attached patch Fixes bitrotSplinter Review
This fixes the bitrot on the previous patch. Auto-complete already seems to work for me with contacts containing apostrophes or parentheses, can anyone confirm this?
Attachment #386737 - Attachment is obsolete: true
Attachment #390655 - Flags: superreview+
Attachment #390655 - Flags: review+
(In reply to comment #7) > Sadly .replace(/['()]/g, escape) fails because of a hidden parameter :-( (In reply to comment #8) > Auto-complete already seems to work for me with contacts containing apostrophes > or parentheses, can anyone confirm this? Neil, this works for me as well. Can you explain? how/why?
Attached patch Additional patchSplinter Review
I couldn't get apostrophes to work reproducibly, but while struggling to come up with an answer to the previous question, I thought it might be worthwhile simplifying the string fu in this method; this happens to make apostrophes work anyway (but we still need to encode brackets of course).
Attachment #391894 - Flags: superreview?(bugzilla)
Attachment #391894 - Flags: review?(bugzilla)
Attachment #391894 - Flags: superreview?(bugzilla)
Attachment #391894 - Flags: superreview+
Attachment #391894 - Flags: review?(bugzilla)
Attachment #391894 - Flags: review+
Comment on attachment 391894 [details] [diff] [review] Additional patch Pushed changeset 3bae43b68010 to comm-central.
So the state of play with this patch should be that apostrophes now work in addressbook search and autocomplete, but (a variant of) attachment 390655 [details] [diff] [review] is still needed to fix addressbook search of brackets and autocomplete still doesn't work for brackets.
Blocks: 585483
the main search case works afaict with v3.1. Does anyone think this should not be marked fixed? autocomplete is a different story. filed bug 585483
filed Bug 703493 for apostrophe autocomplete
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
It seems attachment 390655 [details] [diff] [review] never landed here. So the bracket case had to be fixed again in bug 749097, fortunately in the same way.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: