<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    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-javax.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>
  </body>
</html>