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