Quantcast

[9] Review request for 8167102: [macosx] PrintRequestAttributeSet breaks page size set using PageFormat

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[9] Review request for 8167102: [macosx] PrintRequestAttributeSet breaks page size set using PageFormat

Anton Litvinov-2
Hello,

Could you please review the following fix for the bug.

Bug: https://bugs.openjdk.java.net/browse/JDK-8167102
Webrev: http://cr.openjdk.java.net/~alitvinov/8167102/jdk9/webrev.00

The bug consists in the fact that, if the user's application specifies
the required page size (media size) through "java.awt.print.PrinterJob"
API particularly via "java.awt.print.PageFormat" instance supplied to
the method "PrinterJob.setPrintable(Printable, PageFormat)" and calls
"PrinterJob.print(PrintRequestAttributeSet)" with
"javax.print.attribute.PrintRequestAttributeSet" containing at least 1
any "PrintRequestAttribute", then the page or pages will be printed with
"PageFormat" describing the default page size of the printer which is
different from the expected and originally set by the user's application
"PageFormat".

Debugging showed that the new "PageFormat" with unexpected media size is
created in the method
"sun.print.RasterPrinterJob.getPageFormatFromAttributes()" which is
invoked from the native function
"jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/CPrinterJob.m::javaPrinterJobToNSPrintInfo".
The method "RasterPrinterJob.getPageFormatFromAttributes()" returns a
new "PageFormat" always, if the provided by the user
"PrintRequestAttributeSet" is not empty, and the default values are set
to particular instance variables of this "PageFormat", if
"PrintRequestAttributeSet" does not contain the searched
"OrientationRequested", "MediaSizeName", "MediaPrintableArea" attributes.

THE SOLUTION:
Although it is clearly stated in Java Platform SE 8 API Specification
(documentation of the method "PrinterJob.print(PrintRequestAttributeSet)")
Specification URL:
http://docs.oracle.com/javase/8/docs/api/java/awt/print/PrinterJob.html#print-javax.print.attribute.PrintRequestAttributeSet-

that media size, orientation and imageable area attributes specified in
"PrintRequestAttributeSet" supplied to the method "PrinterJob.print"
will be used for construction of a new "PageFormat", which will be
passed to "Printable" object, instead of the original "PageFormat"
instance set through "PrinterJob.setPrintable" method, the observed in
this issue behavior is a bug, because in the bug test case neither media
size, nor orientation, nor imageable area attributes are specified in
"PrintRequestAttributeSet".

The fix alters the method
"sun.print.RasterPrinterJob.getPageFormatFromAttributes()" to use
corresponding values from "PageFormat" instance originally set through
"PrinterJob" API during construction of the new "PageFormat", when it is
found out that "OrientationRequested", or "MediaSizeName", or
"MediaPrintableArea" attribute is not specified in
"PrintRequestAttributeSet" supplied to "PrinterJob.print" method.

Thank you,
Anton
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] Review request for 8167102: [macosx] PrintRequestAttributeSet breaks page size set using PageFormat

prasanta sadhukhan

I think the real problem is not in RasterPrinterJob.getPageFormatFromAttributes().

One can see that, when we call RasterPrinterJob.setPrintable(), we call updatePageAttributes() which in turn calls updateAttributesWithPageFormat() where orientation, media and mediaprintablearea "attributes" get populated/cached from the "pageformat" supplied by the user.

But, when PrinterJob.print(PrintRequestAttributeSet) is called, it calls setAttributes(attributes) where "attributes" is what is populated by the user. It does not contain orientation,media and mediaprintablearea attributes for this bug, so setAttributes sets cached attribute with this user-set attribute instance
>>else {
>>            // for AWT where pageable is not an instance of OpenBook,
>>            // we need to save paper info
 >>           this.attributes = attributes;
 >>       }

thereby losing the attributes it has cached earlier from user pageformat. I think we should check if this.attributes is not null, then retain those attributes unless explicitly set by the user in user-defined attributes.

But, this code is used by windows,linux,mac so it needs to be tested against all those platforms to ensure we are not regressing anything.

Also, I think user should call validatePage(PageFormat) in application to check if the settings passed is compatible with the printer or not, get compatible/adjusted pageformat and call setPrintable() with that "adjusted" pageformat. If user pass 0,0,fullpaperwidth,fullpaperheight as imageablearea, then he should not expect this setting to be used since printer will have its own hardware limitation (and use some offset printing)

BTW, a regression testcase (even if it is manual) should be created with the fix.

