On Thu, 19 Jun 2025 03:01:15 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 two 
> additional commits since the last revision:
> 
>  - Test fix
>  - javadoc wording..clear image desscription if image is null

Changes requested by aivanov (Reviewer).

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

> 145:         image = Toolkit.getDefaultToolkit().getImage(filename);
> 146:         if (image == null) {
> 147:             this.description = null;

I do not think we have to enforce setting `description` to `null` if the image 
is `null` — **it is up to the developer to decide**.

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

> 220:      * Setting a {@code null} image means
> 221:      * any existing image will be removed
> 222:      * and no image will be rendered.

It doesn't apply to the constructor — there ***can't** be an existing image*!

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

> 387:         this.image = image;
> 388:         if (image == null) {
> 389:             this.description = null;

We must not change the description. Why do we enforce resetting the description 
to `null`?

The app is still free to change the description to an arbitrary value using 
`setDescription` even if the image is `null`.

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

> 389:             this.description = null;
> 390:             return;
> 391:         }

We must not change the description, but we should reset `width` and `height` to 
`-1`.
Suggestion:

            width = -1;
            height = -1;
            return;
        }

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

PR Review: https://git.openjdk.org/jdk/pull/25767#pullrequestreview-2942389792
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2156647302
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2156649370
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2156652325
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2156655243

Reply via email to