Closed
Bug 1131685
Opened 9 years ago
Closed 9 years ago
left arrow key only skips the last character of the highlighted search suggestion
Categories
(Firefox :: Search, defect)
Firefox
Search
Tracking
()
Tracking | Status | |
---|---|---|
firefox39 | --- | verified |
People
(Reporter: kjozwiak, Assigned: enndeakin)
References
Details
Attachments
(2 files, 1 obsolete file)
184.00 KB,
image/gif
|
Details | |
4.96 KB,
patch
|
mak
:
review+
|
Details | Diff | Splinter Review |
When you have search suggestions being listed under the search toolbar, selecting one of the suggestions using the arrow keys and than pressing the left arrow on a particular suggestion will only go to the left once (one character). Pressing it the second time onwards won't do anything. An example would be a user searching for something and wants to either correct the current selected suggestions or append another term. Right now, the user would only be able to move backwards once (a single character). Reproducible on all the OS's: - Win 8.1 x64 - OSX 10.10.2 x64 - Ubuntu 14.04.1 x64 STR: - click on the search toolbar and type "firefox" - once you get the suggested terms appearing, press the down arrow key and select a suggestion from the list - once one of the listed suggestions is highlighted, press the "left" arrow key and you'll notice that you'll only be able to skip the last character and won't be able to go any further. Attached a .gif of the issue (notice that I can only skip the last character and can't go any further than that)
Updated•9 years ago
|
Assignee | ||
Comment 1•9 years ago
|
||
Skip all of the autocomplete handling when the popup is open and noRollupOnCaretMove is set.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•9 years ago
|
||
This is sort of a regression caused by 1116865, although without that change the popup would close and not cause the issue when the caret was moved twice.
Updated•9 years ago
|
Iteration: --- → 38.3 - 23 Feb
Flags: qe-verify?
Updated•9 years ago
|
Flags: qe-verify? → qe-verify+
Assignee | ||
Updated•9 years ago
|
Attachment #8562787 -
Flags: review?(mak77)
Comment 3•9 years ago
|
||
Comment on attachment 8562787 [details] [diff] [review] Fix Review of attachment 8562787 [details] [diff] [review]: ----------------------------------------------------------------- there is a behavioral change here, where left/right/home with this change won't autofill anymore, making this field act very differently from the other autocompletes. In most cases the suggested behavior sounds fine, but what if an ac field doesn't have completeSelectedIndex, but it has noRollup? If I select an entry and press right, I'd expect the selected entry to be completed, so I can keep typing after it, but it won't be completed anymore. Maybe we should make it be if (noRollup && "CompleteSelectedIndex"), so basically noRollup will only work for CompleteSelectedIndex fields? Alternatively, isn't the problem here the call to input->SelectTextRange(value.Length(), value.Length()); this seems to make sense only when the cursor is already at the end before we setTextIndex, would changing that make sense more generally for autocomplete fields?
Updated•9 years ago
|
Flags: needinfo?(enndeakin)
Assignee | ||
Comment 4•9 years ago
|
||
> Alternatively, isn't the problem here the call to
> input->SelectTextRange(value.Length(), value.Length());
> this seems to make sense only when the cursor is already at the end before
> we setTextIndex, would changing that make sense more generally for
> autocomplete fields?
I tried both of these and the latter doesn't feel quite right. The selected item gets filled in inconsistently. For instance, when at the end of the field and moving the caret left, the item gets filled, the textbox changes and the cursor is one step from the end.
Flags: needinfo?(enndeakin)
Assignee | ||
Comment 5•9 years ago
|
||
This is the former suggestion.
Attachment #8562787 -
Attachment is obsolete: true
Attachment #8562787 -
Flags: review?(mak77)
Attachment #8567111 -
Flags: review?(mak77)
Updated•9 years ago
|
Iteration: 38.3 - 23 Feb → 39.1 - 9 Mar
Comment 6•9 years ago
|
||
Comment on attachment 8567111 [details] [diff] [review] searchcarettwice Review of attachment 8567111 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/autocomplete/nsAutoCompleteController.cpp @@ +491,5 @@ > uint32_t minResultsForPopup; > input->GetMinResultsForPopup(&minResultsForPopup); > if (isOpen || (mRowCount > 0 && mRowCount < minResultsForPopup)) { > + // For completeSelectedIndex autocomplete fields, if the popup shouldn't > + // close when the the caret is moved, don't adjust the value or caret. typo: the the caret and s/value or caret/position of the caret/
Attachment #8567111 -
Flags: review?(mak77) → review+
Assignee | ||
Comment 7•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/9e73726c6d30
OS: Mac OS X → All
Hardware: x86 → All
https://hg.mozilla.org/mozilla-central/rev/9e73726c6d30
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
Comment 9•9 years ago
|
||
I was able to reproduce this issue on Firefox 38.0a1 (2015-02-10) using Windows 7 64bit and Ubuntu 14.04 32bit. Verified fixed on Firefox 39.0a1 (2015-03-12) using Windows 7 64bit, Ubuntu 14.04 32bit and Mac OS X 10.9.5.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•