Bug 466257

(In reply to Karl Ove Hufthammer from comment #0)
> I took a look at the source code in
> konqueor/settings/konqhtml/cache/cache.cpp, and the following two lines look
> wrong to me:
> 
> In void Cache::load():
>     //Ensure that maxSizeInMB is greater than 0 if maxSizeInBytes is not 0
>     int maxSizeInMB = maxSizeInBytes == 0 ? 0 : std::max(1, maxSizeInBytes /
> 1000);
> 
> In void Cache::save():
>     //We store the size in bytes, not in MB
>     grp.writeEntry("MaximumCacheSize", m_ui->cacheSize->value()*1000);
> 
> If you divide the number of bytes by 1000, you get kB, *not* MB. Similarly,
> if you multiply an MB value by 1000, you get the number of kB, not the
> number of bytes.
> 
> In the UI (cache.ui), the suffix is set to ‘ MB’. So either the calculation
> should be changed to divide/multiply by 1000^2, or the UI string should be
> changed to ‘ kB’, or, preferably, to use the correct KFormat feature for
> proper formatting
> (https://api.kde.org/frameworks/kcoreaddons/html/classKFormat.html).

Thanks for pointing this out. The conversion factor is clearly wrong and I'll fix it immediately. I'm not sure, instead, about using KFormat here. As far as I can see, KFormat only returns a string representation of the number converted to the appropriate unit, but this won't be useful here, as I need to put the value inside a QSpinBox, which only accepts numbers. Of course I could use KFormat to convert a dummy value, then extract the unit from the string, but I'm not sure it's worth doing so. Could you please clarify your suggestion?

Thanks

Git commit 979560661500f83f04e63d26e94d0531c9a5f52e by Stefano Crocco.
Committed on 26/02/2023 at 11:27.
Pushed by stefanocrocco into branch 'master'.

Use the correct conversion factor

M  +4    -2    settings/konqhtml/cache/cache.cpp

https://invent.kde.org/network/konqueror/commit/979560661500f83f04e63d26e94d0531c9a5f52e

(In reply to Stefano Crocco from comment #1)
> Thanks for pointing this out. The conversion factor is clearly wrong and
> I'll fix it immediately. I'm not sure, instead, about using KFormat here. As
> far as I can see, KFormat only returns a string representation of the number
> converted to the appropriate unit, but this won't be useful here, as I need
> to put the value inside a QSpinBox, which only accepts numbers. Of course I
> could use KFormat to convert a dummy value, then extract the unit from the
> string, but I'm not sure it's worth doing so. Could you please clarify your
> suggestion?

Hmm, I’m not actually sure what the best solution is. The idea was that the spinbox would support the BinaryUnitDialect setting (see bug 364321), so  that spinbox should hold the number of *bytes*, but would be formatted as 'kB’, ‘KIB’ or ‘KB’ (or ‘MB’/‘MiB’) depending on the user‘s BinaryUnitDialect settings (and with the proper conversion factor, e.g. 1000 or 1024).

But this is complicated to implement, and it’s not a good idea that every program with a ‘byte spinbox’ should have a complicated workaround for handling it. There really should be a general ByteSpinBox that would handle this automatically. (See also bug 453853 for a similar problem in Dolphin.) For now, I guess it’s OK to have the text hardcoded as ‘MB’.

BTW, thanks for fixing this bug. And I’ve added a bug report (bug 466468) for general spinbox support for formatting of byte values.