<Swing Dev> [9] Review request for 8154431: Allow source and target based validation for the focus transfer between two JComponents.

[Semyon Sadetsky]

Please review the updated webrev:
The fix is updated according to the reviewers comments:

- the test was updated to check the target object and year typo was updated
- new shouldYieldFocus() javadoc improved
- @depricated documentation is added to the old shouldYieldFocus()
- Swing component emphasized in the class javadoc

--Semyon

On 4/20/2016 11:40 AM, Semyon Sadetsky wrote:
> On 4/19/2016 10:41 PM, Phil Race wrote:
>> On 04/19/2016 11:05 AM, Semyon Sadetsky wrote:
>>> On 4/19/2016 7:47 PM, Phil Race wrote:
>>>> Hi,
>>>>
>>>> You are deprecating shouldYieldFocus(JComponent) and yet this class 
>>>> directly uses it.
>>>> Is this deprecation really the right thing to do ?
>>> Why is this not correct? There are plenty examples in JDK: 
>>> Component#setVisible() & Component#show(), Component#transferFocus() 
>>> & Component#nextFocus(), etc...
>>> This is necessary for backward compatibility.
>>
>> My question is why deprecate it ?
> I can repeat my reply to your anther e-mail in this thread if you did 
> not noticed it.
>
> > I deprecated shouldYieldFocus(JCopmponent) and added 
> shouldYieldFocus(JCopmponent, JComponent). I cannot simply remove the 
> first because
> > it's public and can be called from user code. But it's not used in 
> JDK code outside this class anymore only internally for compatibility.
> > I can remove the @Depricated annotation but still cannot get why do 
> we need to keep two overloaded shouldYieldFocus methods if only one is
> >  supposed to be used? That may confuse user which one of them to 
> use, so @Depricated is a hint.
>>
>> So far as I can see unless some one wants to over-ride verifyTarget() 
>> they are
>> fine to continue over-riding this method and ignore the new one.
> Yes, users may continue to use their old code after the fix. No 
> changes required on user side. It seems that is what is the backward 
> compatibility means.
>>
>> Leaving aside the merits of those previous changes, and at least one 
>> of those I think was dubious,
>> all you have done is at an @Deprecated annotation. There is no 
>> @deprecated javadoc tag, and you
>> have left doc which says when this method is called. In fact that doc 
>> is now very misleading.
>> It is not called. You call the new one.
> Agreed. I will add @depricated to the method javadoc and ask user do 
> not use it for the newly written code.
>>
>>
>> BTW I see you mis-spell over-ridden as overriden in verifyTarget(..)
> Just stats from JDK: 5 usages in comments of "over-riden" and more 
> then 300 "overriden"... Google Translate was spoiled :)
>
> --Semyon
>>
>>>>
>>>> The new over-loaded shouldYieldFocus() is perhaps not much more 
>>>> than a utility.
>>>> And the doc says "calls verify(input)" which seems odd since you do not
>>>> directly call it. And you are just describing what the default 
>>>> implementation
>>>> does aren't you ?
>>> This is also necessary for compatibility. There may be 
>>> implementations of the InputVerify where the shouldYieldFocus() is 
>>> overloaded since it is public (that was initial design mistake, I 
>>> suppose). At the same time the shouldYieldFocus() is the entry point 
>>> that plugs InputVerify into the JComponent.
>>> But you are correct "calls verify(input)" is not precisely describes 
>>> what happens in this method. Maybe just make it more indirect, like 
>>> "validate the source and the target components..."?
>>
>> Now I am rather confused. You make it sound like verify() not 
>> shouldYieldFocus() is all the public
>> API should have contained, but you are adding a public over-load of it.
>> So why does this new method need to be public ? All you need is 
>> verifyTarget(), dont you ?
>>
>> -phil.
>>
>>
>>>
>>> --Semyon
>>>>
>>>> -phil.
>>>>
>>>> On 04/19/2016 04:40 AM, Semyon Sadetsky wrote:
>>>>> Hello,
>>>>>
>>>>> Please review fix for JDK9:
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8154431
>>>>> webrev: http://cr.openjdk.java.net/~ssadetsky/8154431/webrev.00/
>>>>>
>>>>> The thing is the Swing validation doesn't allow to validate state 
>>>>> of the target component of input focus transfer operation.
>>>>> To support that the fix adds two new methods 
>>>>> verifyTarget(JComponent target) and shouldYieldFocus(JComponent 
>>>>> source, JComponent target) to the javax.swing.InputVerifier class, 
>>>>> and its old shouldYieldFocus(JComponent input) is marked as 
>>>>> deprecated.
>>>>> The solution guaranties full compatibility with previous JDK versions.
>>>>>
>>>>> --Semyon
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20160422/1cc523b2/attachment.html>