jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1516145 - /jmeter/trunk/src/core/org/apache/jmeter/testbeans/gui/ComboStringEditor.java
Date Wed, 21 Aug 2013 18:11:13 GMT
On 21 August 2013 16:10, Philippe Mouawad <philippe.mouawad@gmail.com> wrote:
> On Wed, Aug 21, 2013 at 3:43 PM, sebb <sebbaz@gmail.com> wrote:
>
>> On 21 August 2013 13:29,  <pmouawad@apache.org> 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.

>
>>
>> >          String text = translate(initialEditValue);
>> >          if (text == null) {
>> >              text = ""; // will revert to last valid value if invalid
>> >
>> >
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Mime
View raw message