[OpenJDK 2D-Dev] RFR: 8203796: Define API to support specifying ownership of print dialogs.

[Kevin Rushforth]

Looks good.

Thanks.

-- Kevin


On 6/5/2018 12:29 PM, Phil Race wrote:
>
>
> On 06/05/2018 11:46 AM, Kevin Rushforth wrote:
>> My testing of the fix on Windows (with the corresponding FX fix) 
>> looks good. A couple comments / questions:
>>
>> DialogOwner.java:
>>
>> * The docs should probably say what the behavior is if the 
>> application does not have the alwaysOnTop permission. I presume that 
>> the attribute will be silently ignored without that permission? Have 
>> you tested this?
>
> I expect a security exception will be thrown. Although it is unlikely 
> that an application
> will have print permission and not this permission.
> Since the application does not control the size/appearance/content of 
> the dialog
> the reasons for the permission do not apply, so if it did not, it is 
> not a concern if
> it was to always appear on top, but also if the application does not 
> have this permission
> it is not important enough to assert it.
> So a change to catch the exception should be sufficient and I will 
> make that change.
>
>>
>> PrintJob2D.java:
>>
>> * The only change appears to be adding an unused import.
>
> It is not unused. Previously DialogOwner was in the same package.
> Now it is not, so an import is now needed.
>
>>
>>
>> DialogOwnerTest.java:
>>
>> * Does the test need '@key headful' or is this not needed because it 
>> is a manual test?
>
> right, all our manual tests require interaction so are headful.
>>
>> * The new unit test fails to compile for me, but maybe I'm doing 
>> something wrong (I just ran it with jtreg with no special arguments).
>
> fixed
>
> updated webrev : http://cr.openjdk.java.net/~prr/8203796.2/
>
> -phil.
>
>>
>> -- Kevin
>>
>>
>> On 6/4/2018 12:39 PM, Phil Race wrote:
>>> I have posted an updated webrev 
>>> http://cr.openjdk.java.net/~prr/8203796.1
>>> which removes the "native id" case from the public API. Now the plan is
>>> that any code that needs to specify this will need to use native 
>>> code to set it.
>>> The CSR text is also updated, and so is the referenced FX webrev
>>> http://cr.openjdk.java.net/~prr/8195808.1
>>>
>>> -phil.
>>>
>>> On 05/25/2018 11:04 AM, Phil Race wrote:
>>>> This defines a new class "DialogOwner" implementing 
>>>> PrintRequestAttribute
>>>> which allows an application to specify that a Print or Page dialog 
>>>> be  one of
>>>> 1) owned by an application specified AWT window
>>>> 2) owned by an application specified foreign (non-AWT) window
>>>> 3) use AWT's alwaysOnTop functionality where supported.
>>>>
>>>> JDK bug: https://bugs.openjdk.java.net/browse/JDK-8203796
>>>> CSR : https://bugs.openjdk.java.net/browse/JDK-8203834
>>>> JDK webrev : http://cr.openjdk.java.net/~prr/8203796/
>>>>
>>>> Please review the webrev and the CSR.
>>>>
>>>> The webrev will show that this functionality was already mostly 
>>>> available but only as internal API.
>>>>
>>>> To help applications be able to query what is supported, the various
>>>> PrintService implementation classes needed to be updated, for 
>>>> example "ID"
>>>> is reported as unsupported unless specifying using the native 
>>>> dialog on windows.
>>>> That actually pointed out that back in JDK 7 when we added 
>>>> DialogTypeSelection
>>>> to be able to choose native or Swing there was no query support for 
>>>> that to be
>>>> added. It was not a problem though as if it was not supported it 
>>>> really just meant
>>>> that applications would have to live with what dialog was provided, 
>>>> as before.
>>>> But it made a test for this new API more difficult .. so I added 
>>>> code needed for that.
>>>> The test is manual. An automated test might be possible, but 
>>>> probably very flaky.
>>>> But the test only makes you run through the scenarios that we know 
>>>> are supportable.
>>>>
>>>> The motivation for this new public API is to help OpenJFX since it 
>>>> uses the Java 2D PrinterJob class
>>>> to implement its own printing, so there is also an FX side to 
>>>> switch to using this instead
>>>> of the earlier internal API which it can not access when it is 
>>>> outside the JDK :
>>>>
>>>> FX bug : https://bugs.openjdk.java.net/browse/JDK-8195808
>>>> FX webrev: http://cr.openjdk.java.net/~prr/8195808/
>>>>
>>>> -phil.
>>>>
>>>
>>
>