Hi Prasanta,
Changes are fine and test logic works fine. 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. 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. You can update the bug regarding the same with Fix version as 9. Thanks, Jay From: prasanta sadhukhan Sent: Wednesday, March 30, 2016 12:18 PM To: Phil Race; Jayathirth D V Cc: 2d-dev@openjdk.java.net Subject: Re: [9] RFR: JDK-8061258,,[macosx] PrinterJob's native Print Dialog does not reflect specified Copies or Page Ranges Hi Phil, I have updated the webrev with your proposed change. http://cr.openjdk.java.net/~psadhukhan/8061258/webrev.02/ Jay, can you please review and give your +1 on this? Regards Prasanta On 3/29/2016 11:17 PM, Phil Race wrote: Hi, You are calling these unconditionally : 387 jint minPage = JNFCallIntMethod(env, srcPrinterJob, jm_getMinPage); 388 jint maxPage = JNFCallIntMethod(env, srcPrinterJob, jm_getMaxPage); But it seems they are only used/needed in the else {..} Other than that it looks reasonable but I have not had time to apply the patch and verify it .. -phil. On 03/23/2016 11:47 PM, prasanta sadhukhan wrote: Hi Phil, On 3/24/2016 4:39 AM, Philip Race wrote: On 3/23/16, 4:02 AM, prasanta sadhukhan wrote: Hi Phil, Please review a fix for jdk9 Bug: https://bugs.openjdk.java.net/browse/JDK-8061258 Gosh, it appears I submitted that bug 18 months ago although I don't remember how I came across the problem. I need to understand the fix better and it doesn't help that I don't understand this line :- } else if (selectID == 3) { Yes, this is wrong. I was thinking selectID ==3 was for PD_SELECTION but it was not. Anyways, I have rectified this and updated the webrev: HYPERLINK "http://cr.openjdk.java.net/%7Epsadhukhan/8061258/webrev.01/"http://cr.openjdk.java.net/~psadhukhan/8061258/webrev.01/ where the else part handles both PD_SELECTION or PD_NOSELECTION I tested with JobAttributes.setDefaultSelection(ALL/RANGE/SELECTION) and also without defaultselection and all are working fine. Regards Prasanta SelectID is set from a single flag bit .. so it will never be 3, at least not anywhere I can find. What case is that supposed to be covering ? -phil. webrev :HYPERLINK "http://cr.openjdk.java.net/%7Epsadhukhan/8061258/webrev.00/"http://cr.openjdk.java.net/~psadhukhan/8061258/webrev.00/ When user attempts to pre-populate the native dialog with copies and page ranges by calling PrintRequestAttributeSet aset = new HashPrintRequestAttributeSet(); aset.add(new Copies(2)); aset.add(new PageRanges(3,4)); the print dialog does not reflect the user-defined setting. This is because osx native code was calling getNumberOfPages() from OpenBook.java which was returning UNKNOWN_NUMBER_OF_PAGES (-1). Since getNumberOfPages() returned -1, osx always selected NSPrintAllPages or All Radio button. 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. 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. Regards Prasanta
