[rfc][icedtea-web][itweb-settings] Improve IcedTea-Web cache disk space

Thu Sep 11 15:07:24 UTC 2014

On 09/08/2014 08:38 PM, Lukasz Dracz wrote:
>
> Hello,
>
>> Hello,
>>
>> When you check the 'limit cache size box' with a size of 0, the JAR
>> compression option and cache location option are disabled. At the moment,
>> limiting cache size to 0 means that once icedtea-web finishes, the cache is
>> cleared of all files.
>>
>> I think the description for Caching should be reworded a little to make this
>> known. E.g.
>>
>> "No files will be cached. Cached files will be deleted." can be reworded to
>> something like:
>> "Cached files will be deleted on icedtea-web close.
>
> Okay
>
>> As well, the ability to choose where icedtea-web downloads file should still
>> be enabled even if cache has a limit of 0.
>
> Yes since it still downloads the files even if they are deleted after use.
> This provides the user with more choice on where they want the files to be temporarily.
>
>> In terms of code, it functionally looks okay but I would strongly suggest
>> adding logically placed line spaces throughout the code for easier reading.
>
> Yes, sorry about that, I added spaces in a bunch of places now to increase readability.
>
> Thank you,
> Lukasz Dracz

As this patch seems kind of the last version that actually got pushed, I am 
replying to this posting. Are my few nits.

Regarding the discussion about cache size 0 and IcedTea-Web's behavior in this 
case, I suppose that displaying the TIFPCacheSizeSetToNoCaching message is an 
acceptable compromise (because it describes what will actually happen), but only 
as long as we can expect a followup patch that makes IcedTea-Web use a "memory 
only" cache.

 > diff -r af89bcaa02a2 -r e30d71ab91c6 netx/net/sourceforge/jnlp/resources
 > /Messages.properties
 > --- a/netx/net/sourceforge/jnlp/resources/Messages.properties	Wed Sep 10
 > 09:45:10 2014 -0400
 > +++ b/netx/net/sourceforge/jnlp/resources/Messages.properties	Wed Sep 10
 > 10:22:46 2014 -0400
 > @@ -801,6 +801,10 @@
 >  TIFPDeleteFiles=Delete files
 >  TIFPViewFiles=View files...
 >  TIFPFileChooserChooseButton=Choose
 > +TIFPLimitCacheSize=Limit cache size
 > +TIFPCacheSizeSpinnerValueTooLargeWarning=<html><b>WARNING:</b> Uses more
 > space than {0} MB available</html>
 > +TIFPCacheSizeSpinnerLargeValueWarning=<html> Available: {0} MB</html>

Drop the html tags. They are not required here. Besides, HTML trims all leading 
and trailing white spaces, as well as replaces multiple white spaces between 
words with one SPACE (\u0020) character. This probably leads to undesired 
behavior here, if not now then maybe in the future.

 > +TIFPCacheSizeSetToNoCaching=<html>Cached files will be deleted on
 > icedtea-web close.</html>

Again, drop the html tags. They do not serve any purpose here.
Also, please use the proper branding, which is "IcedTea-Web". We may have even a 
public static final constant for this.
And finally, this sentence needs rewording to be proper English. I would suggest:

Cached files will be deleted when IcedTea-Web terminates.

I think the term "terminates" is appropriate here, because the user actually 
never gets to "close" IcedTea-Web directly since its shutdown or /termination/ 
is controlled indirectly via the JNLP application or the JVM.

One last thing I have experienced is that a message for 
"TIFPCacheSizeSpinnerTooltip" is missing when hovering over the cache size 
spinner. Please fix this!

Thank you for improving the cache size UI controls. ;-)

Jacob