466257 2023-02-22 19:50:09 +0000 Wrong calculation of cache size? 2023-02-26 12:07:22 +0000 1 1 2 Applications konqueror general Git Other Linux RESOLVED FIXED NOR normal --- 1 karl konqueror-bugs-null posta https://invent.kde.org/network/konqueror/commit/979560661500f83f04e63d26e94d0531c9a5f52e 0 oldest_to_newest 2210769 0 karl 2023-02-22 19:50:09 +0000 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). 2211855 1 posta 2023-02-26 10:33:05 +0000 (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 2211876 2 posta 2023-02-26 11:27:12 +0000 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 2211877 3 karl 2023-02-26 11:35:25 +0000 (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’. 2211887 4 karl 2023-02-26 12:07:22 +0000 BTW, thanks for fixing this bug. And I’ve added a bug report (bug 466468) for general spinbox support for formatting of byte values.