<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    +1<br>
    <br>
    -phil.<br>
    <br>
    On 7/26/20, 9:20 PM, Prasanta Sadhukhan wrote:
    <blockquote
      cite="mid:e6e2a846-7aae-0de2-d0bd-0bef3dcf949c@oracle.com"
      type="cite">
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
      <p>Hi Phil,<br>
      </p>
      <div class="moz-cite-prefix">On 24-Jul-20 3:48 AM, Philip Race
        wrote:<br>
      </div>
      <blockquote type="cite" cite="mid:5F1A0CCF.8080400@oracle.com">
        <meta content="text/html; charset=UTF-8"
          http-equiv="Content-Type">
        So the bug is that directly calling ServiceUI.printDialog() was
        enabling a non-functional properties button on just the Windows
        platform and you are adding an extra condition to prevent that<br>
        without harming the goal of JDK-4673406 to have it enabled and
        functional when invoked by<br>
        java.awt.PrinterJob attached to a Windows printer. That is much
        more important than the<br>
        bug being fixed here so I'd like definite confirmation of that.<br>
      </blockquote>
      Yes, normal PrinterJob displaying native and cross-platform dialog
      works as before, showing Properties dialog when clicked on it on
      windows. It's only ServiceUI Dialog that disables Properties
      button when no PrinterJob is associated with it.<br>
      <blockquote type="cite" cite="mid:5F1A0CCF.8080400@oracle.com">
        Also since it is an *extra* condition may I assume we do not
        enable this for a PrinterJob<br>
        on mac or linux - the functionality was windows only.<br>
        <br>
      </blockquote>
      <p>Yes, it does not work for mac/linux as we have the condition <br>
      </p>
      <pre style="background-color:#ffffff;color:#000000;font-family:'Courier New';font-size:7.2pt;"><span style="color:#660e7a;font-weight:bold;">btnProperties</span>.setEnabled(<span style="color:#660e7a;font-weight:bold;">uiFactory </span>!= <span style="color:#000080;font-weight:bold;">null </span>&&  job != <span style="color:#000080;font-weight:bold;">null</span>);</pre>
      <p>and uiFactory is null for mac/linux so the button is disabled
        for those platforms.<br>
      </p>
      <p><a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://hg.openjdk.java.net/jdk/client/file/754ec520eb4a/src/java.desktop/unix/classes/sun/print/IPPPrintService.java#l1637">http://hg.openjdk.java.net/jdk/client/file/754ec520eb4a/src/java.desktop/unix/classes/sun/print/IPPPrintService.java#l1637</a></p>
      <blockquote type="cite" cite="mid:5F1A0CCF.8080400@oracle.com"> I
        think the test should not fail if the button is enabled. It
        should fail only if<br>
        the button is enabled *and does nothing*.<br>
      </blockquote>
      <p>OK. Modified webrev with test instructions updated</p>
      <p><a moz-do-not-send="true" class="moz-txt-link-freetext"
          href="http://cr.openjdk.java.net/%7Epsadhukhan/8246742/webrev.2/">http://cr.openjdk.java.net/~psadhukhan/8246742/webrev.2/</a><br>
      </p>
      <blockquote type="cite" cite="mid:5F1A0CCF.8080400@oracle.com"> <br>
        -phil.<br>
        <br>
        On 7/20/20, 8:15 AM, Prasanta Sadhukhan wrote:
        <blockquote
          cite="mid:5298a7b3-acf4-f635-d728-4761a31ac6fa@oracle.com"
          type="cite">
          <meta http-equiv="Content-Type" content="text/html;
            charset=UTF-8">
          <p>Actually, I was a bit wrong in construing that the button
            should just be disabled for ServiceUI.printDialog if
            ServiceUIFactory is null & getUI(MAIN_ROLE...) is null.
            <br>
          </p>
          <p>The "properties" dialog is used for another use-case which
            is for cross-platform dialog also ie, when we have a printer
            job created using (in which case also getUI() will be null
            for windows)<br>
          </p>
          <pre style="background-color:#ffffff;color:#000000;font-family:'Courier New';font-size:7.2pt;">PrintRequestAttributeSet pSet =  <span style="color:#000080;font-weight:bold;">new </span>HashPrintRequestAttributeSet();