Regards
Prasanta
On 3/17/2017 10:59 PM, Anton Litvinov wrote:
Hello,

Could you please review the following fix for the bug.

Bug: https://bugs.openjdk.java.net/browse/JDK-8167102
Webrev: http://cr.openjdk.java.net/~alitvinov/8167102/jdk9/webrev.00

The bug consists in the fact that, if the user's application specifies the required page size (media size) through "java.awt.print.PrinterJob" API particularly via "java.awt.print.PageFormat" instance supplied to the method "PrinterJob.setPrintable(Printable, PageFormat)" and calls "PrinterJob.print(PrintRequestAttributeSet)" with "javax.print.attribute.PrintRequestAttributeSet" containing at least 1 any "PrintRequestAttribute", then the page or pages will be printed with "PageFormat" describing the default page size of the printer which is different from the expected and originally set by the user's application "PageFormat".

Debugging showed that the new "PageFormat" with unexpected media size is created in the method "sun.print.RasterPrinterJob.getPageFormatFromAttributes()" which is invoked from the native function "jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/CPrinterJob.m::javaPrinterJobToNSPrintInfo". The method "RasterPrinterJob.getPageFormatFromAttributes()" returns a new "PageFormat" always, if the provided by the user "PrintRequestAttributeSet" is not empty, and the default values are set to particular instance variables of this "PageFormat", if "PrintRequestAttributeSet" does not contain the searched "OrientationRequested", "MediaSizeName", "MediaPrintableArea" attributes.

THE SOLUTION:
Although it is clearly stated in Java Platform SE 8 API Specification (documentation of the method "PrinterJob.print(PrintRequestAttributeSet)")
Specification URL: http://docs.oracle.com/javase/8/docs/api/java/awt/print/PrinterJob.html#print-javax.print.attribute.PrintRequestAttributeSet-

that media size, orientation and imageable area attributes specified in "PrintRequestAttributeSet" supplied to the method "PrinterJob.print" will be used for construction of a new "PageFormat", which will be passed to "Printable" object, instead of the original "PageFormat" instance set through "PrinterJob.setPrintable" method, the observed in this issue behavior is a bug, because in the bug test case neither media size, nor orientation, nor imageable area attributes are specified in "PrintRequestAttributeSet".

The fix alters the method "sun.print.RasterPrinterJob.getPageFormatFromAttributes()" to use corresponding values from "PageFormat" instance originally set through "PrinterJob" API during construction of the new "PageFormat", when it is found out that "OrientationRequested", or "MediaSizeName", or "MediaPrintableArea" attribute is not specified in "PrintRequestAttributeSet" supplied to "PrinterJob.print" method.

Thank you,
Anton

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] Review request for 8167102: [macosx] PrintRequestAttributeSet breaks page size set using PageFormat

Anton Litvinov-2
Hello Prasanta,

Thank you for review of this fix. I have tried to apply the approach which you suggested and it allowed to resolve the issue. In this case I agree that it would be more correct to resolve the issue in "RasterPrinterJob.setAttributes(PrintRequestAttributeSet)" method. In the first version of the fix I decided to change the method "RasterPrinterJob.getPageFormatFromAttributes()", because it is invoked only from 1 place in JDK code and this place is located in OS X specific code "jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/CPrinterJob.m", so that would automatically minimize the affected by the fix platforms to OS X only.

Starting to work on implementation of the second version of the fix including the regression test.

Thank you,
Anton

On 3/21/2017 12:52 PM, Prasanta Sadhukhan wrote:

I think the real problem is not in RasterPrinterJob.getPageFormatFromAttributes().

One can see that, when we call RasterPrinterJob.setPrintable(), we call updatePageAttributes() which in turn calls updateAttributesWithPageFormat() where orientation, media and mediaprintablearea "attributes" get populated/cached from the "pageformat" supplied by the user.

But, when PrinterJob.print(PrintRequestAttributeSet) is called, it calls setAttributes(attributes) where "attributes" is what is populated by the user. It does not contain orientation,media and mediaprintablearea attributes for this bug, so setAttributes sets cached attribute with this user-set attribute instance
>>else {
>>            // for AWT where pageable is not an instance of OpenBook,
>>            // we need to save paper info
 >>           this.attributes = attributes;
 >>       }

thereby losing the attributes it has cached earlier from user pageformat. I think we should check if this.attributes is not null, then retain those attributes unless explicitly set by the user in user-defined attributes.

But, this code is used by windows,linux,mac so it needs to be tested against all those platforms to ensure we are not regressing anything.

