On Tue, 12 Aug 2025 10:29:54 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:

>> Since we needed to move the `paintIcon`, `paintCheckIcon`, `paintAccText`, 
>> `paintText `and `paintArrowIcon `from `BasicMenuITemUI` to `SwingUtilties3` 
>> so that it can be called from `WindowsMenuItemUI` and `BacisMenuItemUI `to 
>> avoid code duplication, I had to made these methods static since these 
>> methods are called from `paintMenuItem` which is needed to be called from 
>> `WindowsCheckBoxMenuItemUI`, `WindowsRadioButtonMenuItemUI`, `WindowsMenuUI 
>> `via `WindowsMenuItemUI.paintMenuItem` [similar to existing 
>> methods`WindowsMenuItemUI.paintText `and 
>> `WindowsMenuItemUI.paintBackground`] and because the methods were static the 
>> variables accessed by it also needed to be static to avoid build issues
>> 
>> and I didnt want to change the call, signature and content of this methods 
>> so that there was no inadvertent regression introduced which is the reason I 
>> didn't change the parameters of these methods
>
> I understand that methods in SwingUtilities3 had to be static, but why the 
> colors couldn't be passed to the method as parameters? [I did 
> suggest](https://github.com/openjdk/jdk/pull/23324#discussion_r2169199694) 
> passing the colors as parameters:
> 
>> Pass the colors explicitly as parameters to the 
>> `SwingUtilities3.paintAccText` method.
> 
> Storing the colors as global variables in a utility class leaks these 
> objects. After L&F is changed to something else, these colors persist in 
> `SwingUtilities3`.
> 
> The same applies to the new static fields in `WindowsMenuItemUI`. They're not 
> needed. [I had raised this 
> concern](https://github.com/openjdk/jdk/pull/23324#discussion_r2169217699), 
> it went unanswered and marked as resolved. And I believe there's a new bug 
> because of these static fields.

If we have to call `WindowsMenuItemUI.paintMenuItem` from 
`WindowsCheckBoxMenuItemUI`, `WindowsRadioButtonMenuItemUI`, `WindowsMenuUI ` 
then `paintMenuItem `has to be static in `WindowsMenuItemUI`
so `paintAccText ` called from `paintMenuItem` has to be static so even if we 
try to pass the parameters to `SwingUtilities3.paintAccText` from 
`WindowsMenuItemUI.paintAccText ` those parameters need to be static else we 
will be getting this kind of build error

error: non-static variable disabledForeground cannot be referenced from a 
static context
        SwingUtilities3.paintAccText(g, lh, lr, disabledForeground, 
acceleratorSelectionForeground, acceleratorForeground);

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23324#discussion_r2269557832

Reply via email to