Hi Prasanta, Changes are fine and test case looks good. Please make proper indentation for jtreg comments before checkin.
Thanks, Jay -----Original Message----- From: Phil Race Sent: Friday, April 01, 2016 10:47 PM To: prasanta sadhukhan Cc: 2d-dev Subject: Re: [OpenJDK 2D-Dev] [9]: RFR JDK-6357905, , java.awt.JobAttributes.getFromPage() and getToPage() always returns "1". Looks good ! -phil. On 04/01/2016 12:02 AM, prasanta sadhukhan wrote: > Please find the modified webrev with check for existing values to > select which order to do the update. > http://cr.openjdk.java.net/~psadhukhan/6357905/webrev.02/ > > I have checked with > 1) initially user sets from=3, to=4 and then user change from=1,to=2 > 2) initially user sets from=3, to=4 and then user change from=2,to=3 > 2) no initial setting from user, so from=1,to=1 and user change > from=2,to=3 > 3)no initial setting from user, so from=1,to=1 and user change > from=1,to=2 > > Regards > Prasanta > On 3/31/2016 9:19 PM, Philip Race wrote: >> Actually never mind about the > or >= difference that code is fine >> since it is the condition under which the exception is thrown not the >> condition that the setting is accepted ! >> >> But you do still need to check the existing values to see which order >> to do the update. >> >> -phil >> >> On 3/31/16, 8:44 AM, Philip Race wrote: >>> >>> >>> On 3/30/16, 10:22 PM, prasanta sadhukhan wrote: >>>> Hi Phil, >>>> >>>> I found out we do not need to change minPage. >>> That makes more sense now. >>> >>>> Changing toPage before frompage should be enough. >>>> Please find the modified webrev: >>>> http://cr.openjdk.java.net/~psadhukhan/6357905/webrev.01/ >>> >>> Don't you need to check ? setToPage also checks that to > from >>> >>> Supposing we start with from=3,to=4 >>> and you want to change it so that from=1,to=2 when you try to change >>> the to page (in the first step) to 3,2 you will also get an IAE. >>> >>> So you need to examine the existing values to decide which order to >>> do the update. >>> >>> A bad API design IMO, it should have been setPageRange(int from, int >>> to) >>> but it is too late for that. >>> >>> BTW I notice it really is a requirement that from < to and to > from >>> >>> eg : I see >>> (toPage != 0 && fromPage > toPage) || >>> >>> I would have expected >= .. >>> >>> .. in fact the docs for setFromPage say : >>> * @param fromPage an integer greater than zero and less than >>> or equal to >>> * <i>toPage</i> >>> >>> So the implementation looks wrong to me for that and setToPage. >>> >>> -phil. >>>> >>>> Regards >>>> Prasanta >>>> On 3/29/2016 6:45 AM, Philip Race wrote: >>>>> Please add an evaluation to the bug report. >>>>> >>>>> As to the fix I am under the impression that min&max page are >>>>> meant to constrain what the user can enter in the dialog so if min >>>>> page & max page are set to "1" then the problem may be with those >>>>> settings ? >>>>> >>>>> i.e the application can set those values and expect the user can't >>>>> change them so updating our code to circumvent that seems wrong. >>>>> >>>>> -phil. >>>>> >>>>> On 3/28/16, 1:38 AM, prasanta sadhukhan wrote: >>>>>> Hi All, >>>>>> >>>>>> Please review a print job attribute fix for jdk9. >>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-6357905 >>>>>> webrev: http://cr.openjdk.java.net/~psadhukhan/6357905/webrev.00/ >>>>>> >>>>>> The issue was in the Print-dialog, when some pages are specified >>>>>> using "Pages" within "Page range", >>>>>> java.awt.JobAttributes.getFromPage() and getToPage() always >>>>>> returns "1" >>>>>> and not the values as updated by the user. >>>>>> The fix was to get the pageranges attribute as set by the user >>>>>> and obtain the from and to Page range and update the >>>>>> JobAttribute's from and to Page. >>>>>> >>>>>> Regards >>>>>> Prasanta >>>> >