You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jmeter.apache.org by pm...@apache.org on 2013/08/21 14:29:52 UTC
svn commit: r1516145 -
/jmeter/trunk/src/core/org/apache/jmeter/testbeans/gui/ComboStringEditor.java
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();
String text = translate(initialEditValue);
if (text == null) {
text = ""; // will revert to last valid value if invalid
Re: svn commit: r1516145 - /jmeter/trunk/src/core/org/apache/jmeter/testbeans/gui/ComboStringEditor.java
Posted by sebb <se...@gmail.com>.
On 21 August 2013 19:11, sebb <se...@gmail.com> wrote:
> On 21 August 2013 16:10, Philippe Mouawad <ph...@gmail.com> wrote:
>> On Wed, Aug 21, 2013 at 3:43 PM, sebb <se...@gmail.com> wrote:
>>
>>> On 21 August 2013 13:29, <pm...@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.
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.
Re: svn commit: r1516145 - /jmeter/trunk/src/core/org/apache/jmeter/testbeans/gui/ComboStringEditor.java
Posted by sebb <se...@gmail.com>.
On 21 August 2013 16:10, Philippe Mouawad <ph...@gmail.com> wrote:
> On Wed, Aug 21, 2013 at 3:43 PM, sebb <se...@gmail.com> wrote:
>
>> On 21 August 2013 13:29, <pm...@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.
Re: svn commit: r1516145 - /jmeter/trunk/src/core/org/apache/jmeter/testbeans/gui/ComboStringEditor.java
Posted by Philippe Mouawad <ph...@gmail.com>.
On Wed, Aug 21, 2013 at 3:43 PM, sebb <se...@gmail.com> wrote:
> On 21 August 2013 13:29, <pm...@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.
But as to root explanation of why focusLost was not called before
valueChanged, as we say in french "je donne ma langue au chat " :-).
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.
>
> > String text = translate(initialEditValue);
> > if (text == null) {
> > text = ""; // will revert to last valid value if invalid
> >
> >
>
--
Cordialement.
Philippe Mouawad.
Re: svn commit: r1516145 - /jmeter/trunk/src/core/org/apache/jmeter/testbeans/gui/ComboStringEditor.java
Posted by sebb <se...@gmail.com>.
On 21 August 2013 13:29, <pm...@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.
We should probably change all the other occurrences; I'll raise a bug.
This does not fix the issue; but adding requestFocusInWindow() to
JMeterTreeListener seems to have done the trick.
> String text = translate(initialEditValue);
> if (text == null) {
> text = ""; // will revert to last valid value if invalid
>
>