On Tue, 5 Aug 2025 16:18:41 GMT, Khalid Boulanouare <d...@openjdk.org> wrote:
>> This PR will consolidate fixes of the following bugs: >> >> https://bugs.openjdk.org/browse/JDK-8361188 >> https://bugs.openjdk.org/browse/JDK-8361189 >> https://bugs.openjdk.org/browse/JDK-8361190 >> https://bugs.openjdk.org/browse/JDK-8361191 >> https://bugs.openjdk.org/browse/JDK-8361192 >> https://bugs.openjdk.org/browse/JDK-8361193 >> https://bugs.openjdk.org/browse/JDK-8361195 >> >> This PR depends on https://github.com/openjdk/jdk/pull/25971 > > Khalid Boulanouare has updated the pull request incrementally with one > additional commit since the last revision: > > Updates copyright years test/jdk/java/awt/Mixing/AWT_Mixing/MixingPanelsResizing.java line 118: > 116: jbutton.setForeground(jbColor); > 117: jbutton.setBackground(jbColor); > 118: jbutton.setOpaque(true); This should be the default value, I mean Swing components are usually opaque unless it's changed by app developer or look and feel. For example, rounded corners may make the component non-opaque as the background in the rounded corners has to be painted by the parent component. test/jdk/java/awt/Mixing/AWT_Mixing/MixingPanelsResizing.java line 177: > 175: btnLoc = jbutton2.getLocationOnScreen(); > 176: c = robot.getPixelColor(btnLoc.x + 5, btnLoc.y + 5); > 177: System.out.println("Color picked for jbutton2: "+c); Suggestion: System.out.println("Color picked for jbutton2: " + c); Spaces around binary operators. This applies to other similar cases. test/jdk/java/awt/Mixing/AWT_Mixing/MixingPanelsResizing.java line 313: > 311: return true; > 312: } else { > 313: return Math.abs(color.getRed() - refColor.getRed()) < The indentation of the first `if` is wrong. Essentially, this `if` condition can be simplified to return color.equals(refColor) || (Math.abs(color.getRed() - refColor.getRed()) < COLOR_TOLERANCE_MACOSX && Math.abs(color.getGreen() - refColor.getGreen()) < COLOR_TOLERANCE_MACOSX && Math.abs(color.getBlue() - refColor.getBlue()) < COLOR_TOLERANCE_MACOSX; There may be a utility method available for colour tolerance which seems quite common for macOS. test/jdk/java/awt/Mixing/AWT_Mixing/OverlappingTestBase.java line 384: > 382: return false; > 383: } > 384: return true; Suggestion: return !(component == null || (component instanceof java.awt.Scrollbar) || (isMac && (component instanceof java.awt.Button))); There's no need for the `if` statement, just return the value of the boolean expression. You can propagate the negation into the boolean expression. I suggest using parentheses to group the condition with `isMac` to unambiguously express the intention. I prefer operators on the wrapped line, this is the way Java Coding Style suggests, and such a way emphasises it's a continuation line rather than a new statement. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/26625#discussion_r2270160787 PR Review Comment: https://git.openjdk.org/jdk/pull/26625#discussion_r2270151939 PR Review Comment: https://git.openjdk.org/jdk/pull/26625#discussion_r2270141208 PR Review Comment: https://git.openjdk.org/jdk/pull/26625#discussion_r2270121498