[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] [9] RFR 4987884: PrinterJob carries over some values between calls to print.
From:       Prasanta Sadhukhan <prasanta.sadhukhan () oracle ! com>
Date:       2016-11-10 10:56:04
Message-ID: cb0dc2b3-a012-220b-efe8-6c4f12f7038e () oracle ! com
[Download RAW message or body]

Modified webrev to ensure attributes set are cleared before return so 
that subsequent print() call start off with clean-slate.

http://cr.openjdk.java.net/~psadhukhan/4987884/webrev.03/

One thing is, due to this contract for print(PrintRequestAttributes) 
which specifies
/"some attributes may be set directly on the PrinterJob by equivalent 
method calls, (for example), copies: //|setcopies(int)|//, job name: 
//|setJobName(String)|//....
//If a supported attribute-value is specified in this attribute set, it 
will take precedence over the API settings for this print() operation 
only. /"

setCopies, setJobName() attributes can also be set via APIs so those 
attributes are cleared only if set via attributeSet and not when set via 
APIs because in that case, it should be valid across print() calls.

Regards
Prasanta
On 11/5/2016 12:00 AM, Phil Race wrote:
> I've always considered it unfortunate that PrinterJob did not
> prohibit print() being called twice from day one.
> It is not something I would encourage apps to do and I
> would not be surprised to see this as the source of odd bugs.
> If I could I would go update all the implementation code to
> throw IllegalStateException if you call print() twice but
> I suspect it may break code that does this for some reason
> that in all probability is historical (like "I'm saving 
> CPU/memory/whatever")
> So I don't want to put anything in the docs that imply that
> it is something that an app might be encouraged to do.
> 
> i.e. in the test the app does
> 
> aset.add(new PageRanges(1,1));
> ..
> pjob.print(aset);
> 
> ...
> pjob.print();
> ...
> 
> PageRanges is *an example* of what should not be carried over.
> 
> So I think this (and the initial v0 fix) is missing the point of the bug
> which is "The attribute set passed in a by the app should not be 
> remembered
> across calls."
> 
> In fact the proposed doc. arguably legitimises the opposite.
> 1427 * If {@code print(PrintRequestAttributeSet attributes)} is called 
> before
> 1428 * this method for the same PrinterJob, it is expected to 
> use/carryover the
> 1429 * same attributes set passed to
> 1430 * {@code print(PrintRequestAttributeSet attributes)}
> 
> What I was asking for in the bug report was that *any* settings
> in that first print(aset) call should not be preserved as internal
> state once print(aset) returns. So it seems like a clean-up-before-return
> rather than a clean-up-on-reentry.
> 
> This requires careful attention to the contract of this method
> 
> https://docs.oracle.com/javase/8/docs/api/java/awt/print/PrinterJob.html#print-javax.print.attribute.PrintRequestAttributeSet-
>  
> Now since this bug was filed we added the ability to use the native
> dialogs with an attribute set, so there may be some additional
> or deeper pollution of the native state which may make this harder
> to achieve. I don't have a ready answer for that but it does not
> mean that we should give up with out a proper study of this ..
> 
> -phil.
> 
> On 09/29/2016 02:39 AM, Prasanta Sadhukhan wrote:
> > 
> > 
> > On 9/12/2016 12:33 PM, Prasanta Sadhukhan wrote:
> > > Hi All,
> > > 
> > > Updated the spec to clarify that it is expected to use the same 
> > > attribute set passed to print(PrintRequestAttributeSet) if it is 
> > > called before no-args print() method for same PrinterJob.
> > > http://cr.openjdk.java.net/~psadhukhan/4987884/webrev.01/
> > > 
> > Updated PrinterJob spec as opposed to inner class RasterPrinterJob so 
> > that javadocs is updated.
> > http://cr.openjdk.java.net/~psadhukhan/4987884/webrev.02/
> > 
> > Regards
> > Prasanta
> > > Regards
> > > Prasanta
> > > On 8/29/2016 1:41 PM, Prasanta Sadhukhan wrote:
> > > > Hi All,
> > > > 
> > > > If we call
> > > > -----------------
> > > > /        pageFormat = job.pageDialog(pageFormat);//
> > > > //        pageFormat.setPaper(paper);//
> > > > //        job.setPrintable(p, pageFormat);//
> > > > ////
> > > > //        try {//
> > > > //            job.printDialog();//
> > > > //            job.print();//
> > > > //        } catch (PrinterException e) {}//
> > > > /-------------------
> > > > then these attributes are added
> > > > Media, OrientationRequested, MediaPrintableArea, Chromaticity, 
> > > > PrinterResolution, Copies, SheetCollate, Sides
> > > > even if we do not change any fields
> > > > and PageRanges attribute is added if we change page selection from 
> > > > "All" to "Pages".
> > > > 
> > > > In light of that, I do not think we can remove PageRanges attribute 
> > > > from no-args print() method as that will remove user selection.
> > > > Or for that matter, any attributes as those can be set via 
> > > > pageDialog or printDialog.
> > > > 
> > > > I guess we need to mark this as "Wont fix" . spec also is not clear 
> > > > if we are not supposed to honour any attributes already set, if we 
> > > > call no-args print() method.
> > > > Any comments?
> > > > 
> > > > Regards
> > > > Prasanta
> > > > On 7/5/2016 10:52 AM, Prasanta Sadhukhan wrote:
> > > > > Hi All,
> > > > > 
> > > > > Please review a fix whereby it is seen that the PageRanges 
> > > > > attribute passed to 1st print() call is inadvertently been carried 
> > > > > forward to 2nd print
> > > > > if some program tries calling in this sequence
> > > > > ----------
> > > > > /PrinterJob pj = PrinterJob.getPrinterJob();//
> > > > > //pj.setPageable(..)//
> > > > > //pj.print(..)//
> > > > > //pj.setPageable(..)//
> > > > > //pj.print(..)
> > > > > ------------
> > > > > /
> > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-4987884/
> > > > > /webrev: http://cr.openjdk.java.net/~psadhukhan/4987884/webrev.00//
> > > > > 
> > > > > /This is because no-args print() uses an internal attribute set 
> > > > > which is presumed to carry over information not supplied
> > > > > by the client directly but created during a call to no-args 
> > > > > printDialog() or during call to setPrintable() called via 
> > > > > setAttributes().
> > > > > but it apparently seems, we should not be remembering PageRange 
> > > > > attribute between calls to print() as it will result in wrong page 
> > > > > numbers being printed.
> > > > > 
> > > > > Proposed fix to remove the PageRanges attribute from no-args 
> > > > > print() between print() calls.
> > > > > 
> > > > > Regards
> > > > > Prasanta
> > > > 
> > > 
> > 
> 


[Attachment #3 (text/html)]

<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <p>Modified webrev to ensure attributes set are cleared before
      return so that subsequent print() call start off with clean-slate.</p>
    <p><a class="moz-txt-link-freetext" \
href="http://cr.openjdk.java.net/~psadhukhan/4987884/webrev.03/">http://cr.openjdk.java.net/~psadhukhan/4987884/webrev.03/</a><br>
  </p>
    One thing is, due to this contract for print(PrintRequestAttributes)
    which specifies<br>
    <i>"some attributes may be set directly on the PrinterJob by
      equivalent method calls, (for example), copies: \
</i><i><code>setcopies(int)</code></i><i>,  job name: \
</i><i><code>setJobName(String)</code></i><i>....<br>  </i><i>If a supported \
attribute-value is specified in this attribute  set, it will take precedence over the \
API settings for this  print() operation only. </i>"<br>
    <br>
    setCopies, setJobName() attributes can also be set via APIs so those
    attributes are cleared only if set via attributeSet and not when set
    via APIs because in that case, it should be valid across print()
    calls.<br>
    <br>
    Regards<br>
    Prasanta<br>
    <div class="moz-cite-prefix">On 11/5/2016 12:00 AM, Phil Race wrote:<br>
    </div>
    <blockquote
      cite="mid:3d5357b0-d76b-c636-9438-de55f93236b3@oracle.com"
      type="cite">
      <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
      I've always considered it unfortunate that PrinterJob did not<br>
      prohibit print() being called twice from day one.<br>
      It is not something I would encourage apps to do and I<br>
      would not be surprised to see this as the source of odd bugs.<br>
      If I could I would go update all the implementation code to<br>
      throw IllegalStateException if you call print() twice but<br>
      I suspect it may break code that does this for some reason<br>
      that in all probability is historical (like "I'm saving
      CPU/memory/whatever")<br>
      So I don't want to put anything in the docs that imply that<br>
      it is something that an app might be encouraged to do.<br>
      <br>
      i.e. in the test the app does<br>
      <br>
      aset.add(new PageRanges(1,1));<br>
      ..<br>
      pjob.print(aset);<br>
      <br>
      ...<br>
      pjob.print();<br>
      ...<br>
      <br>
      PageRanges is *an example* of what should not be carried over.<br>
      <br>
      So I think this (and the initial v0 fix) is missing the point of
      the bug<br>
      which is "The attribute set passed in a by the app should not be
      remembered<br>
      across calls."<br>
      <br>
      In fact the proposed doc. arguably legitimises the opposite.<br>
      <pre><span class="new">1427      * If {@code print(PrintRequestAttributeSet \
attributes)} is called before</span> <span class="new">1428      * this method for \
the same PrinterJob, it is expected to use/carryover the </span> <span \
class="new">1429      * same attributes set passed to </span> <span class="new">1430  \
* {@code print(PrintRequestAttributeSet attributes)}      </span></pre>  <br>
      What I was asking for in the bug report was that *any* settings<br>
      in that first print(aset) call should not be preserved as internal<br>
      state once print(aset) returns. So it seems like a
      clean-up-before-return<br>
      rather than a clean-up-on-reentry.<br>
      <br>
      This requires careful attention to the contract of this method<br>
      <br>
      <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://docs.oracle.com/javase/8/docs/api/java/awt/print/PrinterJob.html#print-j \
avax.print.attribute.PrintRequestAttributeSet">https://docs.oracle.com/javase/8/docs/a \
pi/java/awt/print/PrinterJob.html#print-javax.print.attribute.PrintRequestAttributeSet</a>-<br>
  <br>
      Now since this bug was filed we added the ability to use the
      native<br>
      dialogs with an attribute set, so there may be some additional<br>
      or deeper pollution of the native state which may make this harder<br>
      to achieve. I don't have a ready answer for that but it does not<br>
      mean that we should give up with out a proper study of this ..<br>
      <br>
      -phil.<br>
      <br>
      <div class="moz-cite-prefix">On 09/29/2016 02:39 AM, Prasanta
        Sadhukhan wrote:<br>
      </div>
      <blockquote cite="mid:57ECE158.5050900@oracle.com" type="cite">
        <meta content="text/html; charset=utf-8"
          http-equiv="Content-Type">
        <br>
        <br>
        <div class="moz-cite-prefix">On 9/12/2016 12:33 PM, Prasanta
          Sadhukhan wrote:<br>
        </div>
        <blockquote cite="mid:57D65324.9040809@oracle.com" type="cite">
          <meta content="text/html; charset=utf-8"
            http-equiv="Content-Type">
          Hi All,<br>
          <br>
          Updated the spec to clarify that it is expected to use the
          same attribute set passed to print(PrintRequestAttributeSet)
          if it is called before no-args print() method for same
          PrinterJob.<br>
          <a moz-do-not-send="true" class="moz-txt-link-freetext"
            href="http://cr.openjdk.java.net/%7Epsadhukhan/4987884/webrev.01/">http://cr.openjdk.java.net/~psadhukhan/4987884/webrev.01/</a><br>
  <br>
        </blockquote>
        Updated PrinterJob spec as opposed to inner class
        RasterPrinterJob so that javadocs is updated.<br>
        <a moz-do-not-send="true" class="moz-txt-link-freetext"
          href="http://cr.openjdk.java.net/%7Epsadhukhan/4987884/webrev.02/">http://cr.openjdk.java.net/~psadhukhan/4987884/webrev.02/</a><br>
  <br>
        Regards<br>
        Prasanta<br>
        <blockquote cite="mid:57D65324.9040809@oracle.com" type="cite">
          Regards<br>
          Prasanta<br>
          <div class="moz-cite-prefix">On 8/29/2016 1:41 PM, Prasanta
            Sadhukhan wrote:<br>
          </div>
          <blockquote cite="mid:57C3EE2E.3060400@oracle.com" type="cite">
            <meta content="text/html; charset=utf-8"
              http-equiv="Content-Type">
            Hi All,<br>
            <br>
            If we call <br>
            -----------------<br>
            <i>               pageFormat = job.pageDialog(pageFormat);</i><i><br>
            </i><i>               pageFormat.setPaper(paper);</i><i><br>
            </i><i>               job.setPrintable(p, pageFormat);</i><i><br>
            </i><i>               </i><i><br>
            </i><i>               try {</i><i><br>
            </i><i>                       job.printDialog();</i><i><br>
            </i><i>                       job.print();</i><i><br>
            </i><i>               } catch (PrinterException e) {}</i><i><br>
            </i>-------------------<br>
            then these attributes are added <br>
            Media, OrientationRequested, MediaPrintableArea,
            Chromaticity, PrinterResolution, Copies, SheetCollate, Sides
            <br>
            even if we do not change any fields<br>
            and PageRanges attribute is added if we change page
            selection from "All" to "Pages".<br>
            <br>
            In light of that, I do not think we can remove PageRanges
            attribute from no-args print() method as that will remove
            user selection.<br>
              Or for that matter, any attributes as those can be set via
            pageDialog or printDialog.<br>
            <br>
            I guess we need to mark this as "Wont fix" . spec also is
            not clear if we are not supposed to honour any attributes
            already set, if we call no-args print() method.<br>
            Any comments? <br>
            <br>
            Regards<br>
            Prasanta<br>
            <div class="moz-cite-prefix">On 7/5/2016 10:52 AM, Prasanta
              Sadhukhan wrote:<br>
            </div>
            <blockquote cite="mid:577B43FA.5080009@oracle.com"
              type="cite">
              <meta http-equiv="content-type" content="text/html;
                charset=utf-8">
              Hi All,<br>
              <br>
              Please review a fix whereby it is seen that the PageRanges
              attribute passed to 1st print() call is inadvertently been
              carried forward to 2nd print<br>
              if some program tries calling in this sequence<br>
              ----------<br>
              <i>PrinterJob pj = PrinterJob.getPrinterJob();</i><i><br>
              </i><i> pj.setPageable(..)</i><i><br>
              </i><i> pj.print(..)</i><i><br>
              </i><i> pj.setPageable(..)</i><i><br>
              </i><i> pj.print(..)<br>
                ------------<br>
              </i><br>
              Bug: <a moz-do-not-send="true"
                class="moz-txt-link-freetext"
                href="https://bugs.openjdk.java.net/browse/JDK-4987884">https://bugs.openjdk.java.net/browse/JDK-4987884</a><i><br>
  </i>webrev: <a moz-do-not-send="true"
                class="moz-txt-link-freetext"
                href="http://cr.openjdk.java.net/%7Epsadhukhan/4987884/webrev.00/">http://cr.openjdk.java.net/~psadhukhan/4987884/webrev.00/</a><i><br>
  <br>
              </i>This is because no-args print() uses an internal
              attribute set which is presumed to carry over information
              not supplied<br>
              by the client directly but created during a call to
              no-args printDialog() or during call to setPrintable()
              called via setAttributes().<br>
              but it apparently seems, we should not be remembering
              PageRange attribute between calls to print() as it will
              result in wrong page numbers being printed.<br>
              <br>
              Proposed fix to remove the PageRanges attribute from
              no-args print() between print() calls.<br>
              <br>
              Regards<br>
              Prasanta<br>
            </blockquote>
            <br>
          </blockquote>
          <br>
        </blockquote>
        <br>
      </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