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

[Semyon Sadetsky]

On 4/20/2016 12:11 AM, Phil Race wrote:
> PS I see the class doc talks about shouldYieldFocus() being called,
> so I don't understand the inter-relationship of that and verify(), but
> it makes me no more sure that deprecating that method is right.
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.
>
> I think perhaps verify() is all that the app is supposed to have 
> over-ridden
> and the comments are written on the assumption it would not over-ride
> anything else.
Right. But in fact shouldYieldFocus is public and not final.
> So perhaps the design mistake could be that shouldYieldFocus() was not 
> final.
> Maybe your new one should be - if it is public at all ...
Probably you are correct. Before this method had the same signature as 
verify() and there was no reason for user to change its default 
implementation because it would not provide anything extra . I think it 
should be package private then.
But now it has 2 arguments and can be taken in consideration as some 
intermediate step in which the results obtained from the verify() and 
the new verifyTarget() are combined, and user may want to change the 
behavior how they are combined. Does it make sense?
>
> BTW you removed text that says
>
> Before focus is transfered to another Swing component
> The key word here is SWING - I looked at the JComponent code
> and it returns true if either source or target is not a JComponent,
> without ever getting near the call to shouldYieldFocus().
I know. That is why I changed the description of the class and its first 
statement now:

" This class provides the validation mechanism for Swing components."

I think we can emphasize this additionally. What do you think about the 
next:

   39  * Before focus is transferred from the source Swing component to 
the target
   40  * Swing component

?

--Semyon

>
> -phil.
>
>
> On 04/19/2016 12: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 ?
>>
>> 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.
>>
>> 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.
>>
>>
>> BTW I see you mis-spell over-ridden as overriden in verifyTarget(..)
>>
>>>>
>>>> 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/20160419/2de55b80/attachment.html>