[OpenJDK 2D-Dev] [9] RFR JDK-8042713, , [macosx] Print dialog does not update attribute set with page range

[Phil Race]

Like the related fix passing down page ranges to the dialog this looks 
reasonable
but I have not had time to apply the patch and test it.
You should test the combination of this fix + that for 8061258 before 
pushing ..

-phil.


On 03/22/2016 04:25 AM, prasanta sadhukhan wrote:
> Hi Phil,
>
> I have modified the webrev as per your review comment regarding 
> 0-based page indices that osx native supports.
> http://cr.openjdk.java.net/~psadhukhan/8042713/webrev.01/
> I have tested with copies and pageranges set with my testcase and also 
> test/java/awt/print/PrinterJob/PageRanges.java
>
> Also, I found one issue with the previous change in that if we select 
> PageRange and then print and then we select ALL, then also previous 
> page ranges was getting printed and not "All" pages.
> This was because osx CPrinterJob.setAttributes() checks for 
> SunPageSelection.class but nowhere SunPageSelection.RANGE/ALL is 
> getting added to attribute in osx
> so attributes.get(SunPageSelection.class) was returning null and
> setPageRange() was getting set with wrong values.
> I added SunPageSelection.RANGE/ALL to attribute and also made sure if 
> SunPageSelection.ALL is selected, then setPageRange(-1, -1) is set 
> similar to windows.
>
> Regards
> Prasanta
> On 3/19/2016 12:51 AM, Phil Race wrote:
>> The bug report seems to be lacking a proper (any) evaluation.
>>
>> It is probably not accurate to say it did not call the "correct" 
>> methods since
>> the problem is actually that there was no method that did this and 
>> you needed
>> to add them and make sure they also did what they in turn called what 
>> native
>> had been calling after setting the attribute.
>>
>>
>> The change itself seems to be mimicing what happens in Windows 
>> printing code,
>> ie WPrinterJob.setRangeCopiesAttribute(..) ?
>>
>> Is this intentional ? If so that would have been useful to point out 
>> so I did not
>> need to hunt it down. Looking at that code I see that is does a 
>> couple of
>> other things (1) has a way of indicating an explicitly set number of 
>> copies
>> vs default copies, (2) accepts a boolean indicating if in the dialog 
>> a range
>> was actually requested. It looks like in the OSX code we only make 
>> the upcall
>> in that case, but I need you to verify that all this works as it is 
>> supposed to.
>>
>> Also native used to use a zero-based index and you have changed that 
>> without
>> commenting on it. First it is still zero-based in the initialisation 
>> and may not
>> always be changed. Secondly if you want 1-based then you seem to have
>> a problem as the new Java method calls
>>
>> setPageRange(from, to);
>>
>> with the 1-based values and that seems to be the method which 
>> accepted the zero-based values.
>> This seems very fishy.
>>
>>
>> -phil.
>>
>>
>> On 03/18/2016 02:02 AM, prasanta sadhukhan wrote:
>>> Hi Phil,
>>>
>>> Please review a fix for
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8042713
>>> webrev: http://cr.openjdk.java.net/~psadhukhan/8042713/webrev.00/
>>>
>>> The OS X platform print dialog has an option to set the page ranges.
>>> If this is done it ought to be reported back in the AttributeSet.
>>> but it was found that the page range attribute is not reported back .
>>>
>>> This is because nsPrintInfoToJavaPrinterJob() does not call correct 
>>> Java setxxxAttributes methods to set the attributes.
>>>
>>> I added the corresponding setPageRangeAttribute method along with 
>>> setCopiesAttribute method to report back the "page range" and 
>>> "copies" in AttributeSet.
>>>
>>> Regards
>>> Prasanta
>>
>