Opened 9 years ago

Closed 9 years ago

Last modified 8 years ago

#10946 closed defect (fixed)

[patch][ccla]ComboBox: pressing next/prev buttons autocompletes text in input area

Reported by: floerke Owned by: Douglas Hays
Priority: high Milestone: 1.6
Component: Dijit - Form Version: 1.4.0
Keywords: Cc:
Blocked By: Blocking:

Description

If you define a dijit.form.ComboBox? with autocomplete=false and you use the 'prev...' 'next...' entries to switch between pages, the ComboBox? will do autocomplete.

This is because

_announceOption: function(/*Node*/ node){

does

this._autoCompleteText(newValue);

without testing this.autocomplete

Maybe the test on this.autocomplete should be moved to the function _autoCompleteText avoiding similiar pitfalls.

Attachments (1)

10946.patch (22.4 KB) - added by Douglas Hays 9 years ago.
Don't announceText if the user used the mouse to select More Choices and autoComplete=false . Added automated tests

Download all attachments as: .zip

Change History (16)

comment:1 Changed 9 years ago by bill

Component: GeneralDijit
Owner: anonymous deleted

Huh, you mean that it writes "next" or "previous" in the <input> area when you press the next/prev buttons?

comment:2 Changed 9 years ago by floerke

No, I mean the autocomplete taking place, when paging while autoComplete="false".

Taking an example from docs.dojocampus.org Combobox: """

<div dojoType="dojo.data.ItemFileReadStore?" jsId="stateStore" url="http://docs.dojocampus.org/moin_static163/js/dojo/trunk/dijit/tests/_data/states.json"> </div> <input dojoType="dijit.form.ComboBox?" autoComplete="false" pageSize="5" value="Kentucky" store="stateStore" searchAttr="name" name="state" id="stateInput">

""" Please note the new properties autoComplete="false" and pageSize="5"

Click in the Combobox and press an 'a'. You see five possibilities with an 'a' prefix and no autocomplete in the TextBox?. This is fine.

Now Click on "next" and you see the next three possibilities, but now the autoComplete takes place and the TextBox? completes to "Armed Forces Europe". This is wrong in my opition, because I do not want autoComplete.

comment:3 Changed 9 years ago by bill

Summary: ComboBoxMixin _announceOption does not care about autoComplete offComboBox: pressing next/prev buttons autocompletes text in input area

Oh I see, although I don't see that example on dojocampus, but I was able to reproduce against trunk by modifying the first test in _autoComplete.html to have pageSize=5 instead of 30.

Regardless of the autoComplete setting pressing next shouldn't write anything into the <input>. (Admittedly it has to write into the textbox when doing keyboard navigation using the up/down arrows, but not for mouse)

comment:4 Changed 9 years ago by Douglas Hays

Owner: set to Douglas Hays

comment:5 Changed 9 years ago by Douglas Hays

If the user is using the down arrow keys to iterate over the choices then the announceOption is required so that screen readers will announce the highlighted option. I attached a patch (against trunk) that checks to see if the mouse was used to select More Choices, and if so, then the textbox stays as is. Please test to see if this fixes your problem and doesn't cause any regressions.

comment:6 Changed 9 years ago by Douglas Hays

Summary: ComboBox: pressing next/prev buttons autocompletes text in input area[patch][ccla]ComboBox: pressing next/prev buttons autocompletes text in input area

Waiting on verification of the patch by floerke...

Changed 9 years ago by Douglas Hays

Attachment: 10946.patch added

Don't announceText if the user used the mouse to select More Choices and autoComplete=false . Added automated tests

comment:7 Changed 9 years ago by Douglas Hays

Milestone: tbd1.6

Deferring to 1.6 since no verification from ticket originator.

comment:8 Changed 9 years ago by bill

I tried the patch, and verified that it solves the problem where (mouse) clicking caused the input to display "More Choices". Looks good to me.

comment:9 Changed 9 years ago by Douglas Hays

Resolution: fixed
Status: newclosed

(In [22557]) Fixes #10946. Detect if the mouse was used to select More/Previous? choices and then not announce that selection if autoComplete=false, which was implicitly doing an autoComplete. Added automated tests to select More Choices both with the keyboard and mouse to ensure correct behavior.

comment:10 Changed 9 years ago by bill

Resolution: fixed
Status: closedreopened

[22557] causes failures in !ComboBox_mouse.html on IE8, in the new "Select More Choices" test stuite, on the two test that the widget._focused is set after clicking a choice in the drop down list.

Also, !FilteringSelect_mouse.html calls alls combo.set('displayedValue', null, false) which gets a null pointer exception in getDisplayQueryString(). Again in the new test suite.

comment:11 Changed 9 years ago by Douglas Hays

Resolution: fixed
Status: reopenedclosed

(In [22622]) Fixes #10946. This was a preexisting IE-only problem that no one noticed until the additional test cases were added. If focus was in a ComboBox? and you click the down arrow, then focus was lost which is a bug. I fixed this by assigning focus in a setTimeout since focus cannot be set in a mousedown event handler. Even if focus was restored, then pressing more/previous menu options again removed focus. I added a focus call in the handler for those 2 menu items.

comment:12 Changed 9 years ago by Douglas Hays

(In [22638]) References #10946. Fix FilteringSelect::isValid to check this.item instead of redundant _isvalid boolean. isValid was sometimes returning true after a successful search in a non-autoComplete widget but before any item was actually selected.

comment:13 Changed 9 years ago by Douglas Hays

(In [22690]) References #10946. While actively entering a value, the partial-valid check should check if the dropdown is still open and thus there's some sort of match.

comment:14 Changed 9 years ago by bill

(In [23879]) Fix spurious error on IE6 (waiting for dropdown to show up), and combine the #10946 tests for non-autocomplete mode into the general dropdown-navigation test.

Refs #10946.

comment:15 Changed 8 years ago by bill

Component: DijitDijit - Form
Note: See TracTickets for help on using tickets.