<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 Phil and Kevin,<div class=""><br class=""></div><div class="">Sorry for specifying wrong JDK version. This review is for JDK 11.</div><div class=""><br class=""></div><div class="">Regarding spec:</div><div class="">   <span class="Apple-tab-span" style="white-space:pre">    </span>It is not mentioned in the focus spec whether the unfocusable maximized frames should be resizable or not.</div><div class="">   <span class="Apple-tab-span" style="white-space:pre">   </span>The fix is given based on the implementation on other platforms (windows and linux/unix) . On both the platforms (windows and unix/linux) the test passes whereas the test fails on Mac.</div><div class=""><span class="Apple-tab-span" style="white-space:pre">  </span>An old Issue <a href="https://bugs.openjdk.java.net/browse/JDK-4980161" class="">https://bugs.openjdk.java.net/browse/JDK-4980161</a> is related to this fix.</div><div class="">  </div><div class="">Regarding fix:</div><div class=""><span class="Apple-tab-span" style="white-space:pre">        </span>The test case <span style="font-family: Menlo; font-size: 11px; background-color: rgb(255, 255, 255);" class="">java/awt/Frame/UnfocusableMaximizedFrameResizablity/UnfocusableMaximizedFrameResizablity.java fails on Mac and passes on windows and linux/unix.</span></div><div class=""><span class="Apple-tab-span" style="white-space:pre">      </span>As per the test case when <span style="font-family: 'Helvetica Neue'; font-size: 13px;" class="">setFocusableWindowState is set to false frame should not be resizable. </span></div><div class=""><span style="font-family: 'Helvetica Neue'; font-size: 13px;" class=""><span class="Apple-tab-span" style="white-space:pre">        </span></span><span style="font-family: Menlo; font-size: 11px; background-color: rgb(255, 255, 255);" class="">updateFocusableWindowState() updates the window focusable state</span><span style="font-family: Menlo; font-size: 11px; background-color: rgb(255, 255, 255);" class=""> and the fix under review sets </span><span style="font-family: Menlo; font-size: 11px; background-color: rgb(255, 255, 255);" class="">RESIZABLE property to </span><span style="font-family: 'Helvetica Neue'; font-size: 13px;" class="">true or false based on focusability of the frame</span><span style="font-family: 'Helvetica Neue'; font-size: 13px;" class="">.</span></div><div class=""><span class="Apple-tab-span" style="white-space:pre">  </span></div><div class="">Regards,</div><div class="">Manajit</div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""><div><blockquote type="cite" class=""><div class="">
  
    <meta http-equiv="Content-Type" content="text/html;
      charset=windows-1252" class="">
  
  <div text="#000000" bgcolor="#FFFFFF" class="">
    webrev appears to be against the correct repo :<br class="">
    <a href="sh://hg.openjdk.java.net/jdk/client" class="">sh://hg.openjdk.java.net/jdk/client</a>
    <br class="">
    <br class="">
    <br class="">
    First can you point to what in the spec. or whatever points to<br class="">
    the correct desired behaviour being what you describe :<br class="">
    >If the frame is focusable then it is
    resizable otherwise not.<br class="">
    <br class="">
    And regarding the fix, it isn't obvious how adding RESIZABLE changes
    anything here<br class="">
    for the unfocusable case which seems to be the situation that is
    under discussion.<br class="">
    <br class="">
    Before: if its unfocusable, then setStyleBits will set the mask to 0
    :<br class="">
    <br class="">
        // this is the counter-point to -[CWindow _nativeSetStyleBit:]<br class="">
        private void setStyleBits(final int mask, final boolean value) {<br class="">
            execute(ptr -> nativeSetNSWindowStyleBits(ptr, mask,
    value ? mask : 0));<br class="">
        }<br class="">
    <br class="">
    After: .. well exactly the same thing .. you just added a RESIZABLE
    bit that will be ignored.<br class="">
    <br class="">
    It may make a difference in the isFocusable case .. but that does
    not match what you<br class="">
    say you are doing.<br class="">
    <br class="">
    -phil.<br class="">
    <br class="">
    <div class="moz-cite-prefix">On 02/16/2018 08:17 AM, Kevin Rushforth
      wrote:<br class="">
    </div>
    <blockquote type="cite" cite="mid:5A870416.1030009@oracle.com" class="">
      <meta content="text/html; charset=windows-1252" http-equiv="Content-Type" class="">
      <br class="">
      > Kindly review the fix for JDK10.<br class="">
      <br class="">
      You mean JDK 11, right?<br class="">
      <br class="">
      -- Kevin<br class="">
      <br class="">
      <br class="">
      Manajit Halder wrote:
      <blockquote cite="mid:D9D0E2F1-34F0-4ACC-B2A4-A8C945D519A8@oracle.com" type="cite" class="">
        <meta http-equiv="Content-Type" content="text/html;
          charset=windows-1252" class="">
        <div class="">
          <div style="margin: 0px; line-height: normal; font-family:
            'Helvetica Neue'; color: rgb(69, 69, 69);" class="">Hi All,</div>
          <div style="margin: 0px; line-height: normal; font-family:
            'Helvetica Neue'; color: rgb(69, 69, 69); min-height: 14px;" class=""><br class="">
          </div>
          <div style="margin: 0px; line-height: normal; font-family:
            'Helvetica Neue'; color: rgb(69, 69, 69);" class="">Kindly
            review the fix for JDK10.</div>
          <div style="margin: 0px; line-height: normal; font-family:
            'Helvetica Neue'; color: rgb(69, 69, 69); min-height: 14px;" class=""><br class="">
          </div>
          <div style="margin: 0px; line-height: normal; font-family:
            'Helvetica Neue'; color: rgb(69, 69, 69);" class="">Bug: </div>
        </div>
        <div style="margin: 0px; line-height: normal;" class=""><font class="" face="Helvetica Neue" color="#454545"><a moz-do-not-send="true" href="https://bugs.openjdk.java.net/browse/JDK-7158623" class="">https://bugs.openjdk.java.net/browse/JDK-7158623</a></font></div>
        <div style="margin: 0px; line-height: normal;" class=""><font class="" face="Helvetica Neue" color="#454545"><br class="">
          </font></div>
        <div class="">Webrev:</div>
        <a moz-do-not-send="true" href="http://cr.openjdk.java.net/%7Emhalder/7158623/webrev.00/" class="">http://cr.openjdk.java.net/~mhalder/7158623/webrev.00/</a>
        <div class=""><br class="">
        </div>
        <div class="">
          <div style="margin: 0px; line-height: normal; font-family:
            'Helvetica Neue'; color: rgb(69, 69, 69);" class="">Issue:</div>
        </div>
        <div style="margin: 0px; line-height: normal; font-family:
          'Helvetica Neue'; color: rgb(69, 69, 69);" class="">Unfocusable
          frame was resizable.</div>
        <div style="margin: 0px; line-height: normal; font-family:
          'Helvetica Neue'; color: rgb(69, 69, 69);" class=""><br class="">
        </div>
        <div style="margin: 0px; line-height: normal; font-family:
          'Helvetica Neue'; color: rgb(69, 69, 69);" class="">Fix:</div>
        <div style="margin: 0px; line-height: normal; font-family:
          'Helvetica Neue'; color: rgb(69, 69, 69);" class="">After the
          fix resizability of the frame depends on
          focusability of the frame. If the frame is focusable then it
          is
          resizable otherwise not.</div>
        <div style="margin: 0px; line-height: normal; font-family:
          'Helvetica Neue'; color: rgb(69, 69, 69);" class="">The test
          code was cleaned to remove old test machinery code.</div>
        <div style="margin: 0px; line-height: normal; font-family:
          'Helvetica Neue'; color: rgb(69, 69, 69);" class=""><br class="">
        </div>
        <div style="margin: 0px; line-height: normal; font-family:
          'Helvetica Neue'; color: rgb(69, 69, 69);" class="">Regards,</div>
        <div style="margin: 0px; line-height: normal; font-family:
          'Helvetica Neue'; color: rgb(69, 69, 69);" class="">Manajit</div>
        <div style="margin: 0px; line-height: normal; font-family:
          'Helvetica Neue'; color: rgb(69, 69, 69);" class=""><br class="">
        </div>
      </blockquote>
    </blockquote>
    <br class="">
  </div>

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