On Fri, 26 Jun 2026 18:01:56 GMT, Phil Race <[email protected]> wrote:

>>> IS_WINXP is now missing a check, no?
>>> 
>> 
>> Yes, it does as per [my comment 
>> above](https://github.com/openjdk/jdk/pull/31676#discussion_r3481899372).
>> 
>> It should look like:
>> 
>> 
>> IS_WINXP (((LOBYTE(LOWORD(::GetVersion())) >= 5) && 
>> (HIBYTE(LOWORD(::GetVersion())) >= 1) || LOBYTE(LOWORD(::GetVersion())) > 5)
>> 
>> 
>> I may have missed or added a parenthesis.
>> 
>>> Also, is there a reason to keep IS_WINXP and IS_WINVISTA? These are also 
>>> older unsupported versions of Windows.
>> 
>> That's was the point of [my other 
>> comment](https://github.com/openjdk/jdk/pull/31676#issuecomment-4810315957), 
>> we can remove `IS_WINXP` too.
>
>> Also, is there a reason to keep IS_WINXP and IS_WINVISTA? These are also 
>> older unsupported versions of Windows.
> 
> The fix will get bigger .. as these are both used in a number of places.
> Unnecessarily perhaps (these days) but it does significantly expand the scope 
> of the change .

You're right,

> `IS_WINXP` is used in many places to determine menu bar colors, to handle 
> subclassing default Windows controls.

as I mentioned a [JBS 
comment](https://bugs.openjdk.org/browse/JDK-8289772?focusedId=14665584&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14665584)
 for [JDK-8289772](https://bugs.openjdk.org/browse/JDK-8289772).

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

PR Review Comment: https://git.openjdk.org/jdk/pull/31676#discussion_r3483444839

Reply via email to