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

[Prasanta Sadhukhan]

looks good to me. Please add @key headful to test before pushing.

Regards
Prasanta
On 8/23/2018 9:49 PM, Anton Litvinov wrote:
> I am sorry for an additional e-mail. In the previous e-mail I forgot 
> to specify, that during the experiment not only JDK 11+28 was used, 
> but also JDK 8u181 b13 was used. The correction in the first sentence 
> below is marked with the bold font style.
>
> On 23/08/2018 16:32, Anton Litvinov wrote:
>> Hello Prasanta,
>>
>> By your explicit request today I have verified and confirm that the 
>> bug is not reproducible on MS Windows 10 OS, Linux Ubuntu 16.04.2 
>> x86_64 OS with JDK 11+28*, JDK 8u181 b13*, therefore I confirm again 
>> that this bug is macOS specific.
>>
>> The root cause of the issue is described in my review request, I do 
>> not see a big reason to accomplish and provide here detailed 
>> comparisons of how printing area code related to this issue is 
>> implemented on all supported platforms; code for Windows, Linux, 
>> macOS platforms has differences. This fix is for this specific bug in 
>> macOS implementation of JDK. The bug is not reproducible on Windows 
>> and Linux, simply because JDK code for Windows, Linux does not 
>> involve calling the method 
>> "RasterPrinterJob.getPageFormatFromAttributes()".
>>
>> I do not want to alter any generic code under this bug record to 
>> address macOS specific issue, it will be certainly more error prone.
>>
>> Thank you,
>> Anton
>>
>> On 23/08/2018 06:40, Prasanta Sadhukhan wrote:
>>> 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
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20180827/bfd80a60/attachment-0001.html>