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

Jayathirth D V jayathirth.d.v at oracle.com
Wed Mar 30 10:58:57 UTC 2016


Hi Prasanta,

Changes are working fine along with 8061258 changes and test case is working fine.
In test case there are some trailing white spaces, unused imports and lines extending 80 character length. Please modify them before pushing.

Thanks,
Jay

-----Original Message-----
From: prasanta sadhukhan 
Sent: Wednesday, March 30, 2016 12:34 PM
To: Phil Race; Jayathirth D V
Cc: 2d-dev at openjdk.java.net
Subject: Re: [9] RFR JDK-8042713,,[macosx] Print dialog does not update attribute set with page range

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




More information about the 2d-dev mailing list