<AWT Dev> [10] Review request for 8155197: Focus transition issue

Sergey Bylokhov Sergey.Bylokhov at oracle.com
Wed Oct 4 18:27:22 UTC 2017

On 10/4/17 03:25, Dmitry Markov wrote:
> Good catch!
> Actually we check restoreFocusTo before its usage, (i.e. we compare 
> restoreFocusTo with most resent focus owner for the window; most recent 
> focus owner is visible and focusable, otherwise it is removed from 
> recent focus owners map).
> So the checks before assignment are not really necessary. I updated the 
> fix (reverted it to the previous version). Please find webrev here: 
> http://cr.openjdk.java.net/~dmarkov/8155197/webrev.03/

isn't the "restoreFocusTo = toFocus;" also redundant?
If we tried to restore the focus synchronously in Window1 (Actually we 
tried to post FocusEvent.Cause.ROLLBACK) and we found that the native 
window was changed to Window2 then why we should post ROLLBACK then the 
focus will be moved back to Window1? I assume that we never post 
ROLLBACK to the Window1 because when the Window2 will got the focus it 
will reset restoreFocusTo.
It is up to you to check it, it looks fine to me as-is.

> Thanks,
> Dmitry
>> On 3 Oct 2017, at 18:30, Sergey Bylokhov <sergey.bylokhov at oracle.com 
>> <mailto:sergey.bylokhov at oracle.com>> wrote:
>> Hi, Dmitry.
>> If this check is valid then probably the same check should be added to 
>> restoreFocus()? before:
>> #173 restoreFocusTo = toFocus;
>> But as it was mentioned before, the component can become 
>> non-visible/non-focusable at any point, for example after this check 
>> but before the assignment. And instead of data validation before 
>> assignment we should check them before use. As far as I understand we 
>> already do this, all usages of restoreFocusTo are a checks == or != to 
>> some other component which is visible and focusable, isn't it?
>> On 10/3/17 08:23, Dmitry Markov wrote:
>>> Hi Semyon,
>>> I have updated the fix based on your suggestion. The new version is 
>>> located athttp://cr.openjdk.java.net/~dmarkov/8155197/webrev.02/
>>> Also I slightly modified the test to simplify it.
>>> Thanks,
>>> Dmitry
>>>> On 2 Oct 2017, at 18:32, Semyon Sadetsky <semyon.sadetsky at oracle.com 
>>>> <mailto:semyon.sadetsky at oracle.com><mailto:semyon.sadetsky at oracle.com>> 
>>>> wrote:
>>>> Hi Dmitry,
>>>>>> Actually the parent frame doesn't own the input focus when the 
>>>>>> issue happens. The focus is on the dialog already and when 
>>>>>> FOCUS_GAINED event comes for the dialog the KFM discovers that the 
>>>>>> dialog should not have the focus and rejects it in line 588 of the 
>>>>>> DefaultKeyboardFocusManager:
>>>>>> restoreFocus(fe, newFocusedWindow);
>>>>>> This happens when the dialog became non-focusable (non-visible) 
>>>>>> after the focus request is sent, but before the corresponding 
>>>>>> FOCUS_GAINED event is handled on the EDT. In this case the focus 
>>>>>> is directly restored to the previously focused window which 
>>>>>> doesn't have the focus at this moment and input focus cannot be 
>>>>>> requested to one of its components synchronously.
>>>>>> Please confirm whether you agree on the root cause of the bug.
>>>>> You are right. I agree with your evaluation.
>>>> Thanks.
>>>> Before setting the restoreFocusTo to toFocus in line 190 I would 
>>>> recheck for toFocus.isShowing() && toFocus.canBeFocusOwner() once 
>>>> again because the component can be made non-focusable concurrently.
>>>> --Semyon
>> --
>> Best regards, Sergey.

Best regards, Sergey.

More information about the awt-dev mailing list