On Sun, 17 Nov 2024 06:50:30 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org> wrote:
>> Since JEP 486 : Permanently Disable the Security Manager >> [https://bugs.openjdk.org/browse/JDK-8338625] is now integrated, calls to >> java.security.AccessController.doPrivileged are obsolete and can be removed. >> >> This PR takes care of the windows-platform files in the java.desktop module >> to have them removed. > > Prasanta Sadhukhan has updated the pull request incrementally with two > additional commits since the last revision: > > - Fix to getBoolean > - FIx Boolean.getBoolean Changes requested by aivanov (Reviewer). 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"); 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. 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? src/java.desktop/windows/classes/sun/awt/Win32FontManager.java line 203: > 201: String[] info = new String[2]; > 202: info[0] = "Arial"; > 203: info[1] = "c:\\windows\\fonts"; What if Windows is installed not on disk `C:` or not into `Windows` directory? Yes, it's out of scope of this particular issue, yet it doesn't feel right, even though it's very uncommon these days. 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. 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. 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. ------------- PR Review: https://git.openjdk.org/jdk/pull/22083#pullrequestreview-2443023439 PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1846832714 PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1846839129 PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1846846898 PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1846849270 PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1846861507 PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1846883030 PR Review Comment: https://git.openjdk.org/jdk/pull/22083#discussion_r1846889627
