[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [9] RFR JDK-8042713, , [macosx] Print dialog does not update attribute set with
From:       Jayathirth D V <jayathirth.d.v () oracle ! com>
Date:       2016-03-30 10:58:57
Message-ID: 66454446-8cc9-410c-b2e4-ccb81eef6b5b () default
[Download RAW message or body]

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


[prev in list] [next in list] [prev in thread] [next in thread]