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

Reply via email to