Hi Prasanta, I again tested updated webrev in Windows and Linux and it works as expected.
webrev.1 looks good to me. Thanks, Jay > On 20-Jul-2020, at 8:45 PM, Prasanta Sadhukhan > <prasanta.sadhuk...@oracle.com> 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 = new HashPrintRequestAttributeSet(); > 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/ > <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 >> >> <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 >> >> <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 >>> >>> <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 >>>> >>>> <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 >>>>>> >>>>>> <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/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 >>>>>> >>>>>> <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 >>>>>> >>>>>> <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 >>>>>> <https://bugs.openjdk.java.net/browse/JDK-8246742> >>>>>> webrev: http://cr.openjdk.java.net/~psadhukhan/8246742/webrev.0/ >>>>>> <http://cr.openjdk.java.net/~psadhukhan/8246742/webrev.0/>Regards >>>>>> Prasanta >>>> >>
