<Swing Dev> [8] Review request for 4199622 RFE: JComboBox shouldn't sending ActionEvents for keyboard navigation
Alexander Scherbatiy
alexandr.scherbatiy at oracle.com
Fri Feb 8 11:11:02 UTC 2013
I am able to reproduce the behaviour from the ComboTest:
cb.getSelectedIndex() = 0
getComboPopup(cb).getList().getSelectedIndex() = 4
Because our initial guess was that these indices are always equal I
suggest to revert this fix to avoid any possible unpredictable regressions.
Thanks,
Alexandr.
On 2/8/2013 2:47 PM, Vladislav Karnaukhov wrote:
> Hello Alex, all,
>
> though I definitely agree with your approach on default JComboBox
> behavior, I don't believe your test really shows a problem.
>
> The method BasicComboBoxUI.getNextIndex is private and is used only to
> determine a delta we should add to or subtract from current list
> position to determine an index of an item we're going to set selected
> in a combo box. Moreover, current implementation always sets list box
> index equal to combo box index when list' popup method is called, see
> BasicComboPopup.show() method. This method is public and theoretically
> could be overridden, but in this case users should now what they're
> doing, right?
>
> When an user navigates with a keyboard, combo box item is always set
> equal to list item (by JComboBox design). With mouse navigation it's
> not true, which is shown by your test.
>
> My property, however, somewhat emulates mouse navigation with a
> keyboard. With this property ON list and combo box indexes may not be
> equal, indeed; but this should not, to my understanding, affect user
> experience. An user may still operate these indexes either separately
> or in conjunction.
>
> Please advice. I'm definitely open to this discussion and, of course,
> am trying to implement this feature as safe and reliable as possible.
>
> Regards,
> - Vlad
>
> On 2/7/2013 10:30 PM, Alexander Potochkin wrote:
>> Hello Alexander
>>>
>>> - key == PAGE_UP / (key == PAGE_DOWN)
>>> Are ui.listBox.getSelectedIndex() and comboBox.getSelectedIndex()
>>> the same when the "ComboBox.noActionOnKeyNavigation"
>>> property is unset? If so, only the ui.listBox.getSelectedIndex() can
>>> be used for both cases.
>>>
>>> - key == HIDE
>>> I would suggest to set the ui.listBox.selected index back to the
>>> comboBox.selected index to prevent some issues where these two indices
>>> are expected to be the same.
>>
>> I would rather not change this code.
>> We have experienced a lot of unexpected issues when we made even more
>> trivial changes.
>> The number of scenarios with custom editors, popups and so on is
>> enormous
>> and sometimes the users can expect those values to be different.
>>
>> Run the test given below.
>> Open the combobox and highlight an item in the middle of the popup.
>> Press backspace.
>> Notice that those values are not the same.
>>
>> I strongly believe that old code should behave *exactly* the same
>> when the
>> "ComboBox.noActionOnKeyNavigation" is off.
>>
>> Would you agree?
>>
>> (Yep, this means that I don't approve this fix at this moment.)
>>
>> Thanks
>> alexp
>>
>> import javax.accessibility.AccessibleContext;
>> import javax.swing.*;
>> import javax.swing.plaf.basic.ComboPopup;
>> import javax.swing.plaf.metal.MetalLookAndFeel;
>> import java.awt.*;
>> import java.awt.event.KeyAdapter;
>> import java.awt.event.KeyEvent;
>>
>> public class ComboTest extends JFrame {
>>
>> public ComboTest() {
>> setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
>> setLayout(new FlowLayout());
>>
>> final JComboBox<String> cb = new JComboBox<>(
>> new String[]{"one", "two", "three","one2", "two2",
>> "three2","one3", "two3", "three3"});
>> cb.setEditable(true);
>>
>> cb.getEditor().getEditorComponent().addKeyListener(new
>> KeyAdapter() {
>> @Override
>> public void keyPressed(KeyEvent e) {
>> System.out.println("cb.getSelectedIndex() = " +
>> cb.getSelectedIndex());
>> System.out.println("getComboPopup(cb).getList().getSelectedIndex() = "
>> +
>> getComboPopup(cb).getList().getSelectedIndex());
>> }
>> });
>>
>> add(cb);
>> setSize(200, 300);
>> }
>>
>> private ComboPopup getComboPopup(JComboBox cb) {
>> AccessibleContext context = cb.getAccessibleContext();
>> for (int i = 0; i < context.getAccessibleChildrenCount();
>> i++) {
>> Object c = context.getAccessibleChild(i);
>> if (c instanceof ComboPopup) {
>> return (ComboPopup) c;
>> }
>> }
>> return null;
>> }
>>
>> public static void main(String... args) throws Exception {
>> UIManager.setLookAndFeel(new MetalLookAndFeel());
>> SwingUtilities.invokeLater(new Runnable() {
>> public void run() {
>> new ComboTest().setVisible(true);
>> }
>> });
>> }
>> }
>>
>>>
>>> Thanks,
>>> Alexandr.
>>>
>>> On 1/30/2013 2:36 PM, Vladislav Karnaukhov wrote:
>>>> Hello Alexandr, all,
>>>>
>>>> please find a new version here:
>>>> http://cr.openjdk.java.net/~vkarnauk/4199622/webrev.05/
>>>>
>>>> On 1/29/2013 07:26 PM, Alexander Scherbatiy wrote:
>>>>>
>>>>> - WindowsLookAndFeel and ComboBox.noActionOnKeyNavigation property
>>>>> is set
>>>>> Press down. Next item is selected and the action listener is
>>>>> invoked. Is it expected behavior (the drop down list is not shown in
>>>>> this case)?
>>>> Yes, it's expected. Customer would like to turn off event firing only
>>>> when popup is open. To keep consistency over different LAFs, I only
>>>> handle the new flag when drop-down is showing. Under Windows, if drop
>>>> list wasn't shown, JComboBox will work as usual even if the flag was
>>>> set.
>>>>
>>>>>
>>>>> - It seems that the code below can have a shorter form:
>>>>> if
>>>>> (UIManager.getBoolean("ComboBox.noActionOnKeyNavigation")) {
>>>>> if (!comboBox.isPopupVisible()) {
>>>>> comboBox.setSelectedIndex(si+1);
>>>>> }
>>>>> } else {
>>>>> comboBox.setSelectedIndex(si+1);
>>>>> }
>>>>> ->
>>>>> if (
>>>>> !(UIManager.getBoolean("ComboBox.noActionOnKeyNavigation") &&
>>>>> comboBox.isPopupVisible()) ) {
>>>>> comboBox.setSelectedIndex(si+1);
>>>>> }
>>>> Agree; fixed.
>>>>
>>>>>
>>>>> - doTest() method in the test throws some exception. It can throw
>>>>> robot
>>>>> and toolkit exceptions as well.
>>>> Agree; fixed.
>>>>
>>>>>
>>>>> Thanks,
>>>>> Alexandr.
>>>>>
>>>
>
>
More information about the swing-dev
mailing list