On Mon, 7 Jul 2025 22:43:21 GMT, Phil Race <p...@openjdk.org> wrote: > I think we need to have the "whole picture" tested to make sure everything > does as we expect. It isn't so easy to tell even from looking at the source > of ImageIcon. Then spec'ed so developers know what to expect. When I wrote a > little test, I see null args do generate NPEs but invalid args don't result > in null images Toolkit images are installed, even if they can't be used > because the source isn't valid. They are "effectively" null, but not > "actually" null. We should at least specify the null arg behaviours and try > to say something about invalid image data. And as a result, I am now flipping > to just documenting the setImage(null) NPE The protected method loadImage > probably needs to say it too. Invalid image data, maybe we can cover that in > a class level statement. Note: I'd like to see every constructor tested with > both null and invalid image data.
I tested with null image/imagedata and invalid imagedata and it seems only these constructors throw NPE (current JDK) for null image (invalid image doesnt throw any exception) ImageIcon(image, desc) ImageIcon(image) ImageIcon(byte[], desc) ImageIcon(byte[]) Current PR checks for null in ImageIcon(image) so it will not throw NPE but ImageIcon(byte[]) will still throw.. Should we handle those in separate PR and let this only be for setImage(null)? ------------- PR Comment: https://git.openjdk.org/jdk/pull/25767#issuecomment-3047479917
