<Swing Dev> [9] Review request for 8167176 Exported elements referring to inaccessible types in java.desktop
Sergey Bylokhov
Sergey.Bylokhov at oracle.com
Wed Oct 19 16:16:52 UTC 2016
Did you check this fix on other platforms?
For example what about WindowsFileChooserUI.java:
DirectoryComboBoxRenderer createDirectoryComboBoxRenderer()
Note than DirectoryComboBoxRenderer is not public.
is it necessary to add "exports" to "-Xlint:"? I assumed that it should
be added somewhere else and disabled if necessary per module.
On 19.10.16 18:56, Philip Race wrote:
> +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.
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>>
>>
--
Best regards, Sergey.
More information about the swing-dev
mailing list