<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type content="text/html; charset=utf-8"><meta name=Generator content="Microsoft Word 15 (filtered medium)"><style><!--
/* Font Definitions */
@font-face
        {font-family:"Cambria Math";
        panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
        {font-family:Consolas;
        panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0in;
        margin-bottom:.0001pt;
        font-size:12.0pt;
        font-family:"Times New Roman",serif;
        color:black;}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:blue;
        text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
        {mso-style-priority:99;
        color:purple;
        text-decoration:underline;}
pre
        {mso-style-priority:99;
        mso-style-link:"HTML Preformatted Char";
        margin:0in;
        margin-bottom:.0001pt;
        font-size:10.0pt;
        font-family:"Courier New";
        color:black;}
span.HTMLPreformattedChar
        {mso-style-name:"HTML Preformatted Char";
        mso-style-priority:99;
        mso-style-link:"HTML Preformatted";
        font-family:Consolas;
        color:black;}
span.new
        {mso-style-name:new;}
span.changed
        {mso-style-name:changed;}
span.EmailStyle21
        {mso-style-type:personal-reply;
        font-family:"Calibri",sans-serif;
        color:#1F497D;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-size:10.0pt;}
@page WordSection1
        {size:8.5in 11.0in;
        margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
        {page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]--></head><body bgcolor=white lang=EN-US link=blue vlink=purple><div class=WordSection1><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Hi Prasanta,<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Changes are fine and test logic works fine. <o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>In test case, There are some trailing white spaces, lines with more than 80 character, and typo’s in println please modify them before check-in.<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Also bug mentions that that both no.of copies and from/to page are not getting set. As we discussed it looks like only PageRanges is not working.<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>You can update the bug regarding the same with Fix version as 9.<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'><o:p> </o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Thanks,<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Jay<o:p></o:p></span></p><p class=MsoNormal><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'><o:p> </o:p></span></p><div><div style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p class=MsoNormal><b><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext'>From:</span></b><span style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:windowtext'> prasanta sadhukhan <br><b>Sent:</b> Wednesday, March 30, 2016 12:18 PM<br><b>To:</b> Phil Race; Jayathirth D V<br><b>Cc:</b> 2d-dev@openjdk.java.net<br><b>Subject:</b> Re: [9] RFR: JDK-8061258,,[macosx] PrinterJob's native Print Dialog does not reflect specified Copies or Page Ranges<o:p></o:p></span></p></div></div><p class=MsoNormal><o:p> </o:p></p><p class=MsoNormal>Hi Phil,<br><br>I have updated the webrev with your proposed change.<br><a href="http://cr.openjdk.java.net/~psadhukhan/8061258/webrev.02/">http://cr.openjdk.java.net/~psadhukhan/8061258/webrev.02/</a><br><br>Jay, can you please review and give your +1 on this?<br><br>Regards<br>Prasanta<o:p></o:p></p><div><p class=MsoNormal>On 3/29/2016 11:17 PM, Phil Race wrote:<o:p></o:p></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><div><pre>Hi,<o:p></o:p></pre><pre><o:p> </o:p></pre><pre>You are calling these unconditionally :<o:p></o:p></pre><pre><span class=new> 387     jint minPage = JNFCallIntMethod(env, srcPrinterJob, jm_getMinPage);</span><o:p></o:p></pre><pre><span class=new> 388     jint maxPage = JNFCallIntMethod(env, srcPrinterJob, jm_getMaxPage);</span><o:p></o:p></pre><p class=MsoNormal><br>But it seems they are only used/needed in the else {..}<br><br>Other than that it looks reasonable but I have not had time to<br>apply the patch and verify it ..<br><br>-phil.<br><br>On 03/23/2016 11:47 PM, prasanta sadhukhan wrote:<o:p></o:p></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal style='margin-bottom:12.0pt'>Hi Phil,<o:p></o:p></p><div><p class=MsoNormal>On 3/24/2016 4:39 AM, Philip Race wrote:<o:p></o:p></p></div><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal><br><br>On 3/23/16, 4:02 AM, prasanta sadhukhan wrote: <o:p></o:p></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal>Hi Phil, <br><br>Please review a fix for jdk9 <br>Bug: <a href="https://bugs.openjdk.java.net/browse/JDK-8061258">https://bugs.openjdk.java.net/browse/JDK-8061258</a> <o:p></o:p></p></blockquote><p class=MsoNormal><br>Gosh, it appears I submitted that bug 18 months ago although<br>I don't remember how I came across the problem.<br><br>I need to understand the fix better and it doesn't help that I don't understand this line :-<o:p></o:p></p><pre><span class=changed> } else if (selectID == 3) {<o:p></o:p></span></pre></blockquote><p class=MsoNormal>Yes, this is wrong. I was thinking selectID ==3 was for PD_SELECTION but it was not.<br>Anyways, I have rectified this and updated the webrev:<br><a href="http://cr.openjdk.java.net/%7Epsadhukhan/8061258/webrev.01/">http://cr.openjdk.java.net/~psadhukhan/8061258/webrev.01/</a><br><br>where the else part handles both <o:p></o:p></p><pre><span class=new>PD_SELECTION or PD_NOSELECTION<o:p></o:p></span></pre><pre><span class=new><o:p> </o:p></span></pre><pre><span class=new>I tested with JobAttributes.setDefaultSelection(ALL/RANGE/SELECTION) and also without defaultselection and all are working fine.<o:p></o:p></span></pre><p class=MsoNormal>Regards<br>Prasanta<br><br><o:p></o:p></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><pre><span class=changed><o:p> </o:p></span></pre><pre><span class=changed>SelectID is set from a single flag bit .. so it will never be 3,<o:p></o:p></span></pre><pre><span class=changed>at least not anywhere I can find. What case is that supposed to<o:p></o:p></span></pre><pre><span class=changed>be covering ?<o:p></o:p></span></pre><pre><span class=changed><o:p> </o:p></span></pre><pre><span class=changed>-phil.<o:p></o:p></span></pre><p class=MsoNormal><br><br><o:p></o:p></p><blockquote style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal>webrev :<a href="http://cr.openjdk.java.net/%7Epsadhukhan/8061258/webrev.00/">http://cr.openjdk.java.net/~psadhukhan/8061258/webrev.00/</a> <br><br>When user attempts to pre-populate the native dialog with copies and page ranges by calling <br> PrintRequestAttributeSet aset = new HashPrintRequestAttributeSet(); <br>        aset.add(new Copies(2)); <br>        aset.add(new PageRanges(3,4)); <br>the print dialog does not reflect the user-defined setting. <br><br>This is because osx native code was calling getNumberOfPages() from OpenBook.java which was returning UNKNOWN_NUMBER_OF_PAGES (-1). <br>Since getNumberOfPages() returned -1, osx always selected NSPrintAllPages or All Radio button. <br><br>I fixed it by removing this call to getNumberOfPages() and rely on getMinPage/getMaxPage() as was done in windows (awt_PrintControl.cpp:AwtPrintControl::InitPrintDialog) to select which radio button to be selected. <br>If fromPage > minPage or toPage < maxPage, it means user has selected page ranges so PageRange radio button is to be selected else "All" radio button to be selected. <br><br>Regards <br>Prasanta <o:p></o:p></p></blockquote></blockquote><p class=MsoNormal><o:p> </o:p></p></blockquote><p class=MsoNormal><o:p> </o:p></p></blockquote><p class=MsoNormal><o:p> </o:p></p></div></body></html>