<AWT Dev> [7] Review Request for 8047961: [TESTBUG] closed/java/awt/Focus/ActivateFocusTest/ActivateFocusTest.html fails intermittently

Anton V. Tarasov anton.tarasov at oracle.com
Mon Dec 1 12:14:24 UTC 2014


Hi Mikhail,

The modified test won't do anything weired, it will test something that meets the spec. However, I 
agree that it won't test the original incident found in 1.4 comparing to jdk 1.3. But since we 
agreed not to treat it as a regression against the spec, I'm ok with the test removal.

Thanks,
Anton.

On 28.11.2014 19:09, mikhail cherkasov wrote:
> Hi Anton, Oleg.
>
> I vote for removing the test.
>
> Rewriting test just will produce something weired that won't test what original regression does.
> A new test won't be a regression test that check problem was found in jdk 1.4.
> I believe jdk4 and 7 has a lot of difference in focus system and due those changes the test became
> irrelevant.

(It's not as different as 1.3 comparing to 1.7, taking into account the fact that focus had been 
drastically refactored in 1.4.)

>
> Thanks,
> Mikhail.
>
> On 11/17/2014 5:42 PM, Anton V. Tarasov wrote:
>> On 17.11.2014 17:38, Oleg Sukhodolsky wrote:
>>> On Mon, Nov 17, 2014 at 5:31 PM, Anton V. Tarasov
>>> <anton.tarasov at oracle.com> wrote:
>>>> On 17.11.2014 17:21, Oleg Sukhodolsky wrote:
>>>>> On Mon, Nov 17, 2014 at 4:34 PM, Anton V. Tarasov
>>>>> <anton.tarasov at oracle.com> wrote:
>>>>>> On 17.11.2014 15:01, Oleg Sukhodolsky wrote:
>>>>>>> Hi Anton,
>>>>>>>
>>>>>>> the bug was a regression introduced in 1.4 (comparing with 1.3.1) this
>>>>>>> is why it was fixed and the test was written.
>>>>>>> Indeed the spec doesn't guarantee that the test will work but at the
>>>>>>> time we were working on the ticket it was decided that we should not
>>>>>>> allow such regressions.
>>>>>>> If (according to the current policy) the spec is more important than
>>>>>>> compatibility in this case I'd suggest to remove the test completely
>>>>>>> since its new version doesn't test for the regression we had earlier
>>>>>>> but just test spec in a very strange/complicated way.
>>>>>>
>>>>>> The test passes on Windows, but fails on Mac and I suppose Linux. I
>>>>>> recall I
>>>>>> made attemps to fix the failure on Linux (in 1.6 or 1.7) but that was
>>>>>> quite
>>>>>> complicated to make something reliable, due to the fact that in XToolkit
>>>>>> even native focus is asynchronous, like in CToolkit. So, from my point of
>>>>>> view hacking it doesn't worth the candles, taking into account all I've
>>>>>> said
>>>>>> below.
>>>>>>
>>>>>> I still like the idea of  replacing requestFocus with
>>>>>> requestFocusInWindow.
>>>>>> We can add comments saying that the original test was modified to avoid
>>>>>> the
>>>>>> endless loop. It didn't actually test for the endless loop, as otherwise
>>>>>> it
>>>>>> would have had a timer. It tested for focus gains count per window, that
>>>>>> remains relevant even in its new form. That's my personal opinion, I
>>>>>> won't
>>>>>> insist on keeping it if you both vote for the opposite...
>>>>> if you do not plan to fix the problem the test reproduces then I'd
>>>>> vote for removing the test (you have enough tests in regression tests
>>>>> suite ;)
>>>>
>>>> I'm afraid I don't, as I don't think this is a real problem :)
>>> so just remove the test, you do not need it ;)
>>
>> No, I won't. I'd leave it at Mikhail's discretion, as he is the author of the request.
>>
>> Mikhail, you have a chance to save the test :)
>>
>> Regards,
>> Anton.
>>
>>>
>>> Oleg.
>>>> Regards,
>>>> Anton.
>>>>
>>>>
>>>>> Regards, Oleg.
>>>>>> Thanks,
>>>>>> Anton.
>>>>>>
>>>>>>
>>>>>>> So, it is you whole should make the final decision but I just do not
>>>>>>> see a reason to keep a test which doesn't test for the particular
>>>>>>> regression in regression tests suite.
>>>>>>>
>>>>>>> Regards, Oleg.
>>>>>>>
>>>>>>> On Mon, Nov 17, 2014 at 2:01 PM, Anton V. Tarasov
>>>>>>> <anton.tarasov at oracle.com> wrote:
>>>>>>>> Hi Oleg,
>>>>>>>>
>>>>>>>> Glad to hear from you :)
>>>>>>>>
>>>>>>>> On 14.11.2014 18:24, Oleg Sukhodolsky wrote:
>>>>>>>>> Sorry to interrupt you but since I do know the test let me say that
>>>>>>>>> requestFocus() is an important part of the test,
>>>>>>>>> if you are going to replace it with requestFocusInWindow() you
>>>>>>>>> (effectively) remove it from list of regression tests.
>>>>>>>>> Please check 4369903 bug for more information.
>>>>>>>>
>>>>>>>> In the bug I see the description of why the infinite loop happens and a
>>>>>>>> suggestion to use requestFocusInWindow to prevent it. From my point of
>>>>>>>> view,
>>>>>>>> the behavior is expected taking into account the asynchronous nature of
>>>>>>>> java
>>>>>>>> focus.
>>>>>>>>
>>>>>>>> By default, when a toplevel window gains activation,
>>>>>>>> KeyboardFocusManager
>>>>>>>> requests focus in it by means of calling the requestFocusInWindow
>>>>>>>> method.
>>>>>>>> Thus, the test case creates an artificial situation with unclear goals.
>>>>>>>> It
>>>>>>>> doesn't seem to reflect a real use case (I can see the bug was filed by
>>>>>>>> an
>>>>>>>> internal engineer, not refering to any customer or user application).
>>>>>>>> In
>>>>>>>> case a developer wants to alter the component receiving focus at the
>>>>>>>> toplevels's activation, he/she can achieve this by means of customizing
>>>>>>>> the
>>>>>>>> focus traversal policy, for instance.
>>>>>>>>
>>>>>>>> The javadoc [1] for the JComponent.requestFocus method warns developers
>>>>>>>> about its usage:
>>>>>>>>
>>>>>>>> <<Note that the use of this method is discouraged because its behavior
>>>>>>>> is
>>>>>>>> platform dependent. Instead we recommend the use of
>>>>>>>> requestFocusInWindow().>>
>>>>>>>>
>>>>>>>> What I'm trying to say is that we'd rather avoid hacking the focus
>>>>>>>> subsystem
>>>>>>>> in order to just solve the infinite loop problem.
>>>>>>>>
>>>>>>>> However, originally the reporter of the bug complains about the
>>>>>>>> following:
>>>>>>>>
>>>>>>>> <<Setting focus during window activation often sends focus to the wrong
>>>>>>>> place, or to multiple places.>>
>>>>>>>>
>>>>>>>> This behavior is incorrect, indeed. But we can verify it with the test
>>>>>>>> case.
>>>>>>>> When a component gains focus we can check that:
>>>>>>>>
>>>>>>>> 1. it is the current focus owner reported by KFM, and the current
>>>>>>>> focused
>>>>>>>> window is its container
>>>>>>>> 2. the opposite component has lost focus prior to that (that is, every
>>>>>>>> FOCUS_GAINED precedes a FOCUS_LOST received by the opposite component)
>>>>>>>> 3. the same about the WINDOW_LOST_FOCUS/WINDOW_GAINED_FOCUS pair
>>>>>>>>
>>>>>>>> The test should be ready for the infinite loop and I can suggest to
>>>>>>>> simply
>>>>>>>> stop requesting focus after a couple of iterations.
>>>>>>>>
>>>>>>>> This could be a compromize. What do you think, Oleg, Mikhail?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Anton.
>>>>>>>>
>>>>>>>> [1]
>>>>>>>>
>>>>>>>>
>>>>>>>> https://docs.oracle.com/javase/8/docs/api/javax/swing/JComponent.html#requestFocus--
>>>>>>>>
>>>>>>>>
>>>>>>>>> Regards, Oleg.
>>>>>>>>>
>>>>>>>>> P.S. feel free to ask more questions if you need.
>>>>>>>>>
>>>>>>>>> On Fri, Nov 14, 2014 at 5:07 PM, Anton V. Tarasov
>>>>>>>>> <anton.tarasov at oracle.com> wrote:
>>>>>>>>>> Hi Mikhail,
>>>>>>>>>>
>>>>>>>>>> Looks fine for me. Thanks! This was an old one... Do you have any
>>>>>>>>>> plans
>>>>>>>>>> to
>>>>>>>>>> fix it in 8/9 as well?
>>>>>>>>>>
>>>>>>>>>> Regards,
>>>>>>>>>> Anton.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 14.11.2014 18:57, mikhail cherkasov wrote:
>>>>>>>>>>> Hello all,
>>>>>>>>>>>
>>>>>>>>>>> Could you please review a simple fix of closed test, the test was
>>>>>>>>>>> moved
>>>>>>>>>>> to
>>>>>>>>>>> open repo
>>>>>>>>>>> and requestFocus was replaced requestFocusInWindow.
>>>>>>>>>>>
>>>>>>>>>>> Because "requestFocus" cause infinite war for focus between two
>>>>>>>>>>> windows,
>>>>>>>>>>> however this
>>>>>>>>>>> behavior is correct and doesn't violet spec.
>>>>>>>>>>>
>>>>>>>>>>> [TESTBUG]
>>>>>>>>>>> closed/java/awt/Focus/ActivateFocusTest/ActivateFocusTest.html
>>>>>>>>>>> fails intermittently
>>>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8047961
>>>>>>>>>>> Webrev: http://cr.openjdk.java.net/~mcherkas/8047961/webrev.00/
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Mikhail.
>>>>>>>>>>>
>>
>



More information about the awt-dev mailing list