On Tue, 12 Aug 2025 04:14:18 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org> 
wrote:

>> They can be added parameters to SU3.paintAccText.
>
> 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.

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

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

Reply via email to