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

[prasanta sadhukhan]

Hi Phil,

Yes, the combination of this fix plus 8061258 works with both the 
testcase I have along with the fix(es).

Jay, can you please review and give your +1 on this?

Regards
Prasanta
On 3/30/2016 12:13 AM, Phil Race wrote:
> 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
>>>
>>
>