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.
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.
I think the test should not fail if the button is enabled. It should
fail only if
the button is enabled *and does nothing*.
-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.sadhuk...@oracle.com
<mailto:prasanta.sadhuk...@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.sadhuk...@oracle.com
<mailto:prasanta.sadhuk...@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
