On Thu, 19 Jun 2025 03:01:15 GMT, Prasanta Sadhukhan <[email protected]>
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