On Tue, 28 Feb 2023 20:09:04 GMT, SWinxy <[email protected]> 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

Reply via email to