Also, I think user should call validatePage(PageFormat) in application to check if the settings passed is compatible with the printer or not, get compatible/adjusted pageformat and call setPrintable() with that "adjusted" pageformat. If user pass 0,0,fullpaperwidth,fullpaperheight as imageablearea, then he should not expect this setting to be used since printer will have its own hardware limitation (and use some offset printing)

BTW, a regression testcase (even if it is manual) should be created with the fix.

Regards
Prasanta
On 3/17/2017 10:59 PM, Anton Litvinov wrote:
Hello,

Could you please review the following fix for the bug.

Bug: https://bugs.openjdk.java.net/browse/JDK-8167102
Webrev: http://cr.openjdk.java.net/~alitvinov/8167102/jdk9/webrev.00

The bug consists in the fact that, if the user's application specifies the required page size (media size) through "java.awt.print.PrinterJob" API particularly via "java.awt.print.PageFormat" instance supplied to the method "PrinterJob.setPrintable(Printable, PageFormat)" and calls "PrinterJob.print(PrintRequestAttributeSet)" with "javax.print.attribute.PrintRequestAttributeSet" containing at least 1 any "PrintRequestAttribute", then the page or pages will be printed with "PageFormat" describing the default page size of the printer which is different from the expected and originally set by the user's application "PageFormat".

Debugging showed that the new "PageFormat" with unexpected media size is created in the method "sun.print.RasterPrinterJob.getPageFormatFromAttributes()" which is invoked from the native function "jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/CPrinterJob.m::javaPrinterJobToNSPrintInfo". The method "RasterPrinterJob.getPageFormatFromAttributes()" returns a new "PageFormat" always, if the provided by the user "PrintRequestAttributeSet" is not empty, and the default values are set to particular instance variables of this "PageFormat", if "PrintRequestAttributeSet" does not contain the searched "OrientationRequested", "MediaSizeName", "MediaPrintableArea" attributes.

THE SOLUTION:
Although it is clearly stated in Java Platform SE 8 API Specification (documentation of the method "PrinterJob.print(PrintRequestAttributeSet)")
Specification URL: http://docs.oracle.com/javase/8/docs/api/java/awt/print/PrinterJob.html#print-javax.print.attribute.PrintRequestAttributeSet-

that media size, orientation and imageable area attributes specified in "PrintRequestAttributeSet" supplied to the method "PrinterJob.print" will be used for construction of a new "PageFormat", which will be passed to "Printable" object, instead of the original "PageFormat" instance set through "PrinterJob.setPrintable" method, the observed in this issue behavior is a bug, because in the bug test case neither media size, nor orientation, nor imageable area attributes are specified in "PrintRequestAttributeSet".

The fix alters the method "sun.print.RasterPrinterJob.getPageFormatFromAttributes()" to use corresponding values from "PageFormat" instance originally set through "PrinterJob" API during construction of the new "PageFormat", when it is found out that "OrientationRequested", or "MediaSizeName", or "MediaPrintableArea" attribute is not specified in "PrintRequestAttributeSet" supplied to "PrinterJob.print" method.

Thank you,
Anton


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] Review request for 8167102: [macosx] PrintRequestAttributeSet breaks page size set using PageFormat

prasanta sadhukhan

Hi Anton,

I thought about your point and have a question: does this issue not reproduce in non-mac platform?
I thought it probably would so suggested modifying setAttributes() . But, if it is only reproduced in mac, we need to find out why despite this setAttributes() flaw, how this is working in non-mac platform?

It probably might work in windows/linux because in RasterPrinterJob.print(attr) method, after it calls setAttributes(), it calls printPage() where it gets the original PageFormat
by calling getPageFormat(pageIndex) and gets the orientation, imageablearea
and not by constructing pageFormat from attributes as it is done in mac.
So, probably, in mac also, we need to do away with getPageFormatFromAttributes() call  and call getPageFormat(pageIndex) from CPrinterJob.m#javaPrinterJobToNSPrintInfo().

Regards
Prasanta
On 3/21/2017 8:15 PM, Anton Litvinov wrote:
Hello Prasanta,

Thank you for review of this fix. I have tried to apply the approach which you suggested and it allowed to resolve the issue. In this case I agree that it would be more correct to resolve the issue in "RasterPrinterJob.setAttributes(PrintRequestAttributeSet)" method. In the first version of the fix I decided to change the method "RasterPrinterJob.getPageFormatFromAttributes()", because it is invoked only from 1 place in JDK code and this place is located in OS X specific code "jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/CPrinterJob.m", so that would automatically minimize the affected by the fix platforms to OS X only.

