On Fri, 24 Feb 2023 04:58:12 GMT, Phil Race <p...@openjdk.org> wrote:
> > > And if a default font prevents the FontUIResource from being installed, > > > how does it get installed the in the first place ? > > > Perhaps that first time the component has no peer and it really is null ? > > > > > > The font is null when it's created. Calling `getFont` causes it to go up > > the hierarchy, if it has a parent. The UI looks to be initialized (and thus > > a font is set) before `getFont` is called. > > That is what I guessed. Accidental or deliberate ? I'd have to spend time to > know. This is the code of `Component.getFont`: https://github.com/openjdk/jdk/blob/b4ea80731c6c0a0328a9801590ba5b081f08c3bd/src/java.desktop/share/classes/java/awt/Component.java#L1912-L1927 Until the UI is shown, there's no heavyweight peer, so the default font doesn't get set. Even more, a `UI` instance is set from a component constructor: https://github.com/openjdk/jdk/blob/b4ea80731c6c0a0328a9801590ba5b081f08c3bd/src/java.desktop/share/classes/javax/swing/JLabel.java#L176-L182 https://github.com/openjdk/jdk/blob/b4ea80731c6c0a0328a9801590ba5b081f08c3bd/src/java.desktop/share/classes/javax/swing/JPanel.java#L85-L90 Thus, a font is (always) set on a `JComponent` when the constructor is called, the default L&F font would be an instance of `UIResource`. However, if the font is reset to `null`, the default font (of heavyweight) components / peers comes into play. The default font gets set in `getGraphics`: https://github.com/openjdk/jdk/blob/b4ea80731c6c0a0328a9801590ba5b081f08c3bd/src/java.desktop/windows/classes/sun/awt/windows/WComponentPeer.java#L640-L643 https://github.com/openjdk/jdk/blob/b4ea80731c6c0a0328a9801590ba5b081f08c3bd/src/java.desktop/unix/classes/sun/awt/X11/XWindow.java#L380-L383 > > > FontUIResource is something devised by Swing, for Swing. > > > Making AWT components depend on it for the convenience of Swing is > > > backwards. > > > > D'oh. I agree. **AWT should not depend on Swing**. > > Sounds like the path would be to undo my last commit and just put a note in > > the code. > > But then there'd either be the correct font not installed or something else > bad ? > How would a note in the code help ? > > Maybe we are going about this all wrong ? > Maybe uninstall isn't needed ? > What are the rules (set by Swing?) for what a L&F should do when installing a > UI ? If it is "if there is a FontUIResource, then feel free to replace it > with yours" then may be everything in this PR (at least about fonts) is > un-needed ? With where the testing led us, I'm inclined to think that the omission of nullifying the font in `uninstallUI` wasn't accidental but rather a deliberate decision. Yes, a `Font` object gets “leaked” after `UI` is uninstalled from a `JComponent`. In majority of cases `uninstallUI` is followed by `installUI` for another Look-and-Feel. (Without a UI delegate, any `JComponent` is unusable.) As such, the `Font` object gets replaced with a newer one if the previous one is `UIResource` or remains intact if it's not `UIResource`, which likely means it was explicitly set by an app developer. The common rule for Swing: if a property is `UIResource`, replace it with a new default; otherwise preserve the property, don't change it. From this point of view, all the properties of a Swing component could remain untouched in `uninstallUI`. At the same time, many properties are reset to `null`. There are issues where font doesn't get reset even though it should. The latest example that comes to my mind is [JDK-6753661](https://bugs.openjdk.org/browse/JDK-6753661) and #12180. Were there others? This current bug [JDK-8278620](https://bugs.openjdk.org/browse/JDK-8278620) was submitted as the result of work on [JDK-8277817](https://bugs.openjdk.org/browse/JDK-8277817) and #6603 where an object had been serialized, if I remember correctly, but the serialized form cannot be read because a class is removed in a later version of JDK. (The `java/awt/dnd/BadSerializationTest/BadSerializationTest.java` test seems to do the wrong thing: it reads a serialized version of a Swing object which was saved by a previous version of JDK, which isn't supported and which was [mentioned in the comments](https://github.com/openjdk/jdk/pull/6603#issuecomment-991295599).) The description of JDK-8278620 states `BasicListUI` correctly uninstalls all its properties, *including the font_. Why is it not affected by the problem we're seeing here where the font gets reset to a non-`ResourceUI` font from the heavyweight peer of the frame? > This is code relied upon by tens of thousands of applications written over a > period of 25 years. A change like this is really risky, when you consider > that there are probably 10 times more apps than there are Swing regresssion > tests .. and only a few of the regression (and other) tests cover this kind > of scenario. > > Testing on every platform of every test we have is a bare minimum. > > In the end my point is that unless (and until) we see some application > complain, these proactive changes are a bad trade-off. Perhaps, font property should be *left untouched* just like it is now. However, resetting the color properties — foreground and background — doesn't seem to cause any issues. As such, the scope of this PR could be reduced to adding the `LookAndFeel.uninstallColors` method and using it in UI classes. As a matter of fact, I ran the client tests with the latest changeset. The set of failing tests [remains the same](https://github.com/openjdk/jdk/pull/10565#issuecomment-1354064460): * `javax/swing/GraphicsConfigNotifier/StalePreferredSize.java` (on Windows only) * one closed test; * one JCK test. Only the `sanity/client/SwingSet/src/SwingSet2DemoTest.java` test is gone from the list of failures. So, this hasn't helped. ------------- PR: https://git.openjdk.org/jdk/pull/10565
