Hi Dmitry,
Looks fine to me too.
Regards,
Alexey
On 05/10/2017 15:27, Dmitry Markov wrote:
Hi Sergey,
On 4 Oct 2017, at 19:27, Sergey Bylokhov <sergey.bylok...@oracle.com
<mailto:sergey.bylok...@oracle.com>> wrote:
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/
<http://cr.openjdk.java.net/%7Edmarkov/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.
Most likely you are right. However if you do not mind I would prefer
to keep it as is.
Thanks,
Dmitry
Thanks,
Dmitry
On 3 Oct 2017, at 18:30, Sergey Bylokhov
<sergey.bylok...@oracle.com
<mailto:sergey.bylok...@oracle.com><mailto: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 locatedathttp://cr.openjdk.java.net/~dmarkov/8155197/webrev.02/
<athttp://cr.openjdk.java.net/%7Edmarkov/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.
--
Best regards, Sergey.
