<AWT Dev> [9] Review request for 8044444 The output's 'Page-n' footer does not show completely.

[Alexander Scherbatiy]

On 11/21/2014 9:20 PM, Phil Race wrote:
> This seems to me to be asking about something I covered already.
>
> >The latter one appears 'correct' in this case since applying it second
> >fixes the output but I don't have enough information  to know why the
> >values differ.
>
> But you have the test case and I don't ..
>
> Did you try any of what I suggested ?

    The CPrinterJob.getPageFormat() returns right selected printer 
format. The problem is that RasterPrinterJob.attributeToPageFormat() 
method creates
    a default page with right page format and overrides page 
size/imageable area after that to predefined ones.
    This happens only because the CPrinterJob.printLoop() native method 
calls javaPrinterJobToNSPrintInfo(...) method after 
javaPageFormatToNSPrintInfo(...).

    The JCK has a set of JTable print tests. I run them using the 
predefined page size/imageable area and with the selected printer settings.
    The page number is properly printed when the selected printer 
settings are used.

    I have updated the fix to preserve the selected printer page size:
      http://cr.openjdk.java.net/~alexsch/8044444/webrev.01/

   Thanks,
   Alexandr.


>
> -phil.
>
> On 11/18/14 8:17 AM, Alexander Scherbatiy wrote:
>>
>>
>>   Hi Phil,
>>
>>     Before the 8011069 fix 
>> RasterPrinterJob.getPageFormatFromAttributes() method returns null 
>> for null attributes
>>  and native page size for ImageableArea has been used.
>>  After the 8011069 fix the attributes are not null and 
>> updateAttributesWithPageFormat() method rewrites
>>  the ImageableArea size to the default constants.
>>
>>   The question is which ImageableArea size is correct?  If there 
>> should be used default values then the 8044444 is not an issue as all 
>> works as expected.
>>   If it is necessary to use native size then I can update the fix to 
>> do that.
>>
>>   Thanks,
>>   Alexandr.
>>
>>
>> On 10/30/2014 11:10 PM, Phil Race wrote:
>>> When we reach this code everything in the job is already configured 
>>> by a
>>> combination of initial settings and user updates and and we just 
>>> need to read
>>> the settings and pass it on to the native NSPrintInfo.
>>> So surely switching the order should not matter unless one of these
>>> is using the 'wrong' PageFormat ?
>>>
>>> -----------------
>>> the body of the method called here :-
>>>
>>> javaPrinterJobToNSPrintInfo(env, jthis, pageable, printInfo);
>>>
>>> gets its PageFormat as follows :-
>>>
>>>     static JNF_MEMBER_CACHE(jm_getPageFormat, sjc_CPrinterJob, 
>>> "getPageFormatFromAttributes", "()Ljava/awt/print/PageFormat;");
>>> ....
>>>
>>>     jobject page = JNFCallObjectMethod(env, srcPrinterJob, 
>>> jm_getPageFormat);
>>>     if (page != NULL) {
>>>         javaPageFormatToNSPrintInfo(env, NULL, page, dst);
>>>     }
>>>
>>>
>>> So this uses the result of making a call to 
>>> RasterPrinterJob.getPageFormatFromAttributes()
>>>
>>>    protected PageFormat getPageFormatFromAttributes() {
>>>        if (attributes == null) {
>>>             return null;
>>>         }
>>>         return attributeToPageFormat(getPrintService(), 
>>> this.attributes);
>>>    }
>>>
>>> -----------------------------
>>>
>>> whereas
>>>
>>> javaPageFormatToNSPrintInfo(env, jthis, page, printInfo);
>>>
>>> is using a PageFormat obtained as follows :-
>>>
>>>     static JNF_MEMBER_CACHE(jm_getPageFormat, sjc_CPrinterJob, 
>>> "getPageFormat", "(I)Ljava/awt/print/PageFormat;");
>>>
>>>     jobject page = JNFCallObjectMethod(env, jthis, jm_getPageFormat, 
>>> 0);
>>>
>>> This is CPrinterJob.getPageFormat() { .. }
>>>
>>>
>>> Although its not easily apparent what the returned values are in 
>>> each of these cases
>>> it does seem they must be different.
>>>
>>> The latter one appears 'correct' in this case since applying it second
>>> fixes the output but I don't have enough information  to know why the
>>> values differ.
>>>
>>> Looking at the fix for 8011069, it avoided an NPE by always
>>> creating an 'attributes' map, albeit an empty one.
>>> This can change the result of calling getPageFormatFromAttributes from
>>> 'null' to a PageFormat built from an empty attribute set.
>>> If the no-args native printDialog() and the no-args print() call is 
>>> used this will be empty.
>>>
>>> So the method will indeed build - at that moment - a page format 
>>> built from
>>> default values.
>>>
>>> Now. If we *do* use the printDialog(PrintRequestAttributeSet) and
>>> print(PrintRequestAttributeSet) methods, then it may well be that this
>>> method is the one that should be called.
>>>
>>> And I think we were previously only in this block of code if that 
>>> were the case
>>> by virtue of the block being guarded by "if (page != NULL)", which 
>>> means
>>> there is an attributeset, which previously meant one of those "with 
>>> args"
>>> methods had been used.
>>>
>>> So I wonder/suspect if the switching of the order will introduce the 
>>> equivalent
>>> problem in that 'with args' case.
>>>
>>> As you can tell just looking at the webrev its nigh on impossible to 
>>> tell
>>> for sure and you'd probably need to play around with testing changing
>>> paper size and orientation in native and cross-platform dialogs to 
>>> test it.
>>>
>>> You could start by seeing if the test 'passes' simply by switching to
>>> 'with args' before & after your fix - ensuring that the same paper 
>>> sizes
>>> are being used. I am not sure what the default settings were that were
>>> created for the empty attribute set vs the ones that are used when you
>>> fixed this. You'll have to tell me that.
>>>
>>> Perhaps what is needed is a unified call to get the PageFormat which
>>> can figure out whether to use the attributes or not. And you could
>>> check if the call to CPrinterJob.getPageFormat() already performs 
>>> that ..
>>>
>>> -phil.
>>>
>>>
>>> On 10/28/2014 01:03 PM, Alexander Scherbatiy wrote:
>>>>
>>>>  Hello,
>>>>
>>>>  Could you review the fix?
>>>>
>>>>  Thanks,
>>>>  Alexandr.
>>>>
>>>> On 7/15/2014 3:28 PM, Alexander Scherbatiy wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> Could you review the fix:
>>>>>   bug: https://bugs.openjdk.java.net/browse/JDK-8044444
>>>>>   webrev: http://cr.openjdk.java.net/~alexsch/8044444/webrev.00
>>>>>
>>>>>   Native method printLoop from CPrinterJob calls 
>>>>> javaPrinterJobToNSPrintInfo(...) method after 
>>>>> javaPageFormatToNSPrintInfo(...).
>>>>>   Both methods set the page size. The initial page size is set in 
>>>>> defaultPage(PageFormat) method.
>>>>>   After the fix 8011069 the printDialog() initializes attributes 
>>>>> which leads that new page size is created in the 
>>>>> attributeToPageFormat(PrintService, PrintRequestAttributeSet) method.
>>>>>
>>>>>   The fix changes order of the javaPrinterJobToNSPrintInfo(...) 
>>>>> javaPageFormatToNSPrintInfo(...) call so initial page size is set 
>>>>> at the end.
>>>>>
>>>>>   There is the JCK test that covers the issue.
>>>>>
>>>>> Thanks,
>>>>> Alexandr.
>>>>>
>>>>
>>>
>>
>