On Mon, Nov 17, 2014 at 5:31 PM, Anton V. Tarasov <anton.tara...@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.tara...@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 ;) 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.tara...@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.tara...@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. >>>>>>>> >