pSet.add(DialogTypeSelection.<span style="color:#660e7a;font-weight:bold;font-style:italic;">COMMON</span>);
pj.printDialog(pSet);</pre>
          <p>And, as per JDK-4673406, it  is likely that supporting this
            <i>"properties" sheet will only be possible where there is
              already a job with which</i><i><br>
            </i><i> to make the association. ie using
              java.awt.PrinterJob.printDialog(AttributeSet) and not for
              direct uses with javax.print.ServiceUI.printDialog(..)</i>"</p>
          <div class="moz-cite-prefix">So, ideally, we should be
            checking if we have a printer job in addition to
            ServiceUIFactory being not null to allow creation of
            association between printer properties and the printer job <br>
          </div>
          <div class="moz-cite-prefix">so if a PrinterJob cross-platform
            printer-dialog is created, then we should support
            "Properties" dialog. <br>
          </div>
          <div class="moz-cite-prefix">If we directly use
            javax.print.ServiceUI.printDialog(..), then in that case no
            printer job will be created, so properties dialog can be
            disabled for that use-case.</div>
          <div class="moz-cite-prefix"><br>
          </div>
          <div class="moz-cite-prefix">Modified webrev: <a
              moz-do-not-send="true" class="moz-txt-link-freetext"
              href="http://cr.openjdk.java.net/%7Epsadhukhan/8246742/webrev.1/">http://cr.openjdk.java.net/~psadhukhan/8246742/webrev.1/</a></div>
          <div class="moz-cite-prefix"><br>
          </div>
          <div class="moz-cite-prefix">Regards</div>
          <div class="moz-cite-prefix">Prasanta<br>
          </div>
          <div class="moz-cite-prefix">On 20-Jul-20 3:53 PM, Jayathirth
            D v wrote:<br>
          </div>
          <blockquote type="cite"
            cite="mid:384E7712-4D11-431C-9F4C-FC79188A16F4@ORACLE.COM">
            <meta http-equiv="Content-Type" content="text/html;
              charset=UTF-8">
            Hi Prasanta,
            <div class=""><br class="">
            </div>
            <div class="">Is just checking for dialog is null fine or we
              also need to verify DocumentProptiesUI in our case? As
              done in else case when dialog is null at <a
href="http://hg.openjdk.java.net/jdk/client/file/fabf11c3a8ca/src/java.desktop/share/classes/sun/print/ServiceDialog.java#l801"
                class="" moz-do-not-send="true">http://hg.openjdk.java.net/jdk/client/file/fabf11c3a8ca/src/java.desktop/share/classes/sun/print/ServiceDialog.java#l801</a> ?</div>
            <div class=""><br class="">
            </div>
            <div class="">I also see that Win32ServiceUIFactory</div>
            <div class="">.getUI() doesnt return null in some specific
              DocumentsPropertyUI at <a
href="http://hg.openjdk.java.net/jdk/client/file/754ec520eb4a/src/java.desktop/windows/classes/sun/print/Win32PrintService.java#l1692"
                class="" moz-do-not-send="true">http://hg.openjdk.java.net/jdk/client/file/754ec520eb4a/src/java.desktop/windows/classes/sun/print/Win32PrintService.java#l1692</a> </div>
            <div class=""><br class="">
            </div>
            <div class="">Thanks,</div>
            <div class="">Jay<br class="">
              <div><br class="">
                <blockquote type="cite" class="">
                  <div class="">On 17-Jul-2020, at 11:22 AM, Prasanta
                    Sadhukhan <<a
                      href="mailto:prasanta.sadhukhan@oracle.com"
                      class="" moz-do-not-send="true">prasanta.sadhukhan@oracle.com</a>>

                    wrote:</div>
                  <br class="Apple-interchange-newline">
                  <div class="">
                    <meta http-equiv="Content-Type" content="text/html;
                      charset=UTF-8" class="">
                    <div class="">
                      <p class="">Hi Jay,</p>
                      <p class="">I am using the same methodology used
                        to determine whether to show the properties
                        dialog when button-clicked on it (which is not
                        to show if dialog is null) as per<br class="">
                      </p>
                      <p class=""><a class="moz-txt-link-freetext"