Starting to work on implementation of the second version of the fix including the regression test.

Thank you,
Anton

On 3/21/2017 12:52 PM, Prasanta Sadhukhan wrote:

I think the real problem is not in RasterPrinterJob.getPageFormatFromAttributes().

One can see that, when we call RasterPrinterJob.setPrintable(), we call updatePageAttributes() which in turn calls updateAttributesWithPageFormat() where orientation, media and mediaprintablearea "attributes" get populated/cached from the "pageformat" supplied by the user.

But, when PrinterJob.print(PrintRequestAttributeSet) is called, it calls setAttributes(attributes) where "attributes" is what is populated by the user. It does not contain orientation,media and mediaprintablearea attributes for this bug, so setAttributes sets cached attribute with this user-set attribute instance
>>else {
>>            // for AWT where pageable is not an instance of OpenBook,
>>            // we need to save paper info
 >>           this.attributes = attributes;
 >>       }

thereby losing the attributes it has cached earlier from user pageformat. I think we should check if this.attributes is not null, then retain those attributes unless explicitly set by the user in user-defined attributes.

But, this code is used by windows,linux,mac so it needs to be tested against all those platforms to ensure we are not regressing anything.

Also, I think user should call validatePage(PageFormat) in application to check if the settings passed is compatible with the printer or not, get compatible/adjusted pageformat and call setPrintable() with that "adjusted" pageformat. If user pass 0,0,fullpaperwidth,fullpaperheight as imageablearea, then he should not expect this setting to be used since printer will have its own hardware limitation (and use some offset printing)

BTW, a regression testcase (even if it is manual) should be created with the fix.

Regards
Prasanta
On 3/17/2017 10:59 PM, Anton Litvinov wrote:
Hello,

Could you please review the following fix for the bug.

Bug: https://bugs.openjdk.java.net/browse/JDK-8167102
Webrev: http://cr.openjdk.java.net/~alitvinov/8167102/jdk9/webrev.00

The bug consists in the fact that, if the user's application specifies the required page size (media size) through "java.awt.print.PrinterJob" API particularly via "java.awt.print.PageFormat" instance supplied to the method "PrinterJob.setPrintable(Printable, PageFormat)" and calls "PrinterJob.print(PrintRequestAttributeSet)" with "javax.print.attribute.PrintRequestAttributeSet" containing at least 1 any "PrintRequestAttribute", then the page or pages will be printed with "PageFormat" describing the default page size of the printer which is different from the expected and originally set by the user's application "PageFormat".

Debugging showed that the new "PageFormat" with unexpected media size is created in the method "sun.print.RasterPrinterJob.getPageFormatFromAttributes()" which is invoked from the native function "jdk/src/java.desktop/macosx/native/libawt_lwawt/awt/CPrinterJob.m::javaPrinterJobToNSPrintInfo". The method "RasterPrinterJob.getPageFormatFromAttributes()" returns a new "PageFormat" always, if the provided by the user "PrintRequestAttributeSet" is not empty, and the default values are set to particular instance variables of this "PageFormat", if "PrintRequestAttributeSet" does not contain the searched "OrientationRequested", "MediaSizeName", "MediaPrintableArea" attributes.

THE SOLUTION:
Although it is clearly stated in Java Platform SE 8 API Specification (documentation of the method "PrinterJob.print(PrintRequestAttributeSet)")
Specification URL: http://docs.oracle.com/javase/8/docs/api/java/awt/print/PrinterJob.html#print-javax.print.attribute.PrintRequestAttributeSet-

that media size, orientation and imageable area attributes specified in "PrintRequestAttributeSet" supplied to the method "PrinterJob.print" will be used for construction of a new "PageFormat", which will be passed to "Printable" object, instead of the original "PageFormat" instance set through "PrinterJob.setPrintable" method, the observed in this issue behavior is a bug, because in the bug test case neither media size, nor orientation, nor imageable area attributes are specified in "PrintRequestAttributeSet".

The fix alters the method "sun.print.RasterPrinterJob.getPageFormatFromAttributes()" to use corresponding values from "PageFormat" instance originally set through "PrinterJob" API during construction of the new "PageFormat", when it is found out that "OrientationRequested", or "MediaSizeName", or "MediaPrintableArea" attribute is not specified in "PrintRequestAttributeSet" supplied to "PrinterJob.print" method.

Thank you,
Anton



Loading...