<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi Semyon,<div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On 5 Oct 2017, at 17:13, Semyon Sadetsky <<a href="mailto:semyon.sadetsky@oracle.com" class="">semyon.sadetsky@oracle.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class="">
  
    <meta http-equiv="Content-Type" content="text/html;
      charset=windows-1252" class="">
  
  <div text="#000000" bgcolor="#FFFFFF" class=""><p class="">Hi Dmitriy,</p>
    On 10/05/2017 07:25 AM, Dmitry Markov wrote:<br class="">
    <blockquote type="cite" cite="mid:3E6523C0-F62D-49C1-A543-02C066BAE3CE@oracle.com" class="">
      <meta http-equiv="Content-Type" content="text/html;
        charset=windows-1252" class="">
      Hi Semyon,
      <div class=""><br class="">
      </div>
      <div class="">Actually the most recent focus owner is updated
        during invocation of setEnabled(), setFocusable() and
        setVisible(). Please find the details below:</div>
      <div class=""> - Component.setEnabled(false), (i.e. disable a
        component) - setEnabled(false) ->
        Component.this.setEnabled(false) -> enable(false) ->
        disable() -> KeyboardFocusManager.clearMostRecentFocusOwner()</div>
      <div class=""> - Component.setFocusable(false), (i.e. make a
        component non-focusable) - setFocusable(false) ->
        transferFocus()
        and KeyboardFocusManager.clearMostRecentFocusOwner()</div>
      <div class=""> - Component.setVisible(false), (i.e. make a
        component invisible) - setVisible(false)
        -> Component.this.setVisible(false) -> show(false) ->
        hide() -> clearMostRecentFocusOwnerOnHide() and
        transferFocus()</div>
      <div class=""><br class="">
      </div>
      <div class="">So the most recent focus owner is always up to date.
        I am sorry, but looks like it is not necessary to perform checks
        such as isShowing() and canBeFocusOwner() before we set some
        value to restoreFocusTo. These checks are implicitly performed
        before usage of restoreFocusTo and I believe that is enough even
        for concurrent execution. What do you think?</div>
    </blockquote><p class="">Yes, this makes sense. I'm only not sure about the case when a
      parent component is made invisible and the most recent focus owner
      is cleared for the parent component but not for its child
      components. So all the child components will start to return
      isShowing() false (e.g. non-focusable) but any of them still may
      be set as most recent focus owner. This makes the isShowing()
      check mandatory for component returned by
      getMostRecentFocusOwner(). <br class=""></p></div></div></blockquote>If a parent component, (i.e. container) is made invisible, the most recent focus owner is cleared for its child components too, see implementation of clearMostRecentFocusOwnerOnHide() in Container class for details. So I do not think that it is really necessary to invoke isShowing() for the component returned by getMostRecentFocusOwner().</div><div><br class=""></div><div>Thanks,</div><div>Dmitry<br class=""><blockquote type="cite" class=""><div class=""><div text="#000000" bgcolor="#FFFFFF" class=""><p class="">
    </p>
    --Semyon<br class="">
    <blockquote type="cite" cite="mid:3E6523C0-F62D-49C1-A543-02C066BAE3CE@oracle.com" class="">
      <div class=""><br class="">
      </div>
      <div class="">Thanks,</div>
      <div class="">Dmitry <br class="">
        <div class="">
          <blockquote type="cite" class="">
            <div class="">On 4 Oct 2017, at 17:23, Semyon Sadetsky <<a href="mailto:semyon.sadetsky@oracle.com" class="" moz-do-not-send="true">semyon.sadetsky@oracle.com</a>>
              wrote:</div>
            <br class="Apple-interchange-newline">
            <div class="">
              <meta http-equiv="Content-Type" content="text/html;
                charset=windows-1252" class="">
              <div text="#000000" bgcolor="#FFFFFF" class=""><p class="">Hi Dmitry,</p><p class="">If you make a detailed look on the Component
                  class methods like setEnables(), setFocusable() and
                  setVisible() the most recent focus owner is cleared or
                  transferred only after the component is made
                  non-focusable. Since the fix is aimed for concurrent
                  scenarios the recheck actually makes sens.  <br class="">
                </p><p class="">Also it would be more correct always trying
                  to determine the first focusable component in
                  traversal order before before the attempt to set focus
                  owner as it is done in restoreFocus(Window aWindow,
                  Component  boolean clearOnFailure):</p><p class="">                if (!toFocus.isShowing() ||
                  !toFocus.canBeFocusOwner()) {<br class="">
                                      toFocus =
                  toFocus.getNextFocusCandidate();<br class="">
                                  }<br class="">
                  otherwise when focus is restored to actually
                  non-focusable component the final focus owner may
                  become null while the next component in container
                  traversal order is expected to have the input focus.<br class="">
                </p><p class="">--Semyon<br class="">
                </p><p class=""><br class="">
                </p>
                <div class="moz-cite-prefix">On 10/04/2017 03:25 AM,
                  Dmitry Markov wrote:<br class="">
                </div>
                <blockquote type="cite" cite="mid:1D8D16A2-6C97-48D9-9C1B-6A132F4DFA19@oracle.com" class="">
                  <meta http-equiv="Content-Type" content="text/html;
                    charset=windows-1252" class="">
                  Hi Sergey,
                  <div class=""><br class="">
                  </div>
                  <div class="">Good catch!</div>
                  <div class="">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).</div>
                  <div class="">So the checks before assignment are not
                    really necessary. I updated the fix (reverted it to
                    the previous version). Please find webrev here: <a href="http://cr.openjdk.java.net/%7Edmarkov/8155197/webrev.03/" class="" moz-do-not-send="true">http://cr.openjdk.java.net/~dmarkov/8155197/webrev.03/</a></div>
                  <div class=""><br class="">
                  </div>
                  <div class="">Thanks,</div>
                  <div class="">Dmitry<br class="">
                    <div class="">
                      <blockquote type="cite" class="">
                        <div class="">On 3 Oct 2017, at 18:30, Sergey
                          Bylokhov <<a href="mailto:sergey.bylokhov@oracle.com" class="" moz-do-not-send="true">sergey.bylokhov@oracle.com</a>>
                          wrote:</div>
                        <br class="Apple-interchange-newline">
                        <div class=""><span style="font-family:
                            Helvetica; font-size: 12px; font-style:
                            normal; font-variant-caps: normal;
                            font-weight: normal; letter-spacing: normal;
                            text-align: start; text-indent: 0px;
                            text-transform: none; white-space: normal;
                            word-spacing: 0px;
                            -webkit-text-stroke-width: 0px; float: none;
                            display: inline !important;" class="">Hi,
                            Dmitry.</span><br style="font-family:
                            Helvetica; font-size: 12px; font-style:
                            normal; font-variant-caps: normal;
                            font-weight: normal; letter-spacing: normal;
                            text-align: start; text-indent: 0px;
                            text-transform: none; white-space: normal;
                            word-spacing: 0px;
                            -webkit-text-stroke-width: 0px;" class="">
                          <span style="font-family: Helvetica;
                            font-size: 12px; font-style: normal;
                            font-variant-caps: normal; font-weight:
                            normal; letter-spacing: normal; text-align:
                            start; text-indent: 0px; text-transform:
                            none; white-space: normal; word-spacing:
                            0px; -webkit-text-stroke-width: 0px; float:
                            none; display: inline !important;" class="">If
                            this check is valid then probably the same
                            check should be added to restoreFocus()?
                            before:</span><br style="font-family:
                            Helvetica; font-size: 12px; font-style:
                            normal; font-variant-caps: normal;
                            font-weight: normal; letter-spacing: normal;
                            text-align: start; text-indent: 0px;
                            text-transform: none; white-space: normal;
                            word-spacing: 0px;
                            -webkit-text-stroke-width: 0px;" class="">
                          <span style="font-family: Helvetica;
                            font-size: 12px; font-style: normal;
                            font-variant-caps: normal; font-weight:
                            normal; letter-spacing: normal; text-align:
                            start; text-indent: 0px; text-transform:
                            none; white-space: normal; word-spacing:
                            0px; -webkit-text-stroke-width: 0px; float:
                            none; display: inline !important;" class="">#173
                            restoreFocusTo = toFocus;</span><br style="font-family: Helvetica; font-size:
                            12px; font-style: normal; font-variant-caps:
                            normal; font-weight: normal; letter-spacing:
                            normal; text-align: start; text-indent: 0px;
                            text-transform: none; white-space: normal;
                            word-spacing: 0px;
                            -webkit-text-stroke-width: 0px;" class="">
                          <br style="font-family: Helvetica; font-size:
                            12px; font-style: normal; font-variant-caps:
                            normal; font-weight: normal; letter-spacing:
                            normal; text-align: start; text-indent: 0px;
                            text-transform: none; white-space: normal;
                            word-spacing: 0px;
                            -webkit-text-stroke-width: 0px;" class="">
                          <span style="font-family: Helvetica;
                            font-size: 12px; font-style: normal;
                            font-variant-caps: normal; font-weight:
                            normal; letter-spacing: normal; text-align:
                            start; text-indent: 0px; text-transform:
                            none; white-space: normal; word-spacing:
                            0px; -webkit-text-stroke-width: 0px; float:
                            none; display: inline !important;" class="">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?</span><br style="font-family: Helvetica; font-size:
                            12px; font-style: normal; font-variant-caps:
                            normal; font-weight: normal; letter-spacing:
                            normal; text-align: start; text-indent: 0px;
                            text-transform: none; white-space: normal;
                            word-spacing: 0px;
                            -webkit-text-stroke-width: 0px;" class="">
                          <br style="font-family: Helvetica; font-size:
                            12px; font-style: normal; font-variant-caps:
                            normal; font-weight: normal; letter-spacing:
                            normal; text-align: start; text-indent: 0px;
                            text-transform: none; white-space: normal;
                            word-spacing: 0px;
                            -webkit-text-stroke-width: 0px;" class="">
                          <span style="font-family: Helvetica;
                            font-size: 12px; font-style: normal;
                            font-variant-caps: normal; font-weight:
                            normal; letter-spacing: normal; text-align:
                            start; text-indent: 0px; text-transform:
                            none; white-space: normal; word-spacing:
                            0px; -webkit-text-stroke-width: 0px; float:
                            none; display: inline !important;" class="">On
                            10/3/17 08:23, Dmitry Markov wrote:</span><br style="font-family: Helvetica; font-size:
                            12px; font-style: normal; font-variant-caps:
                            normal; font-weight: normal; letter-spacing:
                            normal; text-align: start; text-indent: 0px;
                            text-transform: none; white-space: normal;
                            word-spacing: 0px;
                            -webkit-text-stroke-width: 0px;" class="">
                          <blockquote type="cite" style="font-family:
                            Helvetica; font-size: 12px; font-style:
                            normal; font-variant-caps: normal;
                            font-weight: normal; letter-spacing: normal;
                            orphans: auto; text-align: start;
                            text-indent: 0px; text-transform: none;
                            white-space: normal; widows: auto;
                            word-spacing: 0px; -webkit-text-size-adjust:
                            auto; -webkit-text-stroke-width: 0px;" class="">Hi Semyon,<br class="">
                            I have updated the fix based on your
                            suggestion. The new version is located at<span class="Apple-converted-space"> </span><a href="http://cr.openjdk.java.net/%7Edmarkov/8155197/webrev.02/" class="" moz-do-not-send="true">http://cr.openjdk.java.net/~dmarkov/8155197/webrev.02/</a><br class="">
                            Also I slightly modified the test to
                            simplify it.<br class="">
                            Thanks,<br class="">
                            Dmitry<br class="">
                            <blockquote type="cite" class="">On 2 Oct
                              2017, at 18:32, Semyon Sadetsky <<a href="mailto:semyon.sadetsky@oracle.com" class="" moz-do-not-send="true">semyon.sadetsky@oracle.com</a><span class="Apple-converted-space"> </span><<a href="mailto:semyon.sadetsky@oracle.com" class="" moz-do-not-send="true">mailto:semyon.sadetsky@oracle.com</a>>>
                              wrote:<br class="">
                              <br class="">
                              Hi Dmitry,<br class="">
                              <br class="">
                              <blockquote type="cite" class="">
                                <blockquote type="cite" class="">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:<br class="">
                                  <br class="">
                                                             <span class="Apple-converted-space"> </span>restoreFocus(fe,
                                  newFocusedWindow);<br class="">
                                  <br class="">
                                  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.<br class="">
                                  Please confirm whether you agree on
                                  the root cause of the bug.<br class="">
                                  <br class="">
                                </blockquote>
                                You are right. I agree with your
                                evaluation.<br class="">
                              </blockquote>
                              Thanks.<br class="">
                              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.<br class="">
                              <br class="">
                              --Semyon<br class="">
                              <br class="">
                              <br class="">
                            </blockquote>
                          </blockquote>
                          <br style="font-family: Helvetica; font-size:
                            12px; font-style: normal; font-variant-caps:
                            normal; font-weight: normal; letter-spacing:
                            normal; text-align: start; text-indent: 0px;
                            text-transform: none; white-space: normal;
                            word-spacing: 0px;
                            -webkit-text-stroke-width: 0px;" class="">
                          <br style="font-family: Helvetica; font-size:
                            12px; font-style: normal; font-variant-caps:
                            normal; font-weight: normal; letter-spacing:
                            normal; text-align: start; text-indent: 0px;
                            text-transform: none; white-space: normal;
                            word-spacing: 0px;
                            -webkit-text-stroke-width: 0px;" class="">
                          <span style="font-family: Helvetica;
                            font-size: 12px; font-style: normal;
                            font-variant-caps: normal; font-weight:
                            normal; letter-spacing: normal; text-align:
                            start; text-indent: 0px; text-transform:
                            none; white-space: normal; word-spacing:
                            0px; -webkit-text-stroke-width: 0px; float:
                            none; display: inline !important;" class="">--<span class="Apple-converted-space"> </span></span><br style="font-family: Helvetica; font-size:
                            12px; font-style: normal; font-variant-caps:
                            normal; font-weight: normal; letter-spacing:
                            normal; text-align: start; text-indent: 0px;
                            text-transform: none; white-space: normal;
                            word-spacing: 0px;
                            -webkit-text-stroke-width: 0px;" class="">
                          <span style="font-family: Helvetica;
                            font-size: 12px; font-style: normal;
                            font-variant-caps: normal; font-weight:
                            normal; letter-spacing: normal; text-align:
                            start; text-indent: 0px; text-transform:
                            none; white-space: normal; word-spacing:
                            0px; -webkit-text-stroke-width: 0px; float:
                            none; display: inline !important;" class="">Best
                            regards, Sergey.</span></div>
                      </blockquote>
                    </div>
                    <br class="">
                  </div>
                </blockquote>
                <br class="">
              </div>
            </div>
          </blockquote>
        </div>
        <br class="">
      </div>
    </blockquote>
    <br class="">
  </div>

</div></blockquote></div><br class=""></div></body></html>