RFR: 8224261: JProgressBar always with border painted around it

Alexey Ivanov aivanov at openjdk.org
Thu Nov 16 13:16:34 UTC 2023


On Thu, 2 Nov 2023 04:13:17 GMT, Abhishek Kumar <abhiscxk at openjdk.org> wrote:

> JProgressBar is always painted with border irrespective of the value set via the API `setBorderPainted(boolean value)` in Synth (Nimbus and GTK) LAF. Proposed fix is to add a check before painting the component.
> 
> CI jobs are green after the fix. Links attached to JBS.

Changes requested by aivanov (Reviewer).

test/jdk/javax/swing/JProgressBar/TestProgressBarBorder.java line 29:

> 27:  * @key headful
> 28:  * @summary Verifies if JProgressBar border is painted even though border
> 29:  *          painting is set to false

Suggestion:

 * @summary Verifies JProgressBar border is not painted when border
 *          painting is set to false

test/jdk/javax/swing/JProgressBar/TestProgressBarBorder.java line 63:

> 61:             } else {
> 62:                 continue;
> 63:             }

I suggest inverting the condition to skip other L&Fs:
Suggestion:

            if (!laf.getName().contains("Nimbus") && !laf.getName().contains("GTK")) {
                continue;
            }

            System.out.println("Testing LAF: " + laf.getName());
            SwingUtilities.invokeAndWait(() -> setLookAndFeel(laf));

test/jdk/javax/swing/JProgressBar/TestProgressBarBorder.java line 145:

> 143:     */
> 144: 
> 145:     private static boolean compareImage(BufferedImage img1, BufferedImage img2) {

You can use the method from `regtesthelpers/Util`:

https://github.com/openjdk/jdk/blob/9faead1469481e268b451f2853c8fec8613426b9/test/jdk/javax/swing/regtesthelpers/Util.java#L59-L80

If you don't want to use, I suggest reversing the conditions so that `compareImage` returns `true` when images are the same — it's more common and therefore less confusing. In addition to that, return quickly if the sizes are different (can they ever be?) and avoid nesting the `for`-loops inside the `if` statement.

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

PR Review: https://git.openjdk.org/jdk/pull/16467#pullrequestreview-1734324288
PR Review Comment: https://git.openjdk.org/jdk/pull/16467#discussion_r1395663025
PR Review Comment: https://git.openjdk.org/jdk/pull/16467#discussion_r1395666368
PR Review Comment: https://git.openjdk.org/jdk/pull/16467#discussion_r1395673238


More information about the client-libs-dev mailing list