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

[Prasanta Sadhukhan]

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 =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/
>>
>> 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/83347ee2/attachment.htm>