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)
MailNews Core
Address Book
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: wsmwk, Assigned: pi)
References
(Blocks 1 open bug)
Details
Attachments
(2 files, 2 obsolete files)
4.00 KB,
patch
|
pi
:
review+
pi
:
superreview+
|
Details | Diff | Splinter Review |
1.92 KB,
patch
|
standard8
:
review+
standard8
:
superreview+
|
Details | Diff | Splinter Review |
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•16 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•16 years ago
|
||
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•16 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•16 years ago
|
||
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 5•16 years ago
|
||
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•16 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•16 years ago
|
||
Sadly .replace(/['()]/g, escape) fails because of a hidden parameter :-(
Assignee | ||
Comment 8•16 years ago
|
||
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+
Comment 9•16 years ago
|
||
(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?
Comment 10•16 years ago
|
||
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)
Updated•16 years ago
|
Attachment #391894 -
Flags: superreview?(bugzilla)
Attachment #391894 -
Flags: superreview+
Attachment #391894 -
Flags: review?(bugzilla)
Attachment #391894 -
Flags: review+
Comment 11•16 years ago
|
||
Comment on attachment 391894 [details] [diff] [review]
Additional patch
Pushed changeset 3bae43b68010 to comm-central.
Comment 12•16 years ago
|
||
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 | ||
Comment 13•15 years ago
|
||
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
Reporter | ||
Comment 14•14 years ago
|
||
filed Bug 703493 for apostrophe autocomplete
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
![]() |
||
Comment 15•9 years 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.
Description
•