On Tue, 17 Jun 2025 10:08:49 GMT, Prasanta Sadhukhan <psadhuk...@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

Reply via email to