440866 - First AutoCompleteSearch that returns RESULT_NOMATCH cancels all other searches when popup is open

Status: RESOLVED FIXED

(Toolkit :: Autocomplete, defect)

Description

[Ed Lee :Mardak]

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.

Comment 1

[Ed Lee :Mardak]

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

Comment 2

[Felix Fung (:felix)]

Attachment: NO_MATCH in Multiple AutoComplete Searches Cancels Others

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.

Comment 3

[Ed Lee :Mardak]

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.

Comment 4

[Felix Fung (:felix)]

Attachment: NO_MATCH in Multiple AutoComplete Searches Cancels Others

- Removed redundant ONGOING condition

Comment 5

[Ed Lee :Mardak]

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.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

(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

[:Gavin Sharp [email: [e-mail]]]

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!

Comment 8

[Felix Fung (:felix)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/913ee424d1d2

Comment 9

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/913ee424d1d2