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

[prasanta sadhukhan]

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
>