RFR: 8159055: ImageIcon.setImage and ImageIcon(Image) constructor can't handle null parameter [v11]
Alexey Ivanov
aivanov at openjdk.org
Tue Jun 24 14:03:34 UTC 2025
On Mon, 23 Jun 2025 21:17:05 GMT, ExE Boss <duke at openjdk.org> wrote:
>> src/java.desktop/share/classes/javax/swing/ImageIcon.java line 233:
>>
>>> 231: Object o = image.getProperty("comment", imageObserver);
>>> 232: if (o instanceof String) {
>>> 233: description = (String) o;
>>
>> To address [Phil's comment](https://github.com/openjdk/jdk/pull/25767/files#r2155610976):
>>
>>> This is wasted work if the app calls ImageIcon(Image, String) because that promptly over-writes whatever was obtained via this code.
>>
>> If we're going to change the constructors, to avoid *this wasted work* when `ImageIcon(Image, String)` constructor is called, I suggest moving the work into `ImageIcon(Image, String)` and implement `ImageIcon` like this:
>>
>>
>> public ImageIcon (Image image) {
>> String description = null;
>> if (image != null) {
>> Object o = image.getProperty("comment", null);
>> if (o instanceof String) {
>> description = (String) o;
>> }
>> }
>> this(image, description);
>>
>> It is allowed in JDK 22 and later.
>
> Note that this was a preview language feature until **JDK 25**.
I haven't done my due diligence to verify whether this feature can be used. Even if this particular way can't be used, it's still a viable option, just move the code above into a helper method.
public ImageIcon(Image image, String description) {
this.image = image;
this.description = description;
loadImage(image);
}
public ImageIcon (Image image) {
this(image, getImageComment(image));
}
/**
* {@return the {@code "comment"} property of the image
* if the value of the property is a sting}
* @param image the image to get the {@code "comment"} property
*/
private static String getImageComment(Image image) {
if (image == null) {
return null;
}
Object o = image.getProperty("comment", null);
return (o instanceof String) ? (String) o : null;
}
This should be done separately from this changeset, I think, to have shorter, more specific changes.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25767#discussion_r2164102520
More information about the client-libs-dev
mailing list