<Swing Dev> [9] Review request for 8167176 Exported elements referring to inaccessible types in java.desktop
Philip Race
philip.race at oracle.com
Wed Oct 19 15:56:45 UTC 2016
+1 from me ...
-phil.
On 10/19/16, 8:22 AM, Alexandr Scherbatiy wrote:
>
> Hello,
>
> Could you review the updated fix:
> http://cr.openjdk.java.net/~alexsch/8167176/webrev.02
>
> - JRootPane.defaultPressAction/defaultReleaseAction fields are removed
> - MetalBorders.bumps and MetalScrollBarUI.bumps are made private
> - MetalFileChooserUI.createDirectoryComboBoxRenderer() method is
> made private
>
> Thanks,
> Alexandr.
>
> On 10/18/2016 8:23 PM, Phil Race wrote:
>>
>>
>> On 10/18/2016 03:54 AM, Alexandr Scherbatiy wrote:
>>> On 10/18/2016 12:42 AM, Philip Race wrote:
>>>> Hi,
>>>>
>>>> First note that any of the alternatives here require an approved
>>>> CCC before pushing.
>>>>
>>>> As I wrote in the bug the fields in JRootPane are not used. Making
>>>> it a public supertype
>>>> is no more useful than just deleting it. This like hiding the peers
>>>> which was a much
>>>> bigger change so please just delete these fields.
>>>>
>>>> I don't understand the reasoning behind changing "bumps" to be of
>>>> the super-class type Icon
>>>> and introducing a new variable that is what is then used.
>>>>
>>>> Since "MetalBumps" is package private we should be able to safely
>>>> assume no one has
>>>> been using these fields without the aid of reflection and I doubt
>>>> anyone found a need.
>>>> And that won't work in JDK 9 with upcoming jigsaw changes
>>>> If you wanted people to usefully access these then you'd need to
>>>> make MetalBumps public.
>>>> I *am not* suggesting that .. just pointing out that the
>>>> alternatives I see are either
>>>> make it entirely public, or entirely hide is as being a mistake.
>>>> So make the fields all private and do not provide the non-useful
>>>> public replacement.
>>>> And I mean remove today. In JDK 9. This is not as radical as it
>>>> seems since left as
>>>> it is they are useless to anyone as you cannot use them.
>>>>
>>>> Also createDirectoryComboBoxRenderer(..) can be made private too.
>>> All these fields and methods can be used with the public
>>> super-class type in a user code.
>>> For example the code below successfully compiled with JDK 8 and 9:
>>> -------------
>>> public class Test {
>>>
>>> static class CustomToolBarBorder extends
>>> MetalBorders.ToolBarBorder {
>>>
>>> public void doSomething(Graphics g) {
>>> Icon metalBumps = bumps;
>>> metalBumps.paintIcon(new JLabel(), g, 0, 0);
>>> }
>>> }
>>>
>>> static class CustomMetalFileChooserUI extends MetalFileChooserUI {
>>>
>>> public CustomMetalFileChooserUI(JFileChooser filechooser) {
>>> super(filechooser);
>>> }
>>>
>>> public void doSomething() {
>>> DefaultListCellRenderer listCellRenderer =
>>> createDirectoryComboBoxRenderer(getFileChooser());
>>> // do something with the listCellRenderer
>>> }
>>> }
>>
>> See if you can find any 3rd party code at all that uses any of these ...
>>
>>>
>>> static class CustomJRootPane extends JRootPane {
>>>
>>> public void doSomething() {
>>> AbstractAction pressAction = defaultPressAction != null
>>> ? defaultPressAction
>>> : new CustomAction();
>>> AbstractAction releaseAction = defaultReleaseAction != null
>>> ? defaultReleaseAction
>>> : new CustomAction();
>>> }
>>> }
>>> }
>>
>> This example is weird since defaultPressAction and
>> defaultReleaseAction are ALWAYS null
>> unless the application itself set them to something.
>> It is completely acceptable to me to delete these fields.
>>
>>
>>> -------------
>>>
>>> I just need to be sure that the incompatible change introduced by
>>> removing these fields or making them private is acceptable.
>>
>>>
>>>>
>>>> Finally, when these are resolved you should be able to re-enable
>>>> lint !
>>>> http://hg.openjdk.java.net/jdk9/dev/rev/81435a812f59
>>> I prepared the hint enabling fix but I believe it should be
>>> reviewed on the build-dev and jigsaw-dev aliases after the current
>>> fix is pushed.
>>
>> I don't think this is at all necessary. The warning has nothing to do
>> with jigsaw and isn't a build change per se.
>> It should be included with this webrev+subsequent push, under this
>> bug ID.
>>
>>
>> -phil.
>>
>>>
>>> Thanks,
>>> Alexandr.
>>>>
>>>> -phil.
>>>>
>>>> On 10/17/16, 4:22 AM, Alexandr Scherbatiy wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> Could you review the updated fix:
>>>>> http://cr.openjdk.java.net/~alexsch/8167176/webrev.01
>>>>>
>>>>> MetalBorders.bumps and MetalScrollBarUI.bumps fields are nor
>>>>> marked for removal any more.
>>>>>
>>>>> Thanks,
>>>>> Alexandr.
>>>>>
>>>>> On 10/14/2016 3:23 PM, Sergey Bylokhov wrote:
>>>>>> Is it necessary to remove tese fields? For example
>>>>>> MetalScrollBarUI.bumps looks similar to other fields
>>>>>> MetalScrollButton.increaseButton/decreaseButton.
>>>>>>
>>>>>> On 13.10.16 16:15, Alexander Scherbatiy wrote:
>>>>>>>
>>>>>>> Hello,
>>>>>>>
>>>>>>> Could you review the fix:
>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8167176
>>>>>>> webrev: http://cr.openjdk.java.net/~alexsch/8167176/webrev.00
>>>>>>> - Inaccessible classes returned by public API in swing are
>>>>>>> changed to
>>>>>>> their public super classes.
>>>>>>> - Fields which expose inaccessible classes are deprecated and
>>>>>>> marked
>>>>>>> for removal.
>>>>>>>
>>>>>>> Changing a public field type to its superclass is incompatible
>>>>>>> change
>>>>>>> with previous releases. However it is not possible to use a
>>>>>>> class which
>>>>>>> is not exported in JDK 9 because of the modularization feature.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Alexandr.
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>
>>
>
More information about the swing-dev
mailing list