[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-awt-dev
Subject:    Re: <AWT Dev> [10] Review Request 8190230: [macosx] Order of overlapping of modal dialogs is wrong
From:       Dmitry Markov <dmitry.markov () oracle ! com>
Date:       2017-11-01 16:54:36
Message-ID: EC011224-4552-468E-8A94-99B91702D038 () oracle ! com
[Download RAW message or body]

Hi Semyon,

The fix looks good to me.
I have only a minor remark regarding the test. It might be reasonable to use \
Util.testPixelColor() from regtesthelpers package instead of Robot.getPixelColor(). \
However it is up to you and you may keep it as is.

Thanks,
Dmitry 
> On 31 Oct 2017, at 15:05, Semyon Sadetsky <semyon.sadetsky@oracle.com> wrote:
> 
> Thank you, Alexander. I have corrected the webrev.
> 
> --Semyon
> 
> On 10/31/2017 02:25 AM, Alexander Zvegintsev wrote:
> > Hi Semyon,
> > 
> > the fix looks good to me,  but the test link is broken in this webrev.
> > Thanks,
> > Alexander.
> > On 30/10/2017 20:37, Semyon Sadetsky wrote:
> > > 
> > > Hello, 
> > > 
> > > Please review fix for JDK10: 
> > > 
> > > bug: https://bugs.openjdk.java.net/browse/JDK-8190230 \
> > > <https://bugs.openjdk.java.net/browse/JDK-8190230>  
> > > webrev: http://cr.openjdk.java.net/~ssadetsky/8190230/webrev.00/ \
> > > <http://cr.openjdk.java.net/%7Essadetsky/8190230/webrev.00/>  
> > > After the 8169589 fix siblings of the same parent are overlapped in the order \
> > > they appear in  the array of child windows. But this order do not preserve the \
> > > previous overlapping order in which the sibling windows have been shown before \
> > > the order update. That is the cause of unexpected reorder.  
> > > In the fix each platform window is attributed with a timestamp of the moment \
> > > when it was show in front last time. Then this timestamp is used to sort the \
> > > sibling windows array before putting them into an order to each other.  
> > > --Semyon 
> > > 
> > 
> 


[Attachment #3 (unknown)]

<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><div class="">The fix looks good to me.</div><div class="">I have only \
a minor remark regarding the test. It might be reasonable to use \
Util.testPixelColor() from regtesthelpers package instead of Robot.getPixelColor(). \
However it is up to you and you may keep it as is.</div><div class=""><br \
class=""></div><div class="">Thanks,</div><div class="">Dmitry&nbsp;<br \
class=""><div><blockquote type="cite" class=""><div class="">On 31 Oct 2017, at \
15:05, Semyon Sadetsky &lt;<a href="mailto:semyon.sadetsky@oracle.com" \
class="">semyon.sadetsky@oracle.com</a>&gt; wrote:</div><br \
class="Apple-interchange-newline"><div class="">  
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8" class="">
  
  <div text="#000000" bgcolor="#FFFFFF" class=""><p class="">Thank you, Alexander. I \
have corrected the webrev.</p><p class="">--Semyon<br class="">  </p>
    <br class="">
    <div class="moz-cite-prefix">On 10/31/2017 02:25 AM, Alexander
      Zvegintsev wrote:<br class="">
    </div>
    <blockquote type="cite" \
cite="mid:2a26042f-bc6f-8928-3650-df1ef8c5a9bb@oracle.com" class="">  <meta \
http-equiv="Content-Type" content="text/html; charset=utf-8" class=""><p class="">Hi \
Semyon,</p><p class="">the fix looks good to me,&nbsp; but the test link is broken in \
this  webrev.<br class="">
      </p>
      <pre class="moz-signature" cols="72">Thanks,
Alexander.</pre>
      <div class="moz-cite-prefix">On 30/10/2017 20:37, Semyon Sadetsky
        wrote:<br class="">
      </div>
      <blockquote type="cite" \
cite="mid:a13dc787-9229-f08d-4237-870b2241792b@oracle.com" class="">  <meta \
                http-equiv="content-type" content="text/html;
          charset=utf-8" class=""><div class=""> <br \
class="webkit-block-placeholder"></div>  <div class="moz-text-flowed" \
style="font-family: -moz-fixed;  font-size: 12px;" lang="x-unicode">Hello, <br \
class="">  <br class="">
          Please review fix for JDK10: <br class="">
          <br class="">
          bug: <a class="moz-txt-link-freetext" \
href="https://bugs.openjdk.java.net/browse/JDK-8190230" \
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8190230</a>  <br \
class="">  <br class="">
          webrev: <a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/%7Essadetsky/8190230/webrev.00/" \
moz-do-not-send="true">http://cr.openjdk.java.net/~ssadetsky/8190230/webrev.00/</a>  \
<br class="">  <br class="">
          After the 8169589 fix siblings of the same parent are
          overlapped in the order they appear in&nbsp; the array of child
          windows. But this order do not preserve the previous
          overlapping order in which the sibling windows have been shown
          before the order update. That is the cause of unexpected
          reorder. <br class="">
          <br class="">
          In the fix each platform window is attributed with a timestamp
          of the moment when it was show in front last time. Then this
          timestamp is used to sort the sibling windows array before
          putting them into an order to each other. <br class="">
          <br class="">
          --Semyon <br class="">
          <br class="">
        </div>
      </blockquote>
      <br class="">
    </blockquote>
    <br class="">
  </div>

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



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic