<html><head><style type='text/css'>p { margin: 0; }</style></head><body><div style='font-family: Times New Roman; font-size: 12pt; color: #000000'>Looks good.  Approved.<div><br></div><div>Jennifer<br>----- Original Message -----<br>From: prasanta.sadhukhan@oracle.com<br>To: philip.race@oracle.com<br>Cc: 2d-dev@openjdk.java.net, rajeev.chamyal@oracle.com<br>Sent: Friday, November 13, 2015 3:36:15 AM GMT -08:00 US/Canada Pacific<br>Subject: Re: [OpenJDK 2D-Dev] [9] RFR JDK-8067059, , PrinterJob.pageDialog() with DialogSelectionType.NATIVE returns a PageFormat when cancelled.<br><br>
  
    
  
  <div>
    Thanks Phil. I was indeed relying on non-presence of equals method
    in PageFormat.<br>
    <br>
    I have modified the webrev to use equality. Can you please review ?<br>
    <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~psadhukhan/8067059/webrev.01/" target="_blank">http://cr.openjdk.java.net/~psadhukhan/8067059/webrev.01/</a><br>
    <br>
    Regards<br>
    Prasanta<br>
    <div class="moz-cite-prefix">On 11/13/2015 11:35 AM, Philip Race
      wrote:<br>
    </div>
    <blockquote cite="mid:56457DBB.50803@oracle.com">
      
      <br>
      <br>
      On 11/12/15, 9:52 PM, prasanta sadhukhan wrote:
      <blockquote cite="mid:56457A8D.9010104@oracle.com">
        
        Hi Phil,<br>
        <br>
        <div class="moz-cite-prefix">On 11/13/2015 12:05 AM, Philip Race
          wrote:<br>
        </div>
        <blockquote cite="mid:5644DBE6.10303@oracle.com">
          
          Shouldn't this use reference equality ?<br>
        </blockquote>
        It seems Object.java equals() method does == anyways and it
        seems it always better to call equals() than to explicitly check
        for ==<br>
        Is it not correct? If it is not, I will change it.<br>
      </blockquote>
      <br>
      You are relying on PageFormat not over-riding equals.<br>
      It could do so and then the intent of your equals call would be
      broken<br>
      in the case that updates to the dialog resulted in a new
      PageFormat<br>
      but with the equal fieids.<br>
      <br>
      -phil.<br>
      <br>
      <blockquote cite="mid:56457A8D.9010104@oracle.com">
        <blockquote cite="mid:5644DBE6.10303@oracle.com">
          Also the test does a number of wild card imports instead of<br>
          the specific classes it needs. And I am not sure why it<br>
          is importing java.awt.* anyway ..<br>
          <br>
          And I find qualifying "java.lang.String" to be unnecessary.<br>
        </blockquote>
        will update the test.<br>
        <br>
        Regards<br>
        Prasanta<br>
        <blockquote cite="mid:5644DBE6.10303@oracle.com"> <br>
          -phil.<br>
          <br>
          On 11/3/15, 12:59 AM, prasanta sadhukhan wrote:
          <blockquote cite="mid:56387767.7060101@oracle.com">
            
            Hi All,<br>
            <br>
            Please review a fix for jdk9.<br>
            <br>
            Bug: <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8067059" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8067059</a><br>
            webrev: <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/%7Epsadhukhan/8067059/webrev.00/" target="_blank">http://cr.openjdk.java.net/~psadhukhan/8067059/webrev.00/</a><br>
            <br>
            The issue was if we "cancel" Printer Job setup dialog which
            has been created with DialogSelectionType.NATIVE <br>
            it should return "null" as per spec <br>
            <a class="moz-txt-link-freetext" href="https://docs.oracle.com/javase/8/docs/api/java/awt/print/PrinterJob.html#pageDialog-javax.print.attribute.PrintRequestAttributeSet" target="_blank">https://docs.oracle.com/javase/8/docs/api/java/awt/print/PrinterJob.html#pageDialog-javax.print.attribute.PrintRequestAttributeSet</a>-<br>
            <i>[If the user cancels the dialog, the attributes will not
              reflect any changes made by the user, and the return value
              will be null.]<br>
              <br>
            </i>We were returning PageFormat instead. Fixed to return
            null in such case. <br>
            <br>
            Regards<br>
            Prasanta<br>
          </blockquote>
        </blockquote>
        <br>
      </blockquote>
    </blockquote>
    <br>
  </div>

</div></div></body></html>