Community
Participate
Working Groups
1) Drop down a list of choices 2) Select a choice that is different than the currently selected value. valueApplied should be fired by the cell editor.
I wonder if this is a consequence of the changes SWT made to the CCombo control to fix the extra events that were being fired...
It is not related. This bug pre-dates the fix for the other bug. I checked.
This was a new feature from Stefan
I submitted ComboViewer (not in head yet), not ComboBoxCellEditor. This is unrelated.
When you say valueApplied you mean valueChanged(boolean oldValidState, boolean newValidState)correct? Right now we cache the selection and thats it. Application happens on default selection or loss of focus.
"Application happens on default selection" Then perhaps SWT is no longer firing default selection? Either way, you probably have no way of visually verifying this bug without a real client of the propertysheet/celleditors. If you are sure that the cell editor's code has not changed since 2.1, then write an SWT snippet that shows that default selection is no longer fired.
I can test using the Tesk list that default selection is working. STEPS 1) Add a task 2) Bring up the cell editor on the priority 3) Select an entry 4) Hit return And it is applied If you think there is some behaviour that is not happening please give clear steps that I can follow - I have no idea what the problem is you are referring to.
Sorry for not being clear. Steps 1&2 must both be performed using the Mouse, not the keyboard.
Actually, in your own list of steps, the value should have been applied at step 3. For example, if the task is currently set to LOW priority, and the CellEditor appears beside the icon, then you should see the icon change immediately when the new value is selected, not upon ENTER. BTW, I tried sorting task list by priority, which works once. But if you change a task's priority, it does not get resorted. Is this a bug?
*** Bug 54377 has been marked as a duplicate of this bug. ***
Can you give me an example of different behaviour in 2.1? I am checking with the cell editor there but it is covering the icon so I can't see if this has been going on since 2.1 or not.
The different behavior is what I said in comment 1, "valueApplied should be fired by the cell editor" I have unwrapped ComboBoxCellEditor for you and isolated the SWT change: public static void main(String[] args) { Display display = new Display(); Shell shell = new Shell(); shell.setLayout(new GridLayout()); final CCombo combo = new CCombo(shell, 0); combo.setItems(new String[] {"item 1","item 2","item 3"}); combo.addSelectionListener(new SelectionAdapter() { public void widgetDefaultSelected(SelectionEvent e) { System.out.println("widget default selected"); } }); shell.setSize(400,300); shell.open(); while (!shell.isDisposed()) if (!display.readAndDispatch()) display.sleep(); } Steve, "widget default selected" is no longer fired when you select an item with the mouse. This behavior differs from the 2.1 behavior. My guess is you are trying to make it act more like Combo. Is it possible to make Combo like CCombo was instead? It is useful to detect the difference between using ARROW keys to scroll through the choices, and mouse clicking directly on the choice you want. Given the current behavior, these two actions cannot be distinguished.
So the workaround for us would be to make widgetSelected and widgetDefaultSelected do the same thing. The problem with doing that is that you could not use the mouse to navigate then as the editor would keep closing on you.
I didn't understant the last comment. The workaround is problematic only when using the keyboard to navigate through the various choices. In other works: ALT+DOWN.. DOWN.. DOWN.. DOWN.. ENTER should produce 1 default selected event. This is why I prefer the old behavior, and proposed that Combo be change to emulate CCombo.
Randy, widgetDefaultSelected() should never have been sent from the mouse. It is sent when you hit Return in a Combo. Can you confirm that this has changed between 2.x and 3.x? One thing, Return is also used for the default button. If you have a default button in a window, it gets the Return first and eats it as part of SWT.TRAVERSE_RETURN. Is that what you are seeing?
Need to decide on the answer for 3.0
Steve, the test snippet is listed in comment 12. I verified the change between 2.1 and 3.0. I don't care exactly what SWT does, but there needs to be a way to distinguish between navigating via keyboard, and selecting with the mouse. Users expect to see an immediate update (fire value applied) becaues this is how Combos behave when they are in toolbars (for example, Zoom, Font Family, Font Size, etc.)
The "widgetDefaultSelected" bug has been fixed in CCombo for 3.0. It was bogusly sent in 2.1.x. The "widgetSelected" method is called when an item is selected from the list. Nothing more for SWT to do here.
Steve, from what you are saying, it is impossible to restore the 2.1 behavior. If widgetSelected is used to fire valueApplied, then keyboard users will have valueApplied fired repeatedly as they scroll up and down the list of choices using the arrow keys. Bring up Display Properties in windows XP. Press ALT+DOWN to drop down the choice of themes. If you use the arrow keys to select a theme, the preview area does not reflect the selection changing. But, if you select a choice from the drop down with the mouse, the preview area *does* update. How can we make this distinction in SWT? Is there an event when the drop-down is rolled-up? I don't think a mouseDown event is dispatched when clicking on the dropdown.
I have verified the widgetDefaultSelected event with Eclipse 2.1, 2.1.3 and M8, using the code snippet from <A HREF="#c12">comment 12</A>. For eclipse 2.1 and 2.1.3: Event is fired when: - An item is selected with the mouse. - The drop down is shown by clicking on the arrow, then an item is selected from the drop down with the up and down arrow keys, and the enter key is pressed on the selected item. Event is NOT fired when: - Traversing the items in the pull down using only the up and down arrow keys. For M8: Event is fired when: - The drop down is shown by clicking on the arrow, then an item is selected from the drop down with the up and down arrows, and the enter key is pressed on the selected item. Event is NOT fired when: - An item is selected with the mouse. - Traversing the items in the pull down using only the up and down arrow keys.
ALT+UP_ARROW can also cause the drop-down to rollup. It would be nice if this fired some event as well.
Closing as this change is a result of an SWT fix.
CAR, can you decode all this and make sure that CCombo and Combo are doing the right thing? Specifically we need to confirm that we are ok with comment #20. Thanks.
This change in behavior is unacceptable for 3.0.
Specifically, the CellEditor behavior is unacceptable because of its widespread use in the property sheet. I do not have an opinion on the CCombo change.
Randy, I'm not sure why you removed Steve from the cc list when you added me. I assume it was an accident. I'm reassigning this to the SWT component owner since the CCombo change is under review by SWT. If there will be a change to the notifications from what we had in 2.1, please move this back to UI so we can fix up ComboBoxCellEditor accordingly.
Created attachment 9307 [details] Excel spreadsheet showing Combo vs. CCombo events Here's another snippet (pasted below) that compares Combo with CCombo (Editable and Read-Only). I generated the attached Excel Spreadsheet ;) to compare selection and defaultSelection events generated for various user mouse/keyboard actions (if I missed any important ones, let me know). There are a lot of differences in the 'selection' event generation. The 'defaultSelection' events are somewhat more consistent: if the user hits Enter with the list visible or not, they get defaultSelection. (If they hit TAB in a CCombo, they get defaultSelection too - so I can see why everybody got used to hooking default selection in their property sheets). I do think that the eclipse 2.1 behavior was bogus, and that it is correct not to fire defaultSelection when an item is selected. Selecting an item should, and does, fire the 'selection' event. (Of course, it is probably also bogus to have Tab generate defaultSelection... Steve?). Having said that, however, I can see that apps would like to get a consistent event for "the user is really done selecting now". (or... "Is that your final answer?" :) And it seems that users cannot be trusted to hit 'Enter' - they tend to TAB out of things, or 'select-with-mouse-and-then-go-click-elsewhere', without ever dreaming that the UI was expecting an 'Enter'. ;) Steve - what is CCombo's "raison d'etre"? I think we had to create it because Combo had a fixed size on some platforms and it looked too big and ugly in a Table - right? Has anybody checked lately if this is still true? CCombo looks pretty dated on XP... (but I guess that's another issue...) Maybe we need to be thinking that CCombo is a whole different beast, and not trying to make it behave like Combo (because we are failing at that anyhow). Maybe CCombo is just for being in Tables. Maybe it's ok to change the semantics of defaultSelection to mean "the list is gone and the user has really selected something". Other than a new event for Show/Hide of the list, I can't think of anything else to help these people. The advantage of adding the new event is that it allows us to think that we might one day be able to make CCombo look & feel exactly like Combo (with an extra event). Another advantage is that we don't have mixed-up semantics for defaultSelection in SWT. But adding a new event is a bit more of a pain for the apps - they are less likely to think of checking for every possible case. The old bogus behavior lets the app just hook defaultSelection, and they will catch the user hitting Enter, TAB, or select-with-mouse. (Hmmm... just noticed that you will *not* catch the user using arrow keys to select *without* dropping down the list. i.e. TAB to give the CCombo focus, use UP or DOWN, then TAB out... how do you guys handle that case today? You should only be getting 'selection'... are you bogus in that case?) Here's the new snippet: import org.eclipse.swt.*; import org.eclipse.swt.widgets.*; import org.eclipse.swt.layout.*; import org.eclipse.swt.events.*; import org.eclipse.swt.custom.*; public class CComboAccessibilityTest { static Display display; static Shell shell; static Combo combo, readOnlyCombo; static CCombo ccombo, readOnlyCcombo; public static void main(String[] args) { display = new Display(); shell = new Shell(display); shell.setLayout(new GridLayout(4, true)); shell.setText("CCombo Accessibility Test"); Label comboLabel = new Label(shell, SWT.None); comboLabel.setText("Platform (Edit, ComboBox, ComboLBox)"); comboLabel.setLayoutData(new GridData(GridData.HORIZONTAL_ALIGN_CENTER)); Label editableComboLabel = new Label(shell, SWT.None); editableComboLabel.setText("Platform-ReadOnly"); editableComboLabel.setLayoutData(new GridData(GridData.HORIZONTAL_ALIGN_CENTER)); Label ccomboLabel = new Label(shell, SWT.None); ccomboLabel.setText("CCombo (Edit, Button, ListBox)"); ccomboLabel.setLayoutData(new GridData(GridData.HORIZONTAL_ALIGN_CENTER)); Label editableCcomboLabel = new Label(shell, SWT.None); editableCcomboLabel.setText("CCombo-ReadOnly"); editableCcomboLabel.setLayoutData(new GridData(GridData.HORIZONTAL_ALIGN_CENTER)); SelectionListener listener = new SelectionListener() { public void widgetSelected(SelectionEvent e) { System.out.println("selection"); } public void widgetDefaultSelected(SelectionEvent e) { System.out.println("default selection"); } }; combo = new Combo(shell, SWT.NONE); for (int i = 0; i < 5; i++) { combo.add("item" + i); } combo.setText("item0"); combo.setLayoutData(new GridData(GridData.HORIZONTAL_ALIGN_CENTER)); combo.addSelectionListener(listener); readOnlyCombo = new Combo(shell, SWT.READ_ONLY); for (int i = 0; i < 5; i++) { readOnlyCombo.add("item" + i); } readOnlyCombo.setText("item0"); readOnlyCombo.setLayoutData(new GridData(GridData.HORIZONTAL_ALIGN_CENTER)); readOnlyCombo.addSelectionListener(listener); ccombo = new CCombo(shell, SWT.BORDER); for (int i = 0; i < 5; i++) { ccombo.add("item" + i); } ccombo.setText("item0"); ccombo.setLayoutData(new GridData(GridData.HORIZONTAL_ALIGN_CENTER)); ccombo.addSelectionListener(listener); readOnlyCcombo = new CCombo(shell, SWT.BORDER | SWT.READ_ONLY); for (int i = 0; i < 5; i++) { readOnlyCcombo.add("item" + i); } readOnlyCcombo.setText("item0"); readOnlyCcombo.setLayoutData(new GridData(GridData.HORIZONTAL_ALIGN_CENTER)); readOnlyCcombo.addSelectionListener(listener); shell.pack(); shell.open(); while (!shell.isDisposed()) { if (!display.readAndDispatch()) display.sleep(); } } }
My god, we may have to put back the bogus SWT.DefaultSelection for the single selection case. I will check with Veronika when she gets back. It seems that we shouldn't be sending it when the user tabs out of a CCombo. The right answer should be to remove it and fix ComboBoxCellEditor to use the SWT.Traverse callback to do it. ComboBoxCellEditor must already be doing something when the user clicks off with the mouse. Thoughts?
Yes, on focus lost, all types of cell editors fire value applied already.
VI is back! She now owns the problem. Here is my initial position: I believe we need to either completely remove the bogus SWT.DefaultSelection notification or put it all back.
1) https://bugs.eclipse.org/bugs/show_bug.cgi?id=41347 This bug report requested that CCombo NOT fire the default selection on mouse selection - thus the change in 3.0. Since this is consistent with Combo, I think this is the correct behaviour. I am still thinking about the case where the application wants the value applied immediately when selected by the mouse versus delayed when selected by arrows in the list versus ? when selected by arrows in the text field. 2) I can remove the default selection on tabbing from the CCombo. I agree that this is behaviour teh application should provide and indeed, as Randy said in comment #29, Eclipse already handles the focus lost event which would copver the tab case as well.
Please fix the tab thing for 3.0. If the application wants the value to apply right away, then it should hook the SWT.Selection callback as well.
Is it impossible to send a "rolled up" event from CCombo and Combo? This is the *only* way to simulate the behavior seen on Internet Explorer's Address bar combo. Both ALT+UP and MOUSE click cause the combo to "go", but changing selection via DOWN, PAGE_DOWN, etc, does not cause it to go. What is the SWT way to do this??
Created attachment 9548 [details] Display Properties gets it wrong "tab in popup list causing DefaultSelection" removed in head R3.0. I am not going to add the default selection on mouse selection for the following reason - it causes inconsistencies between the value displayed in the text field and the value applied. For example, see the Display Properties dialog attached here. I opened the list for the Theme combo using Alt+Down. I then used the arrow keys to change from Windows XP to Windows Classic. Then I hit Escape. Notice that the Sample shows the Windows XP style but the Theme shows the Windows Classic style. Which one is right? Even Windows does not handle this correctly. Notice also that in the same dialog, under the Appearance tab, the "Windows and buttons" Combo always applies the change on Selection (not Default Selection). So they are not even consistent within one application.
The "default selection on tab" problem has been fixed - there is no longer a default selection on tab. The "no default selection on mouse selection in list" problem will not be fixed - this is intentional.
VI, I am not suggesting that you fire default selected, I am just suggesting that you make it possible to create an IE address-bar-like behavior. There are times when doing something on selection change is cheap and fast and it is OK to do so, and then there are times (such as loading a large WinXP theme, or navigating to a website, or executing an undoable command from the property sheet) when you should wait until the user is really finished selecting what they want to select. In the case of the propertysheet, if the user is selecting the layout manager for container, then it is unacceptable for the choices between the current and the desired to be applied. If the intermediate values are applied (e.g. keyboard user pressing DOWN), the result is *not* the same. Therefore, ComboboxCellEditor cannot apply value on selectionChanged event.
I understand the requirement. I will have a look at what events are provided by the native Combo widget to see what is possible.
When it is expensive, use the SWT.DefaultSelection event and make the guy press CR for now?
I'm reopening this bug. It's fine that the focus-lost part has been fixed, but the original problem is not addressed yet.
Should this be moved to UI? Veronika?
Randy, you are looking for "IE address-bar-like behavior" but if you look at that case as an example you will see that it behaves quite unlike a combo box. For example, you can not use the up and down arrows in the text area to change the selection. If you do not want to have a mismatch between the selection shown in the the text area and the selection you have applied to your UI you will need more than just the ability to know when the list has been closed.
Can we close this stupid PR yet?
From eclipse-dev mailing list: <quote> I am using a combobox cell editor with table. also, i am using a cell modifier subclass. In windows when i run the code, the selection event of the combo box is correctly fired in the ComboBoxCellEditor before the modify() method is called in the CellModifier. However, in linux (red hat version 9), the order is reversed resulting in that always the modify method is called before the widgetSelected() method and thus, the modify event always gets -1 in the value parameter. </quote> Using call hierarchy on: + ICellModifier.modify(...) + saveEditorValue (TableViewerImpl) + applyEditorValue (TableViewerImpl) + initCellEditorListener (""") the last method there is creating an anonymous inner class to listen for "applyEditorValue()". So this is probably the same problem.
Sounds like completely different problem to me (a platform difference in the ordering of the events). Is there a PR and a test case for it? Randy, I'm going to close this PR. You can open separate PR's that describes the combo box behavior you want or you can try and implement what you want and see which API is missing and enter problem reports for that. In any case, "[CellEditors] [regression] ComboBoxCellEditor does not fire value applied" no longer captures this discussion and we will not be changing either SWT.Selection or SWT.DefaultSelection here to fix that (which is what the problem report was about).
It is a related problem. Although the event ordering is probably an unrelated bug, that bug was conveniently hidden by the widgetDefaultSelected event being fired in the past.
moving back to UI for workaround
We don't intend to diverge from the SWT behaviour if it all possible. If you have a suggested patch please attach here.
Moving to later as we will not have time to address this for 3.0.
The workaround for CCombo would be to add a Shell listener, and check shell activation. I tried this and it works. If normal selection event is received when the Shell is not active, defer sending fireValueApplied until the shell becomes active again. Otherwise, the user is using UP/DOWN arrow to change selection without dropping down the list. So in these cases fire value changed immediately, or after a slight delay. Carolyn, if you add a KeyListener to the CCombo in your snippet, it is no longer traversible. I'm going to open a separate bug.
Tod, this has nothing to do with SWT behavior, but a limitation. The "behavior" just happened to be correct in 2.1 but is no longer.
re-opening
marking as WONTFIX, it is unfortunate that the bug was in SWT in 2.1 but working around it might just as well introduce other problems
Tod, I've opened bug 73478.
A similar issue is present in ComboBoxViewerCellEditor. A possible workaround is described in https://bugs.eclipse.org/bugs/show_bug.cgi?id=230398