This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
Please simplify following changeset http://hg.netbeans.org/main-golden/rev/42eefaf2a9f8 by introducing ImageUtilities.loadIcon(String,...)
Created attachment 76238 [details] patch
Y01 I miss some small, simple test. Otherwise OK.
looks ok to me. contrary to jtulach's opinion, I don't think tests of any sort is to be part of the apireview process. it might be helpful in complex changes to see the api usage, but here it is not serving any purpose.
[JG01] Javadoc must emphasize that the returned Icon may not be an ImageIcon. This is relevant because Swing components which grey out when disabled (such as JButton) will generally not work correctly with this method - you really need to use "new ImageIcon(loadImage(...))". So the method should only be used for situations where it is known that the icon will not be used for a button, menu item, etc., or if it is, that the button will always be enabled.
> [JG01] Ok, maybe it would be better to return ImageIcon all the time.
Created attachment 76270 [details] new patch JG01+Y01
If you're going to make that change, then you should remove the warning from the Javadoc of image2Icon. (I'm not sure if there would even be any remaining purpose in the image2Icon method - it would do nothing more than cache ImageIcon instances for images coming from loadImage, which may or may not be helpful.) Consider searching for existing code which could use the new method. In Jackpot syntax: org.openide.util.ImageUtilities.image2Icon(org.openide.util.ImageUtilities.loadImage($r)) => org.openide.util.ImageUtilities.loadIcon($r, false); org.openide.util.ImageUtilities.image2Icon(org.openide.util.ImageUtilities.loadImage($r, $l)) => org.openide.util.ImageUtilities.loadIcon($r, $l); new javax.swing.ImageIcon(org.openide.util.ImageUtilities.loadImage($r)) => org.openide.util.ImageUtilities.loadIcon($r, false); new javax.swing.ImageIcon(org.openide.util.ImageUtilities.loadImage($r, $l)) => org.openide.util.ImageUtilities.loadIcon($r, $l); new javax.swing.ImageIcon(org.openide.util.Utilities.loadImage($r)) => org.openide.util.ImageUtilities.loadIcon($r, false); new javax.swing.ImageIcon(org.openide.util.Utilities.loadImage($r, $l)) => org.openide.util.ImageUtilities.loadIcon($r, $l);
Created attachment 76733 [details] API change patch
Created attachment 76734 [details] usage patch
The Javadoc for loadImageIcon seems to be referring to itself. Don't forget that usages would need to be matched by spec version increments in the module deps.
Thanks for review. I updated dependencies and fixed Javadoc. core-main #94f783e8a5a3 core-main #5dc32844e809
Integrated into 'main-golden', will be available in build *200902130336* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main/rev/94f783e8a5a3 User: Tomas Holy <t_h@netbeans.org> Log: #157254: ImageUtilities.loadIcon is missing