+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.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
