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

[Philip Race]

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

I think the test should not fail if the button is enabled. It should 
fail only if
the button is enabled *and does nothing*.

-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/20200723/3da63be2/attachment-0001.htm>