[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] [9] Review request for 8167102: [macosx] PrintRequestAttributeSet breaks page s
From: Phil Race <philip.race () oracle ! com>
Date: 2017-03-30 18:42:02
Message-ID: f409ab7b-7f1f-e6c2-14c9-0cf303335860 () oracle ! com
[Download RAW message or body]
Hi,
OK I accept your assurances regarding testing and the observed
behaviours, so "+1" to the fix.
If you plan to push this to jdk 9 please follow the RDP2 process and add
the required "Fix comment" and jdk9-fix-request label.
-phil.
On 03/30/2017 11:32 AM, Anton Litvinov wrote:
> Hello Phil,
>
> Thank you very much for review of this fix. The new or the 3rd version
> of the fix, in which I tried to address your remarks, is created and
> is available at the next URL. The changes introduced into the fix are
> summarized in the end of the e-mail. Could you please review the new
> version of the fix.
>
> Webrev (the 3rd version):
> http://cr.openjdk.java.net/~alitvinov/8167102/jdk9/webrev.02
>
> In the second version of the fix I decided not to show the print
> dialog just to reduce the number of manual actions. Today I have
> practically verified that usage of either native or cross-platform
> dialogs does not influence the test behavior anyhow, the document is
> printed with a wrong paper size without the fix, and with the expected
> paper size with the fix applied.
>
> The user who reported the bug used "4 x 6 in" paper size, but in the
> regression test I decided to use A5 paper size, because it is standard
> and most importantly it is small, what lets to easily distinguish it
> from, for example, A4 or "Letter" paper sizes usually used as default
> on printers. But, yes, sure, the bug is not specific to A5, and to
> observe it is necessary just to have a printer's default paper size be
> different from a paper size in "PageFormat" set through
> "PrinterJob.setPrintable(Printable, PageFormat)".
>
> I decided to mark the test, as requiring only Mac platform, because
> the issue occurs only on OS X platform and to reduce the number of
> manual tests for other platforms.
>
> Yes, sure, I verified practically and can confirm that with the
> applied fix, if all or some of the attributes "OrientationRequested",
> "MediaSizeName", "MediaPrintableArea" are specified in
> "PrintRequestAttributeSet" transferred to
> "PrinterJob.print(PrintRequestAttributeSet)" method, then the new
> "PageFormat" based on values of these attributes is still created and
> successfully applied to the printed page. This functionality is not
> changed because, the new page format is created in the method
> "RasterPrinterJob.setAttributes(PrintRequestAttributeSet)" in the "if"
> block specified below
>
> "if ((orientReq != null || media != null || mpa != null) &&
> getPageable() instanceof OpenBook) {"
>
> and saved in the PrinterJob in the end of this "if" block by the
> expression "setPrintable(printable, pf);". The sequence of calls is
> following:
>
> CPrinterJob.print(PrintRequestAttributeSet) -->
> CPrinterJob.setAttributes(PrintRequestAttributeSet) -->
> RasterPrinterJob.setAttributes(PrintRequestAttributeSet)
>
> CHANGES IN THE 3RD VERSION OF THE FIX:
> In this version of the fix only the regression test was changed.
>
> 1) The jtreg argument "27 @requires (os.family == "mac")" was
> removed and I verified that the test works and does not fail on
> Windows OS and Linux OS.
> 2) Showing of the native print dialog is added to the test.
> "102 if (job.printDialog()) {"
> 3) The next "if" block is completely removed, because the condition
> will never be fulfilled.
>
> 94 if (isoA5Size == null) {
> 95 throw new RuntimeException(
> 96 "No 'MediaSize' was found for
> 'MediaSizeName.ISO_A5'.");
> 97 }
>
> 4) The test timeout "60 private static final int testTimeout =
> 300000;" is increased from previous 3 minutes to 5 minutes.
> 5) The hard coded instruction for execution of the test was changed to
> better reflect the test.
>
> Thank you,
> Anton
>
> On 3/29/2017 8:34 PM, Philip Race wrote:
> > If the test is manual anyway, why does it not display a dialog so it
> > is easier
> > to run. On mac you can always then save as PDF so the "virtual
> > printer" isn't needed.
> > Note: this assumes you are using the native dialog not the Swing one.
> > Does this in some way change the test so that it passed anyway ?
> >
> >
> > I also think you should try to verify this with
> > (a) after displaying the cross-platform dialogs.
> > (b) after displaying the native dialogs.
> >
> > There are an amazing number of code paths here ..
> >
> > throw new RuntimeException(
> > "No 'MediaSize' was found for 'MediaSizeName.ISO_A5'.");
> > Why insist on A5 ? Any number of sizes should work ?
> > And making the test fail in such a case is plain wrong.
> >
> > Why @requires (os.family == "mac")
> > since the test can run everywhere ? And it should pass everywhere too.
> > This should be fixed.
> >
> > The directory name in the test is gratuitous and unneeded and
> > the test name itself is ridiculously long.
> > Instead let's call it
> >
> > Regarding the fix itself I am unclear what the impact is in
> > the case the app supplies an attribute set that *does* have one or
> > more attributes that affect PageFormat. It might well be that
> > these are applied already but I'd like assurance that has been verified.
> > Some of the code references below seem to be a bit munged by mail
> > so I can't work out what is being said.
> >
> > -phil.
> >
> >
> >
> > On 3/28/17, 5:55 AM, Anton Litvinov wrote:
> > > Hello Prasanta,
> > >
> > > The correct "PageFormat" for each separate page is retrieved in the
> > > native function "PrinterView.m::rectForPage:(NSInteger)" by invoking
> > > the Java method
> > > "CPrinterJob.getPageformatPrintablePeekgraphics(int)" with the first
> > > expression specified below and getting the value of "PageFormat"
> > > from the returned array with the second expression specified below.
> > >
> > > "jobjectArray objectArray = JNFCallObjectMethod(env, fPrinterJob,
> > > jm_getPageformatPrintablePeekgraphics, jPageNumber); //
> > > AWT_THREADING Safe (AWTRunLoopMode)"
> > > "jobject pageFormat = (*env)->GetObjectArrayElement(env,
> > > objectArray, 0);"
> > >
> > > But in that native method "PrinterView.m::rectForPage" only the page
> > > orientation field "mOrientation" of the retrieved "PageFormat" for
> > > each page is analyzed and is set to "NSPrintInfo" object through
> > > "NSPrintOperation". Thus for some reason at that method analysis of
> > > the paper size is ignored.
> > >
> > > So obviously not taking into account individual paper size of the
> > > pages for the case, which you described, when the user's code
> > > specifies different "PageFormat" instances for different pages of
> > > the document, should take place in JDK, though I did not verify this
> > > practically. But this issue existed before my fix and is not a
> > > result of the fix.
> > >
> > > I do not see anything bad in the hard coded "0" page number used in
> > > my fix, because we need to initialize "NSPrintInfo" with a valid
> > > page size and the page size of the first page of the document is the
> > > only correct or appropriate value for this moment, and because this
> > > approach already exists in "RasterPrinterJob.java" as the next
> > > expression.
> > >
> > > "PageFormat pf = (PageFormat)pageable.getPageFormat(0).clone();"
> > >
> > > From my point of view, the issue about disrespect of different paper
> > > sizes for different pages of the document is the issue which is
> > > different from the issue described in this bug (JDK-8167102) and
> > > should be fixed separately from my bug, because:
> > > - In my bug "java.awt.print.Printable" interface is involved, while
> > > in this new issue "java.awt.print.Pageable" interface is the
> > > explicit requirement;
> > > - In my bug calling "PrinterJob.print(PrintRequestAttributeSet)"
> > > with a non-empty "PrintRequestAttributeSet" is the obligatory and
> > > the key requirement, while in the new issue this condition is
> > > irrelevant.
> > > - For my bug one regression test is necessary, while for the new
> > > issue the different regression test is necessary.
> > >
> > > Therefore I suggest to file a separate bug for the discovered issue
> > > and to address it separately from this bug (JDK-8167102). Would you
> > > agree with this suggestion? Would you approve the second version of
> > > the fix for the bug JDK-8167102?
> > >
> > > Thank you,
> > > Anton
> > >
> > > On 3/28/2017 12:46 PM, Prasanta Sadhukhan wrote:
> > > >
> > > > Hi Anton,
> > > >
> > > >
> > > > On 3/27/2017 10:05 PM, Anton Litvinov wrote:
> > > > > Hello Prasanta,
> > > > >
> > > > > Indeed it is Mac specific, as it was written in my previous
> > > > > e-mail, I verified this fact on Windows OS and Linux OS by your
> > > > > request.
> > > > >
> > > > > Yes, I am fully sure that, when the bug occurs, the paper size of
> > > > > the printed page is wrong, as it is stated in the bug, and I am
> > > > > fully sure that with the applied any of 2 versions of the created
> > > > > fix, the paper size of the printed page becomes correct and as
> > > > > expected. Otherwise, I would not send the fix for review. The
> > > > > instruction, by following which the bug can be reproduced, is
> > > > > written in "STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :" section of
> > > > > the description of the bug in JBS. This bug is raised by the user,
> > > > > who owns a support contract and requests for resolution of this
> > > > > bug, this can be seen in proper comments of the bug record.
> > > > >
> > > > > The automated regression test is not possible for this bug, since
> > > > > it is necessary to verify visually that the document is physically
> > > > > printed on the paper of the expected size. Otherwise, I would send
> > > > > the 1st version of the fix with the automated test already, it is
> > > > > not the first bug in JDK on which I have been working. By your
> > > > > request the regression test, even though it is manual, was created
> > > > > and added to the 2nd version of the fix.
> > > > >
> > > > > Yes, it is correct, the implemented by the test, and the test case
> > > > > method "Printable.print(Graphics, PageFormat, int)" receives the
> > > > > correct instance of "PageFormat" in runtime in scenario described
> > > > > in this bug, because, as you already described, it is extracted
> > > > > using the expression "pageable.getPageFormat(pageIndex)" in the
> > > > > Java method
> > > > > "sun.lwawt.macosx.CPrinterJob.getPageformatPrintablePeekgraphics(int)"
> > > > > called from "PrinterView.m::rectForPage:(NSInteger)" and further
> > > > > transferred in this method to the Java method
> > > > > "sun.lwawt.macosx.CPrinterJob.printAndGetPageFormatArea".
> > > > >
> > > > > The wrong paper size which is returned from the method
> > > > > "RasterPrinterJob.getPageFormatFromAttributes()" and which is set
> > > > > to certain fields of the Objective-C object "NSPrintInfo" in the
> > > > > function "CPrinterJob.m::javaPaperToNSPrintInfo" through the next
> > > > > call sequence
> > > > >
> > > > > CPrinterJob.m::printLoop() -->
> > > > > CPrinterJob.m::javaPrinterJobToNSPrintInfo() -->
> > > > > CPrinterJob.m::javaPageFormatToNSPrintInfo() -->
> > > > > CPrinterJob.m::javaPaperToNSPrintInfo()
> > > > >
> > > > > further influences physical size of all pages printed by 1 printer
> > > > > job.
> > > > >
> > > > > Thus, I consider that firstly "PageFormat" returned from
> > > > > "RasterPrinterJob.getPageFormatFromAttributes()" is wrong, and
> > > > > secondly setting paper size from this wrong "PageFormat" to proper
> > > > > fields of "NSPrintInfo" object is also incorrect and is a root
> > > > > cause of this bug.
> > > > OK. So, pageformat values set to native NSPrintInfo is different
> > > > (wrong) compared to what is being passed to user.
> > > > >
> > > > > Implementation of Java Print Server API surely contains many
> > > > > issues, and only working on this bug I already encountered 2
> > > > > additional different issues, which are described in one of my
> > > > > comments in this bug record in JBS. However, I prefer to resolve
> > > > > issues separately and to resolve this particular bug also
> > > > > separately from other issues which we can indefinitely find while
> > > > > looking at the code and debugging it, also because it is necessary
> > > > > to deliver the fix for this bug to "jdk8u-dev" repository finally,
> > > > > while JDK 9 is currently in RDP 2 phase at which large fixes
> > > > > affecting more platforms or large code scope would hardly be
> > > > > accepted by Group and Area leads or the release team.
> > > > >
> > > > > I would like to bring also your attention again to the fact, which
> > > > > was mentioned in my previous e-mail, that I already ran all manual
> > > > > and automatic "jtreg" regression tests related to printing from
> > > > > open and closed parts of JDK on JDK 9 without the fix and with the
> > > > > fix, what is 198 tests.
> > > > >
> > > > > Do you find anything incorrect in the 2nd version of the fix? Will
> > > > > you approve the 2nd version of the fix?
> > > > >
> > > > I think there might be a (probable) issue with this fix. PageFormat
> > > > of 1st page only is used to get the information.
> > > > jobject page = JNFCallObjectMethod(env, srcPrinterJob,
> > > > jm_getPageFormat, (jint)0);
> > > > What if user has set different pageformat to different page like
> > > > this below? Will it not lose the 2nd page pageformat? I guess in
> > > > windows/linux, we obtain pageformat for each pageindex
> > > >
> > > > PrinterJob job = PrinterJob.getPrinterJob();
> > > > // Create a landscape pageformat
> > > > PageFormat pfl = job.defaultPage();
> > > > pfl.setOrientation(PageFormat.LANDSCAPE);
> > > > //Setup a book
> > > > Book bk = new Book();
> > > > bk.append(new xPrintable(), pfl); // 1st page landscape , can be
> > > > diff. pagesize too
> > > > bk.append(new xxPrintable(), job.defaultPage()); //2nd page portrait
> > > > job.setPageable(bk);
> > > >
> > > > Regards
> > > > Prasanta
> > > > > Thank you,
> > > > > Anton
> > > > >
> > > > > On 3/27/2017 9:52 AM, Prasanta Sadhukhan wrote:
> > > > > >
> > > > > > Hi Anton,
> > > > > >
> > > > > > Ok. So, it seems it mac specific. But, are you sure wrong page
> > > > > > size is used in mac as is claimed in the bug?
> > > > > > I thought we could use automated regression test instead of
> > > > > > manual by checking pageformat value in print() as compared to
> > > > > > what user passes in setPrintable().
> > > > > >
> > > > > > Then, I see in print() method in testcase, the "PageFormat"
> > > > > > argument passed has same values as it is passed in setPrintable()
> > > > > > in main() even without your fix.
> > > > > >
> > > > > > If I reverse trace from print() method present in testcase, I see
> > > > > > it is called from CPrinterJob.java#printAndGetPageFormatArea()
> > > > > > which is called from PrinterView.m#rectForPage. And before
> > > > > > calling printAndGetPageFormatArea() it calls
> > > > > > getPageformatPrintablePeekgraphics() which calls
> > > > > > pageable.getPageFormat(pageIndex), so it should behave same as
> > > > > > windows. Am I missing something?
> > > > > >
> > > > > > Regards
> > > > > > Prasanta
> > > > > > On 3/27/2017 3:32 AM, Anton Litvinov wrote:
> > > > > > > Hello Prasanta,
> > > > > > >
> > > > > > > I verified that the bug is not reproducible using JDK 9 compiled
> > > > > > > from the latest version of "jdk9/client" forest neither on
> > > > > > > Windows, nor on Linux platform, therefore, yes, this bug is only
> > > > > > > Mac specific. Debugging showed that on Windows platform the
> > > > > > > behavior is exactly like you described, on Windows
> > > > > > > "RasterPrinterJob.print(PrintRequestAttributeSet)" method is
> > > > > > > responsible for printing the documents and in this method
> > > > > > > "RasterPrinterJob.printPage(Pageable, int)" is called for each
> > > > > > > page separately, and in this "printPage" method "PageFormat"
> > > > > > > instance used for printing of the page is extracted from the the
> > > > > > > transferred as the method argument instance of "Pageable" by the
> > > > > > > expression "origPage = document.getPageFormat(pageIndex);".
> > > > > > >
> > > > > > > The new version of the fix was created. Could you please review
> > > > > > > the second version of the fix.
> > > > > > >
> > > > > > > Webrev (the 2nd version):
> > > > > > > http://cr.openjdk.java.net/~alitvinov/8167102/jdk9/webrev.01
> > > > > > >
> > > > > > > In the second version of the fix:
> > > > > > > 1. Calling for "RasterPrinterJob.getPageFormatFromAttributes()"
> > > > > > > was substituted for
> > > > > > > "sun.lwawt.macosx.CPrinterJob.getPageFormat(int)" in the native
> > > > > > > method "CPrinterJob.m#javaPrinterJobToNSPrintInfo".
> > > > > > > 2. The method "RasterPrinterJob.getPageFormatFromAttributes()"
> > > > > > > was removed, since it is not called from any other code in "jdk"
> > > > > > > repository.
> > > > > > > 3. The manual regression test was created.
> > > > > > >
> > > > > > > Also on OS X I executed all manual and automatic "jtreg"
> > > > > > > regression tests related to printing from the listed below
> > > > > > > directories and the corresponding directories in the closed
> > > > > > > repositories using both JDK 9 compiled without and with the fix,
> > > > > > > and I verified that no new test failed on JDK 9 with the fix.
> > > > > > >
> > > > > > > The directories with the regression tests from open repositories:
> > > > > > > - "jdk/test/java/awt/print"
> > > > > > > - "jdk/test/javax/print"
> > > > > > >
> > > > > > > Thank you,
> > > > > > > Anton
> > > > > > >
> > > > > > > On 3/22/2017 7:42 AM, Prasanta Sadhukhan wrote:
> > > > > > > >
> > > > > > > > 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
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > 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:
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > > \
> > > > > > > > > > >
> > > > > > > > > > > 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
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Hi,<br>
<br>
OK I accept your assurances regarding testing and the observed
behaviours, so "+1" to the fix.<br>
If you plan to push this to jdk 9 please follow the RDP2 process and
add the required "Fix comment" and jdk9-fix-request label.<br>
<br>
-phil.<br>
<br>
<div class="moz-cite-prefix">On 03/30/2017 11:32 AM, Anton Litvinov
wrote:<br>
</div>
<blockquote
cite="mid:68b40594-ff76-2c7b-2f0b-185038402637@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
Hello Phil,<br>
<br>
Thank you very much for review of this fix. The new or the 3rd
version of the fix, in which I tried to address your remarks, is
created and is available at the next URL. The changes introduced
into the fix are summarized in the end of the e-mail. Could you
please review the new version of the fix.<br>
<br>
Webrev (the 3rd version): <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ealitvinov/8167102/jdk9/webrev.02">http://cr.openjdk.java.net/~alitvinov/8167102/jdk9/webrev.02</a><br>
<br>
In the second version of the fix I decided not to show the print
dialog just to reduce the number of manual actions. Today I have
practically verified that usage of either native or cross-platform
dialogs does not influence the test behavior anyhow, the document
is printed with a wrong paper size without the fix, and with the
expected paper size with the fix applied.<br>
<br>
The user who reported the bug used "4 x 6 in" paper size, but in
the regression test I decided to use A5 paper size, because it is
standard and most importantly it is small, what lets to easily
distinguish it from, for example, A4 or "Letter" paper sizes
usually used as default on printers. But, yes, sure, the bug is
not specific to A5, and to observe it is necessary just to have a
printer's default paper size be different from a paper size in
"PageFormat" set through "PrinterJob.setPrintable(Printable,
PageFormat)".<br>
<br>
I decided to mark the test, as requiring only Mac platform,
because the issue occurs only on OS X platform and to reduce the
number of manual tests for other platforms.<br>
<br>
Yes, sure, I verified practically and can confirm that with the
applied fix, if all or some of the attributes
"OrientationRequested", "MediaSizeName", "MediaPrintableArea" are
specified in "PrintRequestAttributeSet" transferred to
"PrinterJob.print(PrintRequestAttributeSet)" method, then the new
"PageFormat" based on values of these attributes is still created
and successfully applied to the printed page. This functionality
is not changed because, the new page format is created in the
method "RasterPrinterJob.setAttributes(PrintRequestAttributeSet)"
in the "if" block specified below<br>
<br>
"if ((orientReq != null || media != null || mpa != null)
&&<br>
getPageable() instanceof OpenBook) {"<br>
<br>
and saved in the PrinterJob in the end of this "if" block by the
expression "setPrintable(printable, pf);". The sequence of calls
is following:<br>
<br>
CPrinterJob.print(PrintRequestAttributeSet) -->
CPrinterJob.setAttributes(PrintRequestAttributeSet) -->
RasterPrinterJob.setAttributes(PrintRequestAttributeSet)<br>
<br>
CHANGES IN THE 3RD VERSION OF THE FIX:<br>
In this version of the fix only the regression test was changed.<br>
<br>
1) The jtreg argument "27 @requires (os.family == "mac")" was
removed and I verified that the test works and does not fail on
Windows OS and Linux OS.<br>
2) Showing of the native print dialog is added to the test.
"102 if (job.printDialog()) {"<br>
3) The next "if" block is completely removed, because the
condition will never be fulfilled.<br>
<br>
94 if (isoA5Size == null) {<br>
95 throw new RuntimeException(<br>
96 "No 'MediaSize' was found for
'MediaSizeName.ISO_A5'.");<br>
97 }<br>
<br>
4) The test timeout "60 private static final int testTimeout =
300000;" is increased from previous 3 minutes to 5 minutes.<br>
5) The hard coded instruction for execution of the test was
changed to better reflect the test.<br>
<br>
Thank you,<br>
Anton<br>
<br>
<div class="moz-cite-prefix">On 3/29/2017 8:34 PM, Philip Race
wrote:<br>
</div>
<blockquote cite="mid:58DBF01E.20907@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
If the test is manual anyway, why does it not display a dialog
so it is easier<br>
to run. On mac you can always then save as PDF so the "virtual
printer" isn't needed.<br>
Note: this assumes you are using the native dialog not the Swing
one.<br>
Does this in some way change the test so that it passed anyway ?<br>
<br>
<br>
I also think you should try to verify this with<br>
(a) after displaying the cross-platform dialogs.<br>
(b) after displaying the native dialogs.<br>
<br>
There are an amazing number of code paths here ..<br>
<br>
throw new RuntimeException(<br>
"No 'MediaSize' was found for
'MediaSizeName.ISO_A5'.");<br>
Why insist on A5 ? Any number of sizes should work ?<br>
And making the test fail in such a case is plain wrong.<br>
<br>
Why @requires (os.family == "mac")<br>
since the test can run everywhere ? And it should pass
everywhere too.<br>
This should be fixed.<br>
<br>
The directory name in the test is gratuitous and unneeded and<br>
the test name itself is ridiculously long.<br>
Instead let's call it <br>
<br>
Regarding the fix itself I am unclear what the impact is in<br>
the case the app supplies an attribute set that *does* have one
or<br>
more attributes that affect PageFormat. It might well be that<br>
these are applied already but I'd like assurance that has been
verified.<br>
Some of the code references below seem to be a bit munged by
mail<br>
so I can't work out what is being said.<br>
<br>
-phil.<br>
<br>
<br>
<br>
On 3/28/17, 5:55 AM, Anton Litvinov wrote:
<blockquote
cite="mid:b6a5a113-d958-fb6d-4156-d49b9da9b4f1@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
Hello Prasanta,<br>
<br>
The correct "PageFormat" for each separate page is retrieved
in the native function
"PrinterView.m::rectForPage:(NSInteger)" by invoking the Java
method "CPrinterJob.getPageformatPrintablePeekgraphics(int)"
with the first expression specified below and getting the
value of "PageFormat" from the returned array with the second
expression specified below.<br>
<br>
"jobjectArray objectArray = JNFCallObjectMethod(env,
fPrinterJob, jm_getPageformatPrintablePeekgraphics,
jPageNumber); // AWT_THREADING Safe (AWTRunLoopMode)"<br>
"jobject pageFormat = (*env)->GetObjectArrayElement(env,
objectArray, 0);"<br>
<br>
But in that native method "PrinterView.m::rectForPage" only
the page orientation field "mOrientation" of the retrieved
"PageFormat" for each page is analyzed and is set to
"NSPrintInfo" object through "NSPrintOperation". Thus for some
reason at that method analysis of the paper size is ignored.<br>
<br>
So obviously not taking into account individual paper size of
the pages for the case, which you described, when the user's
code specifies different "PageFormat" instances for different
pages of the document, should take place in JDK, though I did
not verify this practically. But this issue existed before my
fix and is not a result of the fix.<br>
<br>
I do not see anything bad in the hard coded "0" page number
used in my fix, because we need to initialize "NSPrintInfo"
with a valid page size and the page size of the first page of
the document is the only correct or appropriate value for this
moment, and because this approach already exists in
"RasterPrinterJob.java" as the next expression.<br>
<br>
"PageFormat pf =
(PageFormat)pageable.getPageFormat(0).clone();"<br>
<br>
From my point of view, the issue about disrespect of different
paper sizes for different pages of the document is the issue
which is different from the issue described in this bug
(JDK-8167102) and should be fixed separately from my bug,
because:<br>
- In my bug "java.awt.print.Printable" interface is involved,
while in this new issue "java.awt.print.Pageable" interface is
the explicit requirement;<br>
- In my bug calling
"PrinterJob.print(PrintRequestAttributeSet)" with a non-empty
"PrintRequestAttributeSet" is the obligatory and the key
requirement, while in the new issue this condition is
irrelevant.<br>
- For my bug one regression test is necessary, while for the
new issue the different regression test is necessary.<br>
<br>
Therefore I suggest to file a separate bug for the discovered
issue and to address it separately from this bug
(JDK-8167102). Would you agree with this suggestion? Would you
approve the second version of the fix for the bug JDK-8167102?<br>
<br>
Thank you,<br>
Anton<br>
<br>
<div class="moz-cite-prefix">On 3/28/2017 12:46 PM, Prasanta
Sadhukhan wrote:<br>
</div>
<blockquote
cite="mid:2fad447d-2219-e3a6-68c5-91fc4822eb29@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<p>Hi Anton,<br>
</p>
<br>
<div class="moz-cite-prefix">On 3/27/2017 10:05 PM, Anton
Litvinov wrote:<br>
</div>
<blockquote
cite="mid:43402a5f-4e4b-ce6f-262d-e6b21176fb99@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
Hello Prasanta,<br>
<br>
Indeed it is Mac specific, as it was written in my
previous e-mail, I verified this fact on Windows OS and
Linux OS by your request.<br>
<br>
Yes, I am fully sure that, when the bug occurs, the paper
size of the printed page is wrong, as it is stated in the
bug, and I am fully sure that with the applied any of 2
versions of the created fix, the paper size of the printed
page becomes correct and as expected. Otherwise, I would
not send the fix for review. The instruction, by following
which the bug can be reproduced, is written in "STEPS TO
FOLLOW TO REPRODUCE THE PROBLEM :" section of the
description of the bug in JBS. This bug is raised by the
user, who owns a support contract and requests for
resolution of this bug, this can be seen in proper
comments of the bug record.<br>
<br>
The automated regression test is not possible for this
bug, since it is necessary to verify visually that the
document is physically printed on the paper of the
expected size. Otherwise, I would send the 1st version of
the fix with the automated test already, it is not the
first bug in JDK on which I have been working. By your
request the regression test, even though it is manual, was
created and added to the 2nd version of the fix.<br>
<br>
Yes, it is correct, the implemented by the test, and the
test case method "Printable.print(Graphics, PageFormat,
int)" receives the correct instance of "PageFormat" in
runtime in scenario described in this bug, because, as you
already described, it is extracted using the expression
"pageable.getPageFormat(pageIndex)" in the Java method
"sun.lwawt.macosx.CPrinterJob.getPageformatPrintablePeekgraphics(int)"
called from "PrinterView.m::rectForPage:(NSInteger)" and
further transferred in this method to the Java method
"sun.lwawt.macosx.CPrinterJob.printAndGetPageFormatArea".<br>
<br>
The wrong paper size which is returned from the method
"RasterPrinterJob.getPageFormatFromAttributes()" and which
is set to certain fields of the Objective-C object
"NSPrintInfo" in the function
"CPrinterJob.m::javaPaperToNSPrintInfo" through the next
call sequence<br>
<br>
CPrinterJob.m::printLoop() -->
CPrinterJob.m::javaPrinterJobToNSPrintInfo() -->
CPrinterJob.m::javaPageFormatToNSPrintInfo() -->
CPrinterJob.m::javaPaperToNSPrintInfo()<br>
<br>
further influences physical size of all pages printed by 1
printer job.<br>
<br>
Thus, I consider that firstly "PageFormat" returned from
"RasterPrinterJob.getPageFormatFromAttributes()" is wrong,
and secondly setting paper size from this wrong
"PageFormat" to proper fields of "NSPrintInfo" object is
also incorrect and is a root cause of this bug.<br>
</blockquote>
OK. So, pageformat values set to native NSPrintInfo is
different (wrong) compared to what is being passed to user.<br>
<blockquote
cite="mid:43402a5f-4e4b-ce6f-262d-e6b21176fb99@oracle.com"
type="cite"> <br>
Implementation of Java Print Server API surely contains
many issues, and only working on this bug I already
encountered 2 additional different issues, which are
described in one of my comments in this bug record in JBS.
However, I prefer to resolve issues separately and to
resolve this particular bug also separately from other
issues which we can indefinitely find while looking at the
code and debugging it, also because it is necessary to
deliver the fix for this bug to "jdk8u-dev" repository
finally, while JDK 9 is currently in RDP 2 phase at which
large fixes affecting more platforms or large code scope
would hardly be accepted by Group and Area leads or the
release team. <br>
<br>
I would like to bring also your attention again to the
fact, which was mentioned in my previous e-mail, that I
already ran all manual and automatic "jtreg" regression
tests related to printing from open and closed parts of
JDK on JDK 9 without the fix and with the fix, what is 198
tests.<br>
<br>
Do you find anything incorrect in the 2nd version of the
fix? Will you approve the 2nd version of the fix?<br>
<br>
</blockquote>
I think there might be a (probable) issue with this fix.
PageFormat of 1st page only is used to get the information.<br>
<pre><span class="changed">jobject page = JNFCallObjectMethod(env, \
srcPrinterJob, jm_getPageFormat, (jint)0);</span></pre> What if user has set \
different pageformat to different page like this below? Will it not lose the 2nd \
page pageformat? I guess in windows/linux, we obtain pageformat for each
pageindex<br>
<meta http-equiv="Content-Type" content="text/html;
charset=utf-8">
<br>
PrinterJob job = PrinterJob.getPrinterJob();<br>
// Create a landscape pageformat<br>
PageFormat pfl = job.defaultPage();<br>
pfl.setOrientation(PageFormat.LANDSCAPE);<br>
//Setup a book<br>
Book bk = new Book();<br>
bk.append(new xPrintable(), pfl); // 1st page landscape ,
can be diff. pagesize too<br>
bk.append(new xxPrintable(), job.defaultPage()); //2nd page
portrait<br>
job.setPageable(bk);<br>
<br>
Regards<br>
Prasanta<br>
<blockquote
cite="mid:43402a5f-4e4b-ce6f-262d-e6b21176fb99@oracle.com"
type="cite"> Thank you,<br>
Anton<br>
<br>
<div class="moz-cite-prefix">On 3/27/2017 9:52 AM,
Prasanta Sadhukhan wrote:<br>
</div>
<blockquote
cite="mid:ce891f45-bc5e-ec00-8074-880e26b93f39@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<p>Hi Anton,<br>
</p>
Ok. So, it seems it mac specific. But, are you sure
wrong page size is used in mac as is claimed in the bug?<br>
I thought we could use automated regression test instead
of manual by checking pageformat value in print() as
compared to what user passes in setPrintable().<br>
<br>
Then, I see in print() method in testcase, the
"PageFormat" argument passed has same values as it is
passed in setPrintable() in main() even without your
fix.<br>
<br>
If I reverse trace from print() method present in
testcase, I see it is called from
CPrinterJob.java#printAndGetPageFormatArea()<br>
which is called from PrinterView.m#rectForPage. And
before calling printAndGetPageFormatArea() it calls
getPageformatPrintablePeekgraphics() which calls
pageable.getPageFormat(pageIndex), so it should behave
same as windows. Am I missing something?<br>
<br>
Regards<br>
Prasanta<br>
<div class="moz-cite-prefix">On 3/27/2017 3:32 AM, Anton
Litvinov wrote:<br>
</div>
<blockquote
cite="mid:89b66f3b-d1f0-5c88-1b7c-6065ad6ce4da@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
Hello Prasanta,<br>
<br>
I verified that the bug is not reproducible using JDK
9 compiled from the latest version of "jdk9/client"
forest neither on Windows, nor on Linux platform,
therefore, yes, this bug is only Mac specific.
Debugging showed that on Windows platform the behavior
is exactly like you described, on Windows
"RasterPrinterJob.print(PrintRequestAttributeSet)"
method is responsible for printing the documents and
in this method "RasterPrinterJob.printPage(Pageable,
int)" is called for each page separately, and in this
"printPage" method "PageFormat" instance used for
printing of the page is extracted from the the
transferred as the method argument instance of
"Pageable" by the expression "origPage =
document.getPageFormat(pageIndex);".<br>
<br>
The new version of the fix was created. Could you
please review the second version of the fix.<br>
<br>
Webrev (the 2nd version): <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Ealitvinov/8167102/jdk9/webrev.01">http://cr.openjdk.java.net/~alitvinov/8167102/jdk9/webrev.01</a><br>
<br>
In the second version of the fix:<br>
1. Calling for
"RasterPrinterJob.getPageFormatFromAttributes()" was
substituted for
"sun.lwawt.macosx.CPrinterJob.getPageFormat(int)" in
the native method
"CPrinterJob.m#javaPrinterJobToNSPrintInfo".<br>
2. The method
"RasterPrinterJob.getPageFormatFromAttributes()" was
removed, since it is not called from any other code in
"jdk" repository.<br>
3. The manual regression test was created.<br>
<br>
Also on OS X I executed all manual and automatic
"jtreg" regression tests related to printing from the
listed below directories and the corresponding
directories in the closed repositories using both JDK
9 compiled without and with the fix, and I verified
that no new test failed on JDK 9 with the fix.<br>
<br>
The directories with the regression tests from open
repositories:<br>
- "jdk/test/java/awt/print"<br>
- "jdk/test/javax/print"<br>
<br>
Thank you,<br>
Anton<br>
<br>
<div class="moz-cite-prefix">On 3/22/2017 7:42 AM,
Prasanta Sadhukhan wrote:<br>
</div>
<blockquote
cite="mid:b5efdb91-dd13-045f-18bd-f95830ba61fe@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<p>Hi Anton,<br>
</p>
I thought about your point and have a question: does
this issue not reproduce in non-mac platform?<br>
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?<br>
<br>
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 <br>
by calling getPageFormat(pageIndex) and gets the
orientation, imageablearea <br>
and not by constructing pageFormat from attributes
as it is done in mac.<br>
So, probably, in mac also, we need to do away with
getPageFormatFromAttributes() call and call
getPageFormat(pageIndex) from
CPrinterJob.m#javaPrinterJobToNSPrintInfo().<br>
<br>
Regards<br>
Prasanta<br>
<div class="moz-cite-prefix">On 3/21/2017 8:15 PM,
Anton Litvinov wrote:<br>
</div>
<blockquote
cite="mid:7d710d7a-386f-a6c3-7f6d-13e56a7309a6@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
Hello Prasanta,<br>
<br>
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.<br>
<br>
Starting to work on implementation of the second
version of the fix including the regression test.<br>
<br>
Thank you,<br>
Anton<br>
<br>
<div class="moz-cite-prefix">On 3/21/2017 12:52
PM, Prasanta Sadhukhan wrote:<br>
</div>
<blockquote
cite="mid:ae716330-c0c4-9b2c-4f6b-c30855e659dc@oracle.com"
type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<p>I think the real problem is not in
RasterPrinterJob.getPageFormatFromAttributes().
<br>
</p>
<p>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. <br>
</p>
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<br>
<i>>>else {</i><i><br>
</i><i>>> // for AWT where
pageable is not an instance of OpenBook,</i><i><br>
</i><i>>> // we need to save
paper info</i><i><br>
</i><i> >> this.attributes =
attributes;</i><i><br>
</i><i> >> }</i><i><br>
</i>
<p><i> </i>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.<br>
</p>
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. <br>
<br>
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)<br>
<br>
BTW, a regression testcase (even if it is
manual) should be created with the fix.<br>
<br>
Regards<br>
Prasanta<br>
<div class="moz-cite-prefix">On 3/17/2017 10:59
PM, Anton Litvinov wrote:<br>
</div>
<blockquote
cite="mid:17a96ade-ea01-95bd-728a-ec3106d1fbfa@oracle.com"
type="cite">Hello, <br>
<br>
Could you please review the following fix for
the bug. <br>
<br>
Bug: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
\
href="https://bugs.openjdk.java.net/browse/JDK-8167102">https://bugs.openjdk.java.net/browse/JDK-8167102</a>
<br>
Webrev: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
\
href="http://cr.openjdk.java.net/%7Ealitvinov/8167102/jdk9/webrev.00">http://cr.openjdk.java.net/~alitvinov/8167102/jdk9/webrev.00</a>
<br>
<br>
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". <br>
<br>
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. <br>
<br>
THE SOLUTION: <br>
Although it is clearly stated in Java Platform
SE 8 API Specification (documentation of the
method
"PrinterJob.print(PrintRequestAttributeSet)")
<br>
Specification URL: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://docs.oracle.com/javase/8/docs/api/java/awt/print/PrinterJob.html#print-ja \
vax.print.attribute.PrintRequestAttributeSet">http://docs.oracle.com/javase/8/docs/api \
/java/awt/print/PrinterJob.html#print-javax.print.attribute.PrintRequestAttributeSet</a>-<br>
<br>
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". <br>
<br>
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. <br>
<br>
Thank you, <br>
Anton <br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic