Community
Participate
Working Groups
On focusLost(), CellEditor fires apply editor value always. It should only do this if the user has modified the editor value in some way. So if the user has not made any changes, it should be interpreted as CANCEL. If the use has made changes, it should be like ENTER. The fix is to track if fireEditorValueChanged() is called once or more prior to focus being lost.
I don't see how, but apparently there is some kind of new bug. I just checked Logic example in 2.1.2 and it doesn't have these problems. So my proposed fix would probably have nothing to do with whatever introduced the problem. I'm guessing this code is unchanged for 3.0?
In CellEditor.focusLost(), change code from: if (isActivated()) { fireApplyEditorValue(); deactivate(); } to: if (isActivated()) { if (isDirty()) fireApplyEditorValue(); else fireCancelEditor(); deactivate(); }
That's a start but unfortunately ComboboxCellEditor is screwed up. It doesn't mark dirty when you select something, AND it calls applyEditorValueAndDeactivate all over the place. And applyEditorValueAndDeactivate will force a mark dirty whether changed or not. It shouldn't be doing any of this. It should be doing just standard stuff like mark dirty only when a selection is made that is different than the current selection, and then use the focusLost() that Randy is suggesting.
Tod, this bug has two bad symptoms: 1) If multiple input objects are active and a property is edited where the input objects have different values, then the objects are all set to the first object's value on focus lost. It is sometimes impossible for the user to know that this has happened unless he goes back and selected each item individually and looks at its property values. 2) No-ops are placed on the undo stack as a result of just clicking around on the propertysheet. This can be confusing to the user who may interpret undo as not working.
(In reply to comment #3) ComboBoxCellEditor behaves strange anyway because of a (what I think is a bug) in unfocusing of CCombo, for which see bugzilla 108431. 108431 has been moved to Platform/UI although I think it is a SWT issue. In my opinion what would be best is solve 108431, and then see if the proposed fix by Randy still screwes up ComboBoxCellEditor. The bug reported in this bugzilla is annoying in a case of mine where I have a subclassed TextCellEditor which makes a difference between a null value for a text, and a empty string value. Because a null value and an empty string both show as an empty string in the TextCellEditor, upon focusLost without any changes made by the user, an empty string is applied by applyEditorValue. I'll have to override focusLost in my subclass and test for isDirty() myself. > That's a start but unfortunately ComboboxCellEditor is screwed up. It doesn't > mark dirty when you select something, AND it calls applyEditorValueAndDeactivate > all over the place. And applyEditorValueAndDeactivate will force a mark dirty > whether changed or not. It shouldn't be doing any of this. > > It should be doing just standard stuff like mark dirty only when a selection is > made that is different than the current selection, and then use the focusLost() > that Randy is suggesting.
> ComboBoxCellEditor behaves strange anyway because of a (what I think is a bug) > in unfocusing of CCombo, for which see bugzilla 108431. 108431 has been moved to > Platform/UI although I think it is a SWT issue. In my opinion what would be best > is solve 108431, and then see if the proposed fix by Randy still screwes up > ComboBoxCellEditor. > The bug reported in this bugzilla is annoying in a case of mine where I have a > subclassed TextCellEditor which makes a difference between a null value for a > text, and a empty string value. Because a null value and an empty string both > show as an empty string in the TextCellEditor, upon focusLost without any > changes made by the user, an empty string is applied by applyEditorValue. I'll > have to override focusLost in my subclass and test for isDirty() myself. On which platform does CCombo lose focus when opening?
(In reply to comment #6) > On which platform does CCombo lose focus when opening? On WinXP, Eclipse 3.2M1
Created attachment 42573 [details] Events firing fixed & dirty flag handling in ComboBoxCellEditor Changes in CellEditor & TextCellEditor - apply events are fired only when changes done in the editor Changes in ComboBoxCellEditor - combo marks dirty flag when changes are done - when combo isn't in READ_ONLY mode, it checks inserted text and sets index if exists for the text
Szymon, here's what I've come up with from the patch you gave me... CellEditor: Looks good. I'd change the 'dirty = true' to a 'markDirty()' in 'valueChanged' to pick up subclasses that may have overriddern 'markDirty' and 'valueChanged' but that don't call valueChanged's super(). TextCellEditor: Looks OK... ComboBoxCellEditor: handleDefaultSelection: has a 'since 3.0' tag but it's a new method...should be 3.2 (copy/paste from TextCellEditor maybe?..;-). widgetSelected: Should we check the value before calling 'markDirty'? Iy may be that we don't have to if the platform will only issue the event when the value has changed. focusLost: has the same code as 'handleDefaultSelection'... Szymon, here are a few more serious questions: createControl: Why is there a change from 'keyPressed' to 'keyReleased'...are we -sure- that we arent' re-introducing defect 14201?? keyReleaseOccurred: Why did you remove the handling for the 'tab' key? - I'm confused by the implementation of key up/down handling. Is this necessary? I thought that the arrow keys were handled by the control and turned into 'selection' changes...). Also, rather than using the constants for the keyCodes you should use the equivalent SWT (ARROW_UP & ARROW_DOWN I think).
Created attachment 42972 [details] Changes to editors due to Eric comments (In reply to comment #9) > ComboBoxCellEditor: > > widgetSelected: Should we check the value before calling 'markDirty'? Iy may be > that we don't have to if the platform will only issue the event when the value > has changed. Ok I put condition in widgetSelected() method. It sets dirty only if selection was changed. > focusLost: has the same code as 'handleDefaultSelection'... I made some refactor due to this comment. I extracted applyEditorValueOrCancelAndDeactivate() method as private. Method focusLost is removed from ComboBoxCellEditor, now method from super class is beign used. > Szymon, here are a few more serious questions: > > createControl: Why is there a change from 'keyPressed' to 'keyReleased'...are > we -sure- that we arent' re-introducing defect 14201?? > > keyReleaseOccurred: > > Why did you remove the handling for the 'tab' key? I made it similar to TextCellEditor keyReleaseOccurred method. When 'Tab' is pressed (and released) no keyPressed or keyReleased events occur. I think that it is ok, 'Tab' key handling is not a task of editors. I changed back to keyPressed :-) > - I'm confused by the implementation of key up/down handling. Is this > necessary? I thought that the arrow keys were handled by the control and turned > into 'selection' changes...). Also, rather than using the constants for the > keyCodes you should use the equivalent SWT (ARROW_UP & ARROW_DOWN I think). Pressing arrow keys causes both selections and keys events. But finally I removed this part of the code.
Thanks Szymon, I'llcheck out the new patch and get back to you. It' be a few days though (I just 'discovered' that I need to update one of the examples and add some fairly significant doc info for trim...;-(.
Created attachment 43085 [details] Small changes in the previous patch I changed applyEditorValueOrCancelAndDeactivate() method in the ComboBoxCellEditor. There is a small piece of code which should select an index in the combo based on typed text (not in READ_ONLY mode). Eric please look at applyEditorValueOrCancelAndDeactivate() method... what do you think?
Any updates on this? I'm not able to get the editor to take a value not in the list (when not READ_ONLY) unless I type it in and manually refresh the view, so it doesn't show up until the next read.
Sorry but this went into 'simmer' since it was a somewhat stale defect to start with. I couldn't quite make out what you are trying to do based on your comment...my current guess is that you want to be able to type values into the upper part of the combo and have them added to the list. Is this correct? If not could you give me your useage scenario? Thanks
Thanks for the response - here is my usage scenario: I have a property sheet with editable combo boxes for the values. There is a list of items used to init the combos. I also want to be able to type values in the combo box and have them stick. It would be nice to manipulate the list and possibly add them at the same time, but this is not really even a requirement. Currently, when I type a value in, it shows up properly, but if I leave the cell or push enter, it disappears. I have tried everything, but I can't get the value to stick. Selecting a value from the list works fine.
(In reply to comment #14) > Sorry but this went into 'simmer' since it was a somewhat stale defect to start > with. In what way was this defect stale when it was submitted, or stale now? Could you elaborate? It's a real problem. The validity has NOT changed since reported. The patch is provided for you in comment 2. All you have to do is commit the changes and close the bug.
Found a workaround that seems to fix it for me. Once again, narrowly avoided having to maintain my own distribution;) I'm not suggesting this as a patch, but it might help ppl looking for a workaround in case this doesn't make it into a build. editor.addListener(new ICellEditorListener() { public void applyEditorValue() { String newText = ((CCombo) editor.getControl()).getText(); // First, see if its in the list for (int i = 0; i < labels.length; i++) { String label = labels[i]; if (label.equals(newText)) { // It should already be handled return; } } //Not found, so add the string to the list and select it String[] items = editor.getItems(); labels = new String[items.length + 1]; for (int i = 0; i < items.length; i++) { String string = labels[i]; labels[i] = items[i]; } labels[labels.length - 1] = newText; editor.setItems(labels); editor.setValue(labels.length - 1); }
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.
