[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-2d-dev
Subject: Re: [OpenJDK 2D-Dev] RFR JDK-8246742: ServiceUI.printDialog does not support properties dialog
From: Philip Race <philip.race () oracle ! com>
Date: 2020-07-27 15:04:13
Message-ID: 5F1EECED.8070601 () oracle ! com
[Download RAW message or body]
+1
-phil.
On 7/26/20, 9:20 PM, Prasanta Sadhukhan wrote:
>
> Hi Phil,
>
> On 24-Jul-20 3:48 AM, Philip Race wrote:
> > 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
> > without harming the goal of JDK-4673406 to have it enabled and
> > functional when invoked by
> > java.awt.PrinterJob attached to a Windows printer. That is much more
> > important than the
> > bug being fixed here so I'd like definite confirmation of that.
> 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.
> > Also since it is an *extra* condition may I assume we do not enable
> > this for a PrinterJob
> > on mac or linux - the functionality was windows only.
> >
> Yes, it does not work for mac/linux as we have the condition
>
> btnProperties.setEnabled(uiFactory!=null&& job !=null);
>
> and uiFactory is null for mac/linux so the button is disabled for
> those platforms.
>
> http://hg.openjdk.java.net/jdk/client/file/754ec520eb4a/src/java.desktop/unix/classes/sun/print/IPPPrintService.java#l1637
>
> > I think the test should not fail if the button is enabled. It should
> > fail only if
> > the button is enabled *and does nothing*.
>
> OK. Modified webrev with test instructions updated
>
> http://cr.openjdk.java.net/~psadhukhan/8246742/webrev.2/
>
> >
> > -phil.
> >
> > On 7/20/20, 8:15 AM, Prasanta Sadhukhan wrote:
> > >
> > > 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.
> > >
> > > 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)
> > >
> > > PrintRequestAttributeSet pSet =newHashPrintRequestAttributeSet();
> > > pSet.add(DialogTypeSelection.COMMON);
> > > pj.printDialog(pSet);
> > >
> > > And, as per JDK-4673406, it is likely that supporting this
> > > /"properties" sheet will only be possible where there is already a
> > > job with which//
> > > //to make the association. ie using
> > > java.awt.PrinterJob.printDialog(AttributeSet) and not for direct
> > > uses with javax.print.ServiceUI.printDialog(..)/"
> > >
> > > 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
> > > so if a PrinterJob cross-platform printer-dialog is created, then we
> > > should support "Properties" dialog.
> > > 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.
> > >
> > > Modified webrev:
> > > http://cr.openjdk.java.net/~psadhukhan/8246742/webrev.1/
> > >
> > > Regards
> > > Prasanta
> > > On 20-Jul-20 3:53 PM, Jayathirth D v wrote:
> > > > Hi Prasanta,
> > > >
> > > > 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
> > > > http://hg.openjdk.java.net/jdk/client/file/fabf11c3a8ca/src/java.desktop/share/classes/sun/print/ServiceDialog.java#l801 \
> > > > ?
> > > > I also see that Win32ServiceUIFactory
> > > > .getUI() doesnt return null in some specific DocumentsPropertyUI at
> > > > http://hg.openjdk.java.net/jdk/client/file/754ec520eb4a/src/java.desktop/windows/classes/sun/print/Win32PrintService.java#l1692 \
> > > >
> > > >
> > > > Thanks,
> > > > Jay
> > > >
> > > > > On 17-Jul-2020, at 11:22 AM, Prasanta Sadhukhan
> > > > > <prasanta.sadhukhan@oracle.com
> > > > > <mailto:prasanta.sadhukhan@oracle.com>> wrote:
> > > > >
> > > > > Hi Jay,
> > > > >
> > > > > 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
> > > > >
> > > > > http://hg.openjdk.java.net/jdk/client/file/fabf11c3a8ca/src/java.desktop/share/classes/sun/print/ServiceDialog.java#l793
> > > > >
> > > > > So, if we cannot show the dialog using that mechanism, we can
> > > > > enable/disable the button itself beforehand using that check.
> > > > >
> > > > > Regards
> > > > > Prasanta.
> > > > > On 16-Jul-20 9:26 PM, Jayathirth D v wrote:
> > > > > > Hi Prasanta,
> > > > > >
> > > > > > I tested the fix in Mac and Windows and it works as mentioned.
> > > > > >
> > > > > > In
> > > > > > http://hg.openjdk.java.net/jdk/client/file/754ec520eb4a/src/java.desktop/windows/classes/sun/print/Win32PrintService.java#l1688 \
> > > > > > we are returning null when "role <= ServiceUIFactory.MAIN_UIROLE".
> > > > > > So it covers 3 roles "MAIN_UIROLE", "ADMIN_UIROLE" and
> > > > > > "ABOUT_UIROLE".
> > > > > >
> > > > > > In your webrev we are checking only for "MAIN_UIROLE".
> > > > > > Is creation of "JDIALOG_UI" restricted only to "MAIN_UIROLE"?
> > > > > >
> > > > > > Thanks,
> > > > > > Jay
> > > > > >
> > > > > > > On 15-Jul-2020, at 6:27 PM, Prasanta Sadhukhan
> > > > > > > <prasanta.sadhukhan@oracle.com
> > > > > > > <mailto:prasanta.sadhukhan@oracle.com>> wrote:
> > > > > > >
> > > > > > > Any reviewer for this?
> > > > > > >
> > > > > > > On 09-Jul-20 1:10 PM, Prasanta Sadhukhan wrote:
> > > > > > > >
> > > > > > > > Hi All,
> > > > > > > >
> > > > > > > > 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.
> > > > > > > >
> > > > > > > > According to JDK-4673406
> > > > > > > > <https://bugs.openjdk.java.net/browse/JDK-4673406>, 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.
> > > > > > > >
> > > > > > > > This button is disabled in linux,mac already. This is because,
> > > > > > > > as per
> > > > > > > >
> > > > > > > > http://hg.openjdk.java.net/jdk/client/annotate/754ec520eb4a/src/java.desktop/share/classes/sun/print/ServiceDialog.java#l964
> > > > > > > >
> > > > > > > > the button is disabled if ServiceUIFactory is null and for
> > > > > > > > linux/mac, it is null
> > > > > > > >
> > > > > > > > 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/share/classes/sun/print/PSStreamPrintService.java#l490 \
> > > > > > > >
> > > > > > > >
> > > > > > > > but for windows, it created "Win32ServiceUIFactory" but it does
> > > > > > > > not handle the properties dialog if "role" requested to be
> > > > > > > > performed by ServiceUI is <= ServiceUIFactory.MAIN_UIROLE
> > > > > > > >
> > > > > > > > [http://hg.openjdk.java.net/jdk/client/file/754ec520eb4a/src/java.desktop/windows/classes/sun/print/Win32PrintService.java#l1688]
> > > > > > > >
> > > > > > > > Proposed fix is to make sure this role is accounted for in the
> > > > > > > > buttonProperties enabling check.
> > > > > > > >
> > > > > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8246742
> > > > > > > >
> > > > > > > > webrev: http://cr.openjdk.java.net/~psadhukhan/8246742/webrev.0/
> > > > > > > >
> > > > > > > > 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">
+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/cl \
asses/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>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic