On 21 August 2013 19:11, sebb <[email protected]> wrote:
> On 21 August 2013 16:10, Philippe Mouawad <[email protected]> wrote:
>> On Wed, Aug 21, 2013 at 3:43 PM, sebb <[email protected]> wrote:
>>
>>> On 21 August 2013 13:29, <[email protected]> wrote:
>>> > Author: pmouawad
>>> > Date: Wed Aug 21 12:29:52 2013
>>> > New Revision: 1516145
>>> >
>>> > URL: http://svn.apache.org/r1516145
>>> > Log:
>>> > Bug 55459 - Elements using ComboStringEditor lose the input value if
>>> user selects another Test Element
>>> > Rollback change using Editor to access value
>>> > Replace requestFocus() by requestFocusInWindow()
>>> > Bugzilla Id: 55459
>>> >
>>> > Modified:
>>> >
>>> jmeter/trunk/src/core/org/apache/jmeter/testbeans/gui/ComboStringEditor.java
>>> >
>>> > Modified:
>>> jmeter/trunk/src/core/org/apache/jmeter/testbeans/gui/ComboStringEditor.java
>>> > URL:
>>> http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/testbeans/gui/ComboStringEditor.java?rev=1516145&r1=1516144&r2=1516145&view=diff
>>> >
>>> ==============================================================================
>>> > ---
>>> jmeter/trunk/src/core/org/apache/jmeter/testbeans/gui/ComboStringEditor.java
>>> (original)
>>> > +++
>>> jmeter/trunk/src/core/org/apache/jmeter/testbeans/gui/ComboStringEditor.java
>>> Wed Aug 21 12:29:52 2013
>>> > @@ -183,11 +183,7 @@ class ComboStringEditor extends Property
>>> > return tags[item-minTagIndex];
>>> > }
>>> > // Not a tag entry, return the original value
>>> > - // combo.getSelectedItem() javadocs says:
>>> > - // If the combo box is editable, then this value may not have
>>> been added to the combo box
>>> > - // with addItem, insertItemAt or the data constructors.
>>> > - JTextComponent textField = (JTextComponent)
>>> combo.getEditor().getEditorComponent();
>>> > - return textField.getText();
>>> > + return (String) value;
>>> > }
>>> >
>>> > /**
>>> > @@ -241,7 +237,7 @@ class ComboStringEditor extends Property
>>> >
>>> > combo.setEditable(true);
>>> >
>>> > - textField.requestFocus();
>>> > + textField.requestFocusInWindow();
>>>
>>> I see that requestFocus() is discouraged as it is platform dependent.
>>> Code should call requestFocusInWindow() instead.
>>>
>> I had already done the changes locally, will commit this evening.
>>
>>> We should probably change all the other occurrences; I'll raise a bug.
>>>
>>> Agree
>>
>>> This does not fix the issue; but adding requestFocusInWindow() to
>>> JMeterTreeListener seems to have done the trick.
>>>
>>
>> Well frankly, I reviewed the changes made by the bug we think introduced
>> the regression. I think it requires a double check.
>> I don't see what could have introduced this. I wonder if it was not working
>> fine by some sort of chance.
>
> I suppose it could have been a side effect of how the code was organised.
> I suspect it was coded correctly, but just not documented, so a subtle
> change to the code caused it to break.
>
>> But as to root explanation of why focusLost was not called before
>> valueChanged, as we say in french "je donne ma langue au chat " :-).
>
> We have "Has the cat got your tongue?" but that has a different
> meaning ("why are you (unexpectedly) silent?")
>
>> With current commit, we force the focus on tree, and by consequence
>> focusLost on all components of TestElement GUI, so it seems clean to me.
>
> Yes, I think it is a much better solution, and I think we can proceed
> on that basis.
>
> However it would still be useful to know why the behaviour changed.
> Just in case there is some other subtle bug lurking.
>
> I hope to try and find out which commit broke it, and see if that
> helps our understanding.
It was definitely the commit associated with Bug 55103
However, that is not an easy change to analyse.
>>
>>>
>>> > String text = translate(initialEditValue);
>>> > if (text == null) {
>>> > text = ""; // will revert to last valid value if invalid
>>> >
>>> >
>>>
>>
>>
>>
>> --
>> Cordialement.
>> Philippe Mouawad.
