Hi Sergey, 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/ <http://cr.openjdk.java.net/~dmarkov/8155197/webrev.03/>
Thanks, Dmitry > On 3 Oct 2017, at 18:30, Sergey Bylokhov <sergey.bylok...@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 >> at http://cr.openjdk.java.net/~dmarkov/8155197/webrev.02/ >> <http://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.sadet...@oracle.com >>> <mailto:semyon.sadet...@oracle.com> <mailto:semyon.sadet...@oracle.com >>> <mailto:semyon.sadet...@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.
