On Wed, 5 Mar 2025 03:13:32 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org> wrote:
>> When JRadioButtonMenuItem is called with imageIcon, then only imageIcon is >> shown without radiobutton in WIndowsLookAndFeel as there was no provision of >> drawing the radiobutton alongside icon. >> If icon is not there, the radiobutton is drawn. Added provision of drawing >> the radiobutton windows Skin even when imageIcon is present. > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > Review comment fix There is a lot to absorb here. I don't want to get into line by line code discussion, we need to have the big picture right first. (1) This affects all releases, right ? The evaluation in the bug report is non-existent. I expect to see a full description of how it came about (history) what the consequences are, what the options are etc. Without that the PR is in a vacuum. (2) My rough understanding is that this was "observed" because of a change in Windows 11 vs earlier Windows versions with the highlighting of the radio button .. but the fix has some how become focused on the ImageIcon ? If I don't have this right, give me a clear and complete explanation of the issue (ie part of the missing evaluation) Surely that isn't a Windows 10 vs Windows 11 issue ?? Unless it is an accidental by-product of some layout problem ? I am seeing code in the pre-fix side of the diff that does paint an icon .. so I am not sure I understand. If the same control in native windows doesn't allow such an icon, then we are within our rights to ignore it. There are a bunch of settings, such as b/g color etc that various L&Fs ignore if they aren't appropriate. Search the javax.swing javadoc for the word "ignore" and you'll find hundreds of matches. I can't actually tell from the PR description what the inentention is "Added provision of drawing the radiobutton windows Skin even when imageIcon is present." Does that mean it continues to ignore the imageicon - ie not render it ? But as I wrote there is something that looks to be painting an icon. Maybe we are talking about the "icon" that is actually the rendering of the radio button - not an application supplied imageicon ? I'd like 100% clarity here. (3) Any fix requiring a CSR would be a problem - at least if it is SE spec significant, because it seems like this fix needs to be backported all the way to 8u. If there is a *better* fix that requires a CSR for the future, we can do that as a follow-on for JDK 25 only, but we should start with what can be backported, and do the better fix as a follow-on. I'll deviate slightly from my "I won't get in to lines for code here", and say that FWIW the current fix isn't what I'd like to see. Putting a check for Windows L&F in a BasicUI class ?? We had a similar issue with GTK some months ago and it was something that we managed to avoid. Isn't there a Windows L&F sub-class we can do this in ?? (4) We are not any time soon de-supporting Windows 10. Windows 10 is still supported by Microsoft the day JDK 25 ships, even for regular users. We need to make this work properly on both Windows 10 and Windows 11. NB in both cases, there are unsupported older updates and still supported updates .. like 22H2 for Windows 10 is the one to check. If this means the code needs to work out if it is on Windows 11 vs 10, so be it. ------------- PR Comment: https://git.openjdk.org/jdk/pull/23324#issuecomment-2704698129
