RFR: 8278620: properties installed by javax.swing.LookAndFeel installColors and installColorsAndFont are not uninstalled [v6]

[SWinxy]

On Tue, 28 Feb 2023 20:09:04 GMT, SWinxy <duke at openjdk.org> wrote:

>> Many `installDefaults` methods set the font, foreground, and background on objects but their inverse methods `uninstallDefaults` do not remove them. I've added an inverse method to remove the colors and font to call for the `uninstallDefaults` methods that install defaults.
>> 
>> `AquaButtonUI` can call its super since it would otherwise be repeated code. `BasicComboBoxUI` (weirdly) installs the properties again when it should be uninstalling them, so I changed.
>> 
>> I noticed that, in a few subclasses, only one of calls to the super of `installDefaults` and `uninstallDefaults` are made. That is, an overridden `installDefaults` may call its super while the overridden `uninstallDefaults` does not call its super (or vise versa). These classes are: `AquaTabbedPaneUI`, `SynthMenuItemUI`, `SynthSplitPaneUI`, and `XTextAreaPeer`.
>> 
>> Sorry I couldn't write a test; I wasn't sure how I should have accessed the protected variable aside from creating extending classes for each class that changed.
>> 
>> See also #6603, where this issue was discovered.
>
> SWinxy has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains ten commits:
> 
>  - Merge branch 'master' into 8278620
>    
>    # Conflicts:
>    #	src/java.desktop/macosx/classes/com/apple/laf/AquaButtonUI.java
>  - Revert "Make default fonts FontUIResources"
>    
>    This reverts commit 1cc422224331440317150f80cf567338376f95df.
>  - Make default fonts FontUIResources
>  - Minor cleanup on StalePreferredSize
>  - BasicPanelUI uninstalling font causes cascading problems
>  - Merge branch 'master' into 8278620
>  - Sync with CSR
>    
>    Only difference is that I added an Oxford comma
>  - Accidental imports added
>  - 8278620: properties installed by javax.swing.LookAndFeel installColors and installColorsAndFont are not uninstalled
>    
>    Many installDefaults methods set the font, foreground, and background on objects but their inverse methods uninstallDefaults do not remove them. I've added an inverse method to remove the colors and font to call for the uninstallDefaults methods that install defaults.
>    
>    AquaButtonUI can call its super since it would otherwise be repeated code. BasicComboBoxUI (weirdly) installs the properties again when it should be uninstalling them, so I changed.
>    
>    I noticed that, in a few subclasses, only one of calls to the super of installDefaults and uninstallDefaults are made. That is, an overridden installDefaults may call its super while the overridden uninstallDefaults does not call its super (or vise versa). These classes are: AquaTabbedPaneUI, SynthMenuItemUI, SynthSplitPaneUI, and XTextAreaPeer.

Apologies for the delay. I wrongly assumed that making the fonts FontUIResources would fix it. I only ran the tests on macOS, which is absolutely my fault. I'm not testing enough before submitting PRs. 😔

> > 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.

I don't *think* it was deliberate. Probably this same issue was ran into back then, and the easiest thing to do was to not remove the font.

> 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?

And I'm also not sure why! I'll do some more investigating. Thanks for the writeup, Alexey.

-------------

PR: https://git.openjdk.org/jdk/pull/10565