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
