[OpenJDK 2D-Dev] [12] Review request for 8201818: [macosx] Printing attributes break page size set via "java.awt.print.Book" object

[Prasanta Sadhukhan]

Hi Anton,

One thing I would like to know..since you told this fix will only 
affects macOS platform, did you check if this is not reproducible in 
windows or linux, if not why? If yes, then maybe we should put the fix 
in more generic place!!

Regards
Prasanta
On 8/23/2018 12:29 AM, Anton Litvinov wrote:
> Hello Phil,
>
> Thank you for review of this fix. It is correct, this code area was 
> already touched by 3 fixes which you specified, but they are just 2 
> attempts to fix the same issue and 1 rollback of the 1st fix, which 
> was dictated by the fact that at that time JDK 9 passed RDP 1. Answers 
> to your questions are following:
>
> 1.  No, absolutely, this bug is not a regression from the fix for 
> 8181659. This bug exists in JDK for a long time, this is proved by the 
> fact that I was able to reproduce it with JDK 8u112 b16, while the fix 
> for 8167102 was never released in GA JDK 8 update, except for 1 
> released build JDK 8u131 b32. I consider this bug as an independent 
> issue from 8167102, because of a difference in a test scenario (in 
> 8167102 "java.awt.print.Printable" interface is involved, in this bug 
> "java.awt.print.Pageable" interface is involved), but at the same time 
> they both have the same root cause. The end user, which reported 
> 8167102 changed conditions by moving its application from setting 
> "Printable" object to a printer job to setting "Pageable" object 
> ("java.awt.print.Book") and encountered this issue.
>
> 2.  Yes, sure, I verified that the bug 8167102 is still not 
> reproducible with this fix by using the existing "jtreg" regression 
> test "test/jdk/java/awt/print/PageFormat/WrongPaperPrintingTest.java".
>
> 3.  Today I have run all available "jtreg" manual and automatic 
> regression tests in the directories specified below and in our closed 
> directories (total 189 tests) using both JDK 12 compiled without and 
> with the fix, and verified that no new test failed on JDK 12 with the 
> fix.
>
> Directories with the executed regression tests:
> - "test/jdk/java/awt/print"
> - "test/jdk/javax/print"
>
> 4. Yes, I verified that the fix works as expected for specified by you 
> 3 cases:
> a)  no print dialog - attached to the bug record test case 
> "PrintingAttributeBreaksBookPageSize.java"
> b)  native print dialog - the created regression test 
> "test/jdk/java/awt/print/PageFormat/WrongPaperForBookPrintingTest.java"
> c)  swing print dialog - the regression test in point #b with 
> modification in 1 line.
>
> Thank you,
> Anton
>
> On 21/08/2018 00:37, Philip Race wrote:
>> Hi,
>>
>> The fix looks reasonable.
>> But we've touched the code being touched here quite recently and the 
>> history
>> there shows that this can be tricky to get right :
>> 8167102: [macosx] PrintRequestAttributeSet breaks page size set using 
>> PageFormat
>> caused
>> 8181192: [macos] javafx.print.PrinterJob.showPrintDialog() hangs on 
>> macOS
>> so then we had
>> 8181659: Create an alternative fix for JDK-8167102, whose fix was 
>> backed out
>>
>> Is this new bug a regression caused by the previous fix ?
>> Have you verified the previous fix is still functional ?
>> What tests have been run ?
>> Can you make sure this works in the cases of
>> a) no dialog
>> b) swing dialog
>> c) native dialog.
>>
>> -phil.
>>
>> On 8/17/18, 12:42 PM, Anton Litvinov wrote:
>>> Hello,
>>>
>>> Could you please review the following fix for the bug.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8201818
>>> Webrev: http://cr.openjdk.java.net/~alitvinov/8201818/jdk12/webrev.00
>>>
>>> The bug consists in the fact that, if any one printing attribute is 
>>> contained in "PrintRequestAttributeSet" during the call to 
>>> "java.awt.print.PrinterJob.print(PrintRequestAttributeSet)" method, 
>>> a page size from "java.awt.print.PageFormat" object provided by 
>>> "java.awt.print.Book" set to a printer job through 
>>> "PrinterJob.setPageable(Pageable)" method is not respected and the 
>>> printer job prints the document on a page of a size equal to a 
>>> default page size of a selected printer.
>>>
>>> CAUSE OF THE BUG:
>>> The root cause of the bug is 
>>> "sun.print.RasterPrinterJob.getPageFormatFromAttributes()" method 
>>> which returns "PageFormat" object constructed from default settings 
>>> of the selected printer, if the attribute set in the printer job is 
>>> not empty. For the case in this bug, when the user explicitly sets 
>>> "Book" object for the printer job, this method should not return 
>>> anything except for "null", because:
>>>
>>> 1. According to the documentation for the method 
>>> "PrinterJob.print(PrintRequestAttributeSet)" from Java Platform SE 8 
>>> API Specification, if a user sets "java.awt.print.Pageable" object 
>>> for "PrinterJob", then printing attributes related to media size, 
>>> imageable area, page orientation will not influence anyhow the 
>>> "PageFormat" object used for printing, and only "PageFormat" 
>>> provided by the "Pageable" object will be used.
>>> Statement: "For clients of the Pageable interface, the PageFormat 
>>> will always be as supplied by that interface, on a per page basis."
>>> Specification URL #1: 
>>> https://docs.oracle.com/javase/8/docs/api/java/awt/print/PrinterJob.html#print-javax.print.attribute.PrintRequestAttributeSet-
>>>
>>> 2. Documentation on the method 
>>> "PrinterJob.printDialog(PrintRequestAttributeSet)" acknowledges the 
>>> point #1 by the statement "If using the Pageable interface, clients 
>>> which intend to use media selected by the user must create a 
>>> PageFormat derived from the user's selections.".
>>> Specification URL #2: 
>>> https://docs.oracle.com/javase/8/docs/api/java/awt/print/PrinterJob.html#printDialog-javax.print.attribute.PrintRequestAttributeSet-
>>>
>>> THE SOLUTION:
>>> The solution consists in changing the method 
>>> "RasterPrinterJob.getPageFormatFromAttributes()" to return "null" 
>>> for cases, when the "Pageable" object set in the printer job is not 
>>> an instance of "sun.print.OpenBook" class, which is exclusively used 
>>> by the printer job to wrap "Printable" object set by the user 
>>> through "PrinterJob.setPrintable(Printable)".
>>>
>>> The fix affects only macOS platform, because 
>>> "RasterPrinterJob.getPageFormatFromAttributes()" method is called 
>>> only from the macOS specific native code in the file 
>>> "src/java.desktop/macosx/native/libawt_lwawt/awt/CPrinterJob.m".
>>>
>>> Thank you,
>>> Anton
>