On Mon, 18 Nov 2024 15:45:20 GMT, Alexey Ivanov <[email protected]> wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Fix to getBoolean
>> - FIx Boolean.getBoolean
>
> src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/WindowsLookAndFeel.java
> line 185:
>
>> 183: // to be switched off either at runtime or programmatically
>> 184: //
>> 185: String systemFonts =
>> System.getProperty("swing.useSystemFontSettings");
>
> I guess this line and the following line could be combined into one simple
> line:
>
>
> useSystemFontSettings = Boolean.getBoolean("swing.useSystemFontSettings");
A goal here is to MINIMISE optional refactoring, which is what this suggestion
is.
> src/java.desktop/windows/classes/com/sun/java/swing/plaf/windows/XPStyle.java
> line 134:
>
>> 132: }
>> 133: if (themeActive.booleanValue()) {
>> 134: String propertyAction =
>> System.getProperty("swing.noxp");
>
> The variable name is now confusing… it's not an action anymore. Alternative,
> `getProperty` can be inlined in the `if` statement.
A goal here is to MINIMISE optional refactoring, this is borderline. I'll leave
it up to Prasanta.
> src/java.desktop/windows/classes/sun/awt/Win32FontManager.java line 84:
>
>> 82: * printing the printer driver can access the fonts.
>> 83: */
>> 84: registerJREFontsWithPlatform(jreFontDirName);
>
> Are there any JRE fonts? Shall this line be removed too?
That has has nothing to do with removing doPrivileged. Leave it.
> src/java.desktop/windows/classes/sun/awt/Win32FontManager.java line 211:
>
>> 209: File file = new File(path);
>> 210: if (file.exists()) {
>> 211: dir = dirs[i];
>
> This can be simplified to
>
> info[1] = dirs[i];
>
> and `dir` variable can be removed.
A goal here is to MINIMISE optional refactoring, which is what this suggestion
is.
> src/java.desktop/windows/classes/sun/awt/windows/WFramePeer.java line 81:
>
>> 79: private native void clearMaximizedBounds();
>> 80:
>> 81: private static final boolean keepOnMinimize = "true".equals(
>
> Can be replaced with
> `Boolean.getBoolean("sun.awt.keepWorkingSetOnMinimize");`.
>
> It would be consistent with other updates, too.
Let's leave it. I regret making the suggestion to use Boolean.getBoolean earlier
> src/java.desktop/windows/classes/sun/awt/windows/WPathGraphics.java line 102:
>
>> 100:
>> 101: if (textLayoutStr != null) {
>> 102: useGDITextLayout = Boolean.getBoolean(textLayoutStr);
>
> This line seems wrong: `textLayoutStr` is a value of the property, and we now
> read a new property with the value of the
> `sun.java2d.print.enableGDITextLayout` property.
>
> Is it what's intended? The following
> line—`textLayoutStr.equalsIgnoreCase("prefer")`—assumes it's not the intended
> usage.
If there's an existing problem, let's examine it under a separate bug. This
change is about equivalence
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1847169851
PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1847171162
PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1847172487
PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1847172744
PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1847174694
PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1847182852
