The default bug view has changed. See this FAQ.

can't search address book for contact that includes apostrophe

RESOLVED FIXED

Status

MailNews Core
Address Book
RESOLVED FIXED
8 years ago
10 months ago

People

(Reporter: wsmwk, Assigned: pi)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

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 :)
(Assignee)

Comment 1

8 years ago
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
(Assignee)

Comment 2

8 years ago
Created attachment 385690 [details] [diff] [review]
Possible fix

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
(Assignee)

Comment 3

8 years ago
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
(Assignee)

Comment 4

8 years ago
Created attachment 386737 [details] [diff] [review]
The fix (encodes apostrophes and parentheses)

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 6

8 years ago
Comment on attachment 386737 [details] [diff] [review]
The fix (encodes apostrophes and parentheses)

Don't forget nsAbAutoCompleteSearch.js!
Attachment #386737 - Flags: superreview?(neil) → superreview+

Comment 7

8 years ago
Sadly .replace(/['()]/g, escape) fails because of a hidden parameter :-(
(Assignee)

Comment 8

8 years ago
Created attachment 390655 [details] [diff] [review]
Fixes bitrot

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?
Created attachment 391894 [details] [diff] [review]
Additional patch

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.
(Reporter)

Updated

7 years ago
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
Last Resolved: 5 years ago
Resolution: --- → FIXED

Comment 15

10 months ago
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.