On Mon, 16 Jun 2025 14:59:13 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:
> 
>   Fix for constructor

Changes requested by aivanov (Reviewer).

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

> 222:      * @see java.awt.Image#getProperty
> 223:      */
> 224:     public ImageIcon (Image image) {

Does it make sense to expand the javadoc for all the constructors to specify 
how `null` is handled?

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

> 378:         if (image == null) {
> 379:             return;
> 380:         }

Should `setImage` clear / update the description automatically?

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

> 38:         // Setting null image shouldn't cause NPE
> 39:         icon.setImage(null);
> 40:     }

Add the test for `new ImageIcon((Image) null)`: currently it throws 
`NullPointerException` but it won't throw it after your fix.

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

PR Review: https://git.openjdk.org/jdk/pull/25767#pullrequestreview-2932514833
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2150241193
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2150249349
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2150245194

Reply via email to