[OpenJDK 2D-Dev] [9]: RFR JDK-6357905, , java.awt.JobAttributes.getFromPage() and getToPage() always returns "1".

[Phil Race]

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
>>>>
>