href="http://hg.openjdk.java.net/jdk/client/file/fabf11c3a8ca/src/java.desktop/share/classes/sun/print/ServiceDialog.java#l793"
                          moz-do-not-send="true">http://hg.openjdk.java.net/jdk/client/file/fabf11c3a8ca/src/java.desktop/share/classes/sun/print/ServiceDialog.java#l793</a></p>
                      <p class="">So, if we cannot show the dialog using
                        that mechanism, we can enable/disable the button
                        itself beforehand using that check.</p>
                      Regards<br class="">
                      Prasanta.<br class="">
                      <div class="moz-cite-prefix">On 16-Jul-20 9:26 PM,
                        Jayathirth D v wrote:<br class="">
                      </div>
                      <blockquote type="cite"
                        cite="mid:6FFB1DCE-E1F1-490C-A888-6A1FE3490A94@ORACLE.COM"
                        class="">
                        <meta http-equiv="Content-Type"
                          content="text/html; charset=UTF-8" class="">
                        Hi Prasanta,
                        <div class=""><br class="">
                        </div>
                        <div class="">I tested the fix in Mac and
                          Windows and it works as mentioned.</div>
                        <div class=""><br class="">
                        </div>
                        <div class="">In <a
href="http://hg.openjdk.java.net/jdk/client/file/754ec520eb4a/src/java.desktop/windows/classes/sun/print/Win32PrintService.java#l1688"
                            class="" moz-do-not-send="true">http://hg.openjdk.java.net/jdk/client/file/754ec520eb4a/src/java.desktop/windows/classes/sun/print/Win32PrintService.java#l1688</a> we

                          are returning null when “role <=
                          ServiceUIFactory.MAIN_UIROLE”. So it covers 3
                          roles “MAIN_UIROLE”, “ADMIN_UIROLE” and
                          “ABOUT_UIROLE”.</div>
                        <div class=""><br class="">
                        </div>
                        <div class="">In your webrev we are checking
                          only for “<font class="">MAIN_UIROLE”.
                            Is creation of <span style="caret-color:
                              rgb(0, 0, 0);" class="">“</span></font>JDIALOG_UI”

                          restricted only to “<span style="caret-color:
                            rgb(0, 0, 0);" class="">MAIN_UIROLE</span>”?</div>
                        <div class=""><br class="">
                        </div>
                        <div class="">Thanks,</div>
                        <div class="">Jay</div>
                        <div class="">
                          <div class=""><br class="">
                            <blockquote type="cite" class="">
                              <div class="">On 15-Jul-2020, at 6:27 PM,
                                Prasanta Sadhukhan <<a
                                  href="mailto:prasanta.sadhukhan@oracle.com"
                                  class="" moz-do-not-send="true">prasanta.sadhukhan@oracle.com</a>>

                                wrote:</div>
                              <br class="Apple-interchange-newline">
                              <div class="">
                                <meta http-equiv="Content-Type"
                                  content="text/html; charset=UTF-8"
                                  class="">
                                <div class="">
                                  <p class="">Any reviewer for this?<br
                                      class="">
                                  </p>
                                  <div class="moz-cite-prefix">On
                                    09-Jul-20 1:10 PM, Prasanta
                                    Sadhukhan wrote:<br class="">
                                  </div>
                                  <blockquote type="cite"
                                    cite="mid:05a59b29-c359-58a0-daa2-e572e52d3e6b@oracle.com"
                                    class="">
                                    <meta http-equiv="content-type"
                                      content="text/html; charset=UTF-8"
                                      class="">
                                    <p class="">Hi All,</p>
                                    <p class="">Please review a fix for
                                      an issue where "Properties" button
                                      in ServiceUI.printDialog is
                                      enabled in windows but clicking it
                                      doesn't show any dialog.</p>
                                    <p class="">According to <a
                                        href="https://bugs.openjdk.java.net/browse/JDK-4673406"
                                        title="RFE: Java Printing:
                                        Provide a way to display win32
                                        printer driver's dialog"
                                        class="issue-link"
                                        data-issue-key="JDK-4673406"
                                        moz-do-not-send="true"><strike
                                          class="">JDK-4673406</strike></a>,
                                      the properties dialog isn't
                                      supported for direct uses with
                                      javax.print.ServiceUI.printDialog,
                                      so it makes sense to disable this
                                      properies button for this usecase.</p>
                                    <p class="">This button is disabled
                                      in linux,mac already. This is
                                      because, as per<br class="">
                                    </p>
                                    <p class=""><a
                                        class="moz-txt-link-freetext"
