On Tue, 5 Dec 2023 07:55:22 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org> wrote:
> > > > It is probably easy just drop the usage of loadedImage and use the > > > > image instead? > > > > > > > > > Ideally. Yet there's a corner case: if `url` is null, there's nothing to > > > load; and it doesn't make sense to re-try, the URL won't change. If it > > > were not for this case, the `image` field alone would do the job. > > > > > > Not sure how it is expected to work, the url is a parameter and can be > > changed on the fly(see the usage of base in the StyleSheet.java) > > Yes, seems like setBase can be used to change image URL on the fly...Modified > to use image field for DCL... I am against this latest change: it doesn't fix much but it would cause creating the URL each time `getImage` is called if the URL is invalid for whatever reason. Usually, `setBase` isn't changed on the fly, it's set once when the `StyleSheet` and/or `HTMLDocument` is created. If `StyleSheet.setBase` is called to modify the base, the image needs reloading *whether it's already loaded or not* — the latest change doesn't address this problem in any way: if the image is successfully loaded, it's what's returned even though the base has been changed; and only if the image couldn't be loaded with the old base, the image will be re-loaded again. This is asymmetric: in one case, it's re-loaded, in another — it isn't. I'm for keeping the current behaviour: if the URL is invalid, the image isn't loaded and will never be for this instance of` CSS.BackgroundImage`; if the URL is valid, the image gets loaded. ------------- PR Comment: https://git.openjdk.org/jdk/pull/16917#issuecomment-1840752465