eirikbakke commented on code in PR #8114:
URL: https://github.com/apache/netbeans/pull/8114#discussion_r1904109624
##########
platform/openide.util.ui/src/org/openide/util/ImageUtilities.java:
##########
@@ -210,6 +210,59 @@ public static final Image loadImage(String resource,
boolean localized) {
return loadImageInternal(resource, localized);
}
+ /**
+ * Load an image from an URL. If the URL uses the {@code nbresloc}
protocol, it is loaded using
+ * the resource loading mechanism provided by {@link
#loadImage(java.lang.String)}. An SVG
+ * image may be substituted when available.
+ *
+ * <p>This method is intended for use only when a URL must be used instead
of a resource path,
+ * e.g. in the implementation of pre-existing NetBeans APIs.
+ *
+ * @param url the URL of the image, possibly with the nbresloc protocol
+ * @return the loaded image, or either null or an uninitialized image if
the image was not
+ * available
+ * @since 7.34
+ */
+ public static final Image loadImage(URL url) {
+ Parameters.notNull("icon", url);
+ if (url.getProtocol().equals("nbresloc")) { // NOI18N
+ // Omit the initial slash of the path.
+ return loadImage(url.getPath().substring(1));
+ } else {
+ /* Observed to return an image with size (-1, -1) if URL points to
a non-existent file
+ (after ensureLoaded(Image) is called). */
+ return Toolkit.getDefaultToolkit().createImage(url);
Review Comment:
OK, It seems clear that loadImage(URL) is ugly in several ways, and it would
be effectively deprecated from the start.
The goal here was to factor out existing uses of Toolkit.createImage(URL)
without actually changing any behaviors, except in the well-defined
"nbresloc:/" case. If we start to change the code beyond that, more things can
break. Some of the call sites that use Toolkit.createImage are API
implementations, so it's hard to find and test all the specific uses.
Possible alternatives:
1) Removing loadImage(URL) from ImageUtilities, and just inline the special
handling of the nbresloc protocol in each of the 9 cases. The existing
Toolkit.createImage calls remain like before for other URL protocols.
2) Like 1, but make changes only as needed to fix specific missing SVG icons
(rather than all 9 cases).
3) Keep the new ImageUtilities.loadImage(URL) method, but deprecate it from
the start. We could switch the parameter to URI like Neil suggests (this is not
supposed to change the behavior, right?).
4) Like 3, but with whitelisting logic added. I'm not sure how to do that
for the "jar" protocol. This could break some existing functionality if we get
it wrong.
5) Like 4, but leaving loadImage un-deprecated.
I think I'd lean towards 3. It avoids modifying existing behavior.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
