720598 - nsAutocompleteController HandleKeyNavigation() and EnterMatch() call GetDefaultCompleteValue() with a bogus aResultIndex

Status: RESOLVED FIXED

(Toolkit :: Autocomplete, defect)

Description

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

GetDefaultCompleteValue()'s aResultIndex argument is supposed to be the index of the result to use within mResults.

HandleKeyNavigation() and EnterMatch() both try to check the default complete value in order to restore the proper case after an item is deemed to have been "selected" (by pressing a right/left arrow, or by pressing enter). However, they do this by passing in popup's selected index as aResultIndex, which is not at all related to nsIAutocompleteResults. The result is that GetDefaultCompleteValue() in these cases will fail unless you happen to have the right item selected in the popup (usually the first or second), in which case the callers just do nothing.

To do this properly, I guess it would need to use RowIndexToSearch() to convert the popup selectedIndex into an index into mResults.

Comment 1

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

I think I can make a test for this

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: patch v1.0

In the end this was just confusing code, selectedIndex is likely always -1 there, since otherwise we'd take the other path in the if condition.
So this hardcodes -1 and adds a generic test for completeDefaultIndex, not incredibly useful but more testing > less testing.

Comment 3

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

Comment on attachment 591566 [details] [diff] [review]
patch v1.0

thanks!

Comment 4

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1f2edf4aabf1

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1f2edf4aabf1