href="http://hg.openjdk.java.net/jdk/client/annotate/754ec520eb4a/src/java.desktop/share/classes/sun/print/ServiceDialog.java#l964"
                                        moz-do-not-send="true">http://hg.openjdk.java.net/jdk/client/annotate/754ec520eb4a/src/java.desktop/share/classes/sun/print/ServiceDialog.java#l964</a></p>
                                    <p class="">the button is disabled
                                      if ServiceUIFactory is null and
                                      for linux/mac, it is null</p>
                                    <a class="moz-txt-link-freetext"
href="http://hg.openjdk.java.net/jdk/client/file/754ec520eb4a/src/java.desktop/unix/classes/sun/print/IPPPrintService.java#l1637"
                                      moz-do-not-send="true">http://hg.openjdk.java.net/jdk/client/file/754ec520eb4a/src/java.desktop/unix/classes/sun/print/IPPPrintService.java#l1637</a><br
                                      class="">
                                    <a class="moz-txt-link-freetext"
href="http://hg.openjdk.java.net/jdk/client/file/754ec520eb4a/src/java.desktop/share/classes/sun/print/PSStreamPrintService.java#l490"
                                      moz-do-not-send="true">http://hg.openjdk.java.net/jdk/client/file/754ec520eb4a/src/java.desktop/share/classes/sun/print/PSStreamPrintService.java#l490</a>
                                    <p class="">but for windows, it
                                      created "Win32ServiceUIFactory"
                                      but it does not handle the
                                      properties dialog if "role"
                                      requested to be performed by
                                      ServiceUI is <=
                                      ServiceUIFactory.<span
                                        style="color:#660e7a;font-weight:bold;font-style:italic;"
                                        class="">MAIN_UIROLE</span></p>
                                    <p class=""><span
                                        style="color:#660e7a;font-weight:bold;font-style:italic;"
                                        class=""></span>[<a
                                        class="moz-txt-link-freetext"
href="http://hg.openjdk.java.net/jdk/client/file/754ec520eb4a/src/java.desktop/windows/classes/sun/print/Win32PrintService.java#l1688"
                                        moz-do-not-send="true">http://hg.openjdk.java.net/jdk/client/file/754ec520eb4a/src/java.desktop/windows/classes/sun/print/Win32PrintService.java#l1688</a>]<br
                                        class="">
                                    </p>
                                    <p class="">Proposed fix is to make
                                      sure this role is accounted for in
                                      the buttonProperties enabling
                                      check.</p>
                                    <p class="">Bug: <a
                                        class="moz-txt-link-freetext"
                                        href="https://bugs.openjdk.java.net/browse/JDK-8246742"
                                        moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8246742</a></p>
                                    <p class="">webrev: <a
                                        class="moz-txt-link-freetext"
                                        href="http://cr.openjdk.java.net/%7Epsadhukhan/8246742/webrev.0/"
                                        moz-do-not-send="true">http://cr.openjdk.java.net/~psadhukhan/8246742/webrev.0/</a></p>
                                    Regards<br class="">
                                    Prasanta<br class="">
                                    <span style="color: rgb(102, 14,
                                      122);" class=""></span> </blockquote>
                                </div>
                              </div>
                            </blockquote>
                          </div>
                          <br class="">
                        </div>
                      </blockquote>
                    </div>
                  </div>
                </blockquote>
              </div>
              <br class="">
            </div>
          </blockquote>
        </blockquote>
      </blockquote>
    </blockquote>
  </body>
</html>