Closed Bug 720598 Opened 8 years ago Closed 8 years ago

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

Categories

(Toolkit :: Autocomplete, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla12

People

(Reporter: Gavin, Assigned: mak)

Details

Attachments

(1 file)

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.
I think I can make a test for this
Assignee: nobody → mak77
Attached patch patch v1.0Splinter Review
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.
Attachment #591566 - Flags: review?(gavin.sharp)
Comment on attachment 591566 [details] [diff] [review]
patch v1.0

thanks!
Attachment #591566 - Flags: review?(gavin.sharp) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f2edf4aabf1
Flags: in-testsuite+
Target Milestone: --- → mozilla12
https://hg.mozilla.org/mozilla-central/rev/1f2edf4aabf1
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.