[OpenJDK 2D-Dev] RFR JDK-8246742: ServiceUI.printDialog does not support properties dialog

[Philip Race]

+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 at oracle.com 
>>>>> <mailto:prasanta.sadhukhan at 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 at oracle.com 
>>>>>>> <mailto:prasanta.sadhukhan at 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
>>>>>>
>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/2d-dev/attachments/20200727/011365f8/attachment-0001.htm>