JMRI / JMRI

JMRI model railroad digital command & control software
https://www.jmri.org
Other
241 stars 332 forks source link

javax.swing.plaf.basic.BasicSliderUI access in Java 10 and later #6467

Closed bobjacobsen closed 3 years ago

bobjacobsen commented 5 years ago

(This is about migration to Java 10 and particularly later releases; it's here to record the info, not as an urgent item for work. Please don't panic)

Running JMRI 4.15.2 on Java 10.0.2 on Linux gives:

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by jmri.util.swing.SliderSnap$Init (file:/home/phil/Downloads/JMRI/jmri.jar) to method javax.swing.plaf.basic.BasicSliderUI.xPositionForValue(int)

There are references to javax.swing.plaf.basic.BasicSliderUI in both jmri.util.swing.SliderSnap and jmri.jmrit.throttle.ControlPanelCustomSliderUI.

SliderSnap is some imported & modified code (from here which also explains what it does and how it does it) that changes (i.e. improves) the behavior of all JSliders created after it is invoked. JMRI's apps.Apps base invokes it as part of startup.

ControlPanelCustomSliderUI seems to be just that: A customization of JSlider for the jmri.jmrit.throttle.ControlPanel class. It's not referenced anywhere else. Unfortunately, there are no useful comments in the class. It redoes a lot of the painting code, but it's not clear what's replication of the super-class (as of the time it was written) and what's added/changed. ControlPanel uses it only when certain options are selected:

        if (InstanceManager.getDefault(ThrottleFrameManager.class).getThrottlesPreferences().isUsingExThrottle()
                && InstanceManager.getDefault(ThrottleFrameManager.class).getThrottlesPreferences().isUsingFunctionIcon()) {
            speedSlider.setUI(new ControlPanelCustomSliderUI(speedSlider));
        }
mattharris commented 5 years ago

Looks like the underlying issue is being triggered in the Init method of jmri.util.swing.SliderSnap, namely from around line 401 where we change accessibility to a couple of protected methods.

So, we need to either find a way of getting the information we need in a safe way, consider a graceful fallback when this isn't available, or write our own JSlider implementation...

bobjacobsen commented 5 years ago

For a couple years, we could add --add-opens javax.swing.plaf.basic option to all our various startup and test files to tell Java we know about this. That'll also surface the next message. Perhaps eventually somebody will update the borrowed code so we don't have to...

On the other hand, we have lots of startup and test scripts, and it's hard to keep them all up to date. I'm not really sure we want to go down this direction.

rhwood commented 5 years ago

Its also entirely possible that changes to the standard JSlider in 1.7 (its drawing method was rewritten) make the customizations in SliderSnap unneeded (it appears to be so from modifying the Java Tutorials SliderDemo to snap to ticks).

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. JMRI is governed by a small group of maintainers which means not all opened issues may receive direct feedback.

stale[bot] commented 3 years ago

This issue has been automatically closed due to lack of activity. In an effort to reduce noise, please do not comment any further. Note that the maintainers may elect to reopen this issue at a later date if deemed necessary.