Closed
Bug 440866
Opened 17 years ago
Closed 13 years ago
First AutoCompleteSearch that returns RESULT_NOMATCH cancels all other searches when popup is open
Categories
(Toolkit :: Autocomplete, defect)
Toolkit
Autocomplete
Tracking
()
RESOLVED
FIXED
mozilla11
People
(Reporter: Mardak, Assigned: felix)
Details
Attachments
(1 file, 1 obsolete file)
1.29 KB,
patch
|
Gavin
:
review+
Mardak
:
feedback+
|
Details | Diff | Splinter Review |
If you specify multiple searches for an autocomplete, if there are any that return RESULT_NOMATCH before the other searches finish, all the other ones will get canceled if the popup is already open.
This is because the first one that gives RESULT_NOMATCH makes the autocomplete controller close the popup, which causes the other searches to stop for some reason.
For a workaround, return RESULT_NOMATCH_ONGOING but you'll need to make sure to eventually give a RESULT_NOMATCH.. otherwise it'll think it's still searching.
Reporter | ||
Comment 1•14 years ago
|
||
(In reply to comment #0)
> controller close the popup, which causes the other searches to stop for some
> reason.
This is probably...
nsAutoCompleteController::ProcessResult
if mRowCount == 0 and result != ONGOING -> call ClosePopup()
eventually this triggers...
autocomplete.xml#autocomplete-base-popup popuphiding handler
unconditionally calls controller.stopSearch()
Assignee | ||
Updated•13 years ago
|
Assignee: nobody → ffung
Assignee | ||
Comment 2•13 years ago
|
||
I don't think individual searches should return NO_MATCH_ONGOING because that would require an individual search to know of the status of other searches in some way or another.
Attachment #572614 -
Flags: review?(edilee)
Reporter | ||
Comment 3•13 years ago
|
||
Comment on attachment 572614 [details] [diff] [review]
NO_MATCH in Multiple AutoComplete Searches Cancels Others
>+++ b/toolkit/components/autocomplete/nsAutoCompleteController.cpp
>@@ -1301,20 +1301,22 @@ nsAutoCompleteController::ProcessResult(
>- else if (result != nsIAutoCompleteResult::RESULT_NOMATCH_ONGOING)
>+ } else if (mSearchesOngoing == 0 &&
>+ result != nsIAutoCompleteResult::RESULT_NOMATCH_ONGOING) {
Why not just check mSearchesOngoing == 0 without the other ONGOING check? If result /was/ ONGOING, mSearchesOngoing would not be 0.
Assignee | ||
Comment 4•13 years ago
|
||
- Removed redundant ONGOING condition
Attachment #572614 -
Attachment is obsolete: true
Attachment #573603 -
Flags: review?(edilee)
Attachment #572614 -
Flags: review?(edilee)
Reporter | ||
Comment 5•13 years ago
|
||
Comment on attachment 573603 [details] [diff] [review]
NO_MATCH in Multiple AutoComplete Searches Cancels Others
I'm not sure what's the styling rules are anymore about bracing single line ifs.
Attachment #573603 -
Flags: review?(gavin.sharp)
Attachment #573603 -
Flags: review?(edilee)
Attachment #573603 -
Flags: feedback+
Comment 6•13 years ago
|
||
(In reply to Felix Fung (:felix) from comment #2)
> I don't think individual searches should return NO_MATCH_ONGOING because
> that would require an individual search to know of the status of other
> searches in some way or another.
RESULT_NOMATCH_ONGOING means that the individual search hasn't yet produced results, but is still attempting to. I'm not sure why this is useful to indicate, though. The controller can't really do anything in this case, and there's no reason to notify if there are no new results, AFAICT...
Comment 7•13 years ago
|
||
Comment on attachment 573603 [details] [diff] [review]
NO_MATCH in Multiple AutoComplete Searches Cancels Others
Seems like it should be pretty easy to write a test for this (similar to test_378079.js). r=me with that!
Attachment #573603 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 8•13 years ago
|
||
Comment 9•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla11
Updated•13 years ago
|
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•