Forget to attach the link: http://cr.openjdk.java.net/~ssadetsky/8154431/webrev.01/

--Semyon

On 4/22/2016 11:08 AM, Semyon Sadetsky wrote:
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







Reply via email to