RFR: 8159055: ImageIcon setImage and constructor can't handle null parameter [v9]

Alexey Ivanov aivanov at openjdk.org
Wed Jun 18 17:45:33 UTC 2025


On Tue, 17 Jun 2025 10:08:49 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> When trying to call 'icon.setImage(null);' where 'icon' is an instance of ImageIcon, a null pointer exception is thrown at runtime.
>> The code tried to get the `id` for that image and instantiates `MediaTracker` to associate the null image to that `id` and checks the status of loading this null image, removes the null image from the tracker and then tries to get the image width where it throws NPE as image is null.
>> 
>> It's better to not go through all MediaTracker usage and bail out initially itself for null image..
>
> Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   SUmmary fix

Changes requested by aivanov (Reviewer).

src/java.desktop/share/classes/javax/swing/ImageIcon.java line 218:

> 216:     /**
> 217:      * Creates an ImageIcon from an image object.
> 218:      * Setting a {@code null} image will not render any image icon.

Suggestion:

     * Passing a {@code null} image renders no icon.

src/java.desktop/share/classes/javax/swing/ImageIcon.java line 374:

> 372:     /**
> 373:      * Sets the image displayed by this icon.
> 374:      * Setting a {@code null} image will not render any image icon.

Suggestion:

     * Setting a {@code null} image renders no icon.

I think this is better: it's shorter, it uses the present tense because it's fact and it doesn't depend on anything.

test/jdk/javax/swing/ImageIcon/ImageIconNullImageTest.java line 27:

> 25:  * @test
> 26:  * @bug 8159055
> 27:  * @summary Verifies ImageIcon setImage and constructor handles null parameter

Suggestion:

 * @summary Verifies ImageIcon setImage and ImageIcon(Image) constructor handle null parameter

Let's be more specific.

test/jdk/javax/swing/ImageIcon/ImageIconNullImageTest.java line 42:

> 40: 
> 41:     private static void testImageIconNull() {
> 42:         // Setting null image shouldn't cause NPE

Suggestion:

        // Passing null image shouldn't cause NPE

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

PR Review: https://git.openjdk.org/jdk/pull/25767#pullrequestreview-2939948842
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2155166397
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2155163642
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2155170970
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2155043719


More information about the client-libs-dev mailing list