Bug 410980 - Sorting of albums in tree view by name inconsistent with Dolphin
Summary: Sorting of albums in tree view by name inconsistent with Dolphin
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Albums-TreeView (show other bugs)
Version: 6.2.0
Platform: Appimage Linux
: NOR normal
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-08-16 13:12 UTC by Daniel Laidig
Modified: 2022-02-06 21:09 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 7.6.0


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Laidig 2019-08-16 13:12:19 UTC
SUMMARY
The sorting behavior of albums by name in the tree view used to be consistent with other applications, especially Dolphin, meaning that special characters are not ignored (verified by running 5.9.0). This behavior seems to have changed recently.

Personally, this breaks my workflow as I use a folder structure with underscores to ensure some items appear at the top and "yyyy", "yyyy-mm", "yyyy-mm-dd" prefixes for names (see an example below). Considering https://xkcd.com/1172/, I agree that this is not a valid argument, but I would argue that being consistent with Dolphin would be a good reason to revert to the old way.

STEPS TO REPRODUCE
1. Create the following directories inside a subdirectory of the collection:
---
_Important_Top_Album
2019 General
2019-01-01 New Year
2019-07 Summer vacation
2019-07-30 Some other album
Stuff
---
2. In digikam, choose View -> Sort Items -> By Folder

OBSERVED RESULT
In the album tree view in digikam, the albums are sorted in the following order: (6.2.0 Appimage)
---
2019-01-01 New Year
2019-07-30 Some other album
2019-07 Summer vacation
2019 General
_Important_Top_Album
Stuff
---

EXPECTED RESULT
The albums should be sorted in the order of creation (underscore first, space before hyphen). This is the behavior found in Dolphin (and digikam 5.9.0).

SOFTWARE/OS VERSION
KDE Plasma Version: 5.15.4
KDE Frameworks Version: 5.56.0 
Qt Version: 5.12.2

ADDITIONAL INFORMATION
After some grepping, I thought that AlbumFilterModel::lessThan() in core/libs/models/albumfiltermodel.cpp should be the relevant piece of code. However, this function has not changed between 5.9.0 and master, so I'm guessing that there has been some other kind of refactoring or that I am looking in the wrong place. Unfortunately, I do not have time to set up a development environment right now.
Comment 1 caulier.gilles 2019-08-16 13:26:30 UTC
Can you take a screenshots of compared side by side tree-views from Dolphin and digiKam, just to be sure.

Thanks in advance

Gilles Caulier
Comment 2 Maik Qualmann 2019-08-16 13:26:57 UTC
The sorting is correct here under my Linux developer version and the Windows version. The problem is the AppImage. We have a locale problem. Even with the new AppImage built under Mageia, it continues to exist. You have to start the AppImage with "LANG=C /path/to/digikam.appimage".

Maik
Comment 3 Maik Qualmann 2019-08-16 13:32:07 UTC
The QCollator depends on the locale used. I could imagine that there is no locale in the AppImage and therefore the QCollator is not working properly.

Maik
Comment 4 Maik Qualmann 2019-08-16 13:40:51 UTC
*** Bug 407506 has been marked as a duplicate of this bug. ***
Comment 5 Daniel Laidig 2019-08-16 19:07:56 UTC
Thanks for your amazingly fast reaction! :)

I can verify that this is indeed a locale issue, but the behavior of your workaround is not exactly the same. On the two systems I tested (Fedora 30, Kubuntu 19.04), setting LANG=C changes the sort order to
---
2019 General
2019-01-01 New Year
2019-07 Summer vacation
2019-07-30 Some other album
Stuff
_Important_Top_Album
---
i.e. the date-based names are sorted correctly while the sorting of the underscore is different. In Dolphin and with digikam from the distribution packages (6.1.0 on Fedora, 5.9.0 on Kubuntu), "_Important_Top_Album" is always the top entry.

Starting the Appimage with LANG=C gives a "Your locale has changed since this album was last opened." warning. Therefore, wouldn't the following be a better workaround?

LANG=C.UTF-8 ./digikam-6.2.0-x86-64.appimage

For me, this workaround is good enough. Let me know if I can help, for example with testing a fixed Appimage.
Comment 6 caulier.gilles 2019-08-17 07:51:22 UTC
Daniel,

You can hack yourself the startup shell script inside AppImage to try a work around with the locale problem.

In fact, AppImage is something like ISO9600 disk mounted in virtual disk in memory and attached to a mount point in /tmp. It's very easy to extract the whole contents in your home directory during a digiKam session.

Start the AppImage and look in /tmp. You will found a subdir starting with "digi" prefix. All files in this directory are AppImage in fact in read only.

Just copy the directory from /tmp to your home directory and that all. You have an uncompressed version of AppImage available in read/write.

The main entry in this bundle is the AppRun shell script. This one in fact that we embed while AppImage construction :

https://cgit.kde.org/digikam.git/tree/project/bundles/appimage/data/AppRun

As you can see, there are plenty of customization inside. You can edit this script and test until you found the right solution to your problem.

Best

Gilles Caulier
Comment 7 caulier.gilles 2019-09-15 12:39:14 UTC
Maik, 

I found something important about AppImage and LANG=C stuff.

In fact the dysfunction described here (and certainly other ones in bugzilla), depend of the options used to compile Qt5 for AppImage.

Typically, after Qt5.9, the icu library support for internationalisation become deprecated and iconv must be used instead. Also, under Centos6, libicu become an older dependency, and recompiling this library will make a big puzzle. So i switched the options to compile Qt without icu.

But, now as i see when i migrate from Qt 5.11 to 5.12 LTS, using iconv generate plenty of warning on the console when digiKam is running and QCompleter class used to populate albums/tags list from database.

This particular warnings on the console are common in fact and easy to found in Qt forum. To resume: never compile Qt without icu support, as for the moment, this do not work as expected.

As we migrate the AppImage build host from Centos6 to Mageia6, by chance libicu is a very recent version in Mageia. I check also the Qt5 rpm spec from Mageia git repository to see how Qt is compiled, and i see that ICU still here.

So i currently recompile digiKam 6.4.0 AppImage with ICU support to see if this problem disappear.

Best

Gilles Caulier
Comment 8 Maik Qualmann 2019-09-15 13:21:49 UTC
Interesting. My guess is that the library can not determine the correct locale because we're inside the AppImage. The only question is, where does she get the information from? From "/etc/locale.conf" or from the environment variables?

Maik
Comment 9 caulier.gilles 2019-09-15 13:31:59 UTC
No idea. The Qt code relevant is complex, but QLocale is used in background.

With iconv dependency, Qt 5.12.5 LTS report now these messages on the console at digiKam startup :

Digikam::DPluginLoader::registerGenericPlugins: Generic plugin named "Twitter" registered to Digikam::ImageWindow(0x4325780, name = "Image Editor")
Digikam::DPluginLoader::registerGenericPlugins: Generic plugin named "Video Slideshow" registered to Digikam::ImageWindow(0x4325780, name = "Image Editor")
Digikam::DPluginLoader::registerGenericPlugins: Generic plugin named "YandexFotki" registered to Digikam::ImageWindow(0x4325780, name = "Image Editor")
unknown: Numeric mode unsupported in the posix collation implementation
unknown: Ignoring punctuation unsupported in the posix collation implementation
unknown: Numeric mode unsupported in the posix collation implementation
unknown: Ignoring punctuation unsupported in the posix collation implementation
unknown: Numeric mode unsupported in the posix collation implementation
unknown: Ignoring punctuation unsupported in the posix collation implementation
unknown: Numeric mode unsupported in the posix collation implementation
unknown: Ignoring punctuation unsupported in the posix collation implementation
unknown: Numeric mode unsupported in the posix collation implementation
unknown: Ignoring punctuation unsupported in the posix collation implementation
unknown: Numeric mode unsupported in the posix collation implementation
unknown: Ignoring punctuation unsupported in the posix collation implementation
unknown: Numeric mode unsupported in the posix collation implementation
unknown: Ignoring punctuation unsupported in the posix collation implementation
unknown: Numeric mode unsupported in the posix collation implementation
unknown: Ignoring punctuation unsupported in the posix collation implementation
unknown: Numeric mode unsupported in the posix collation implementation
unknown: Ignoring punctuation unsupported in the posix collation implementation
unknown: Numeric mode unsupported in the posix collation implementation

Searching these strings with google point to QCompleter source code :

https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qcollator_posix.cpp#n49

As you can see it come from QCollator initialization. This class is used to compare string depending of the locale.

https://doc.qt.io/qt-5/qcollator.html

This will explain the dysfunction from this file.
Comment 10 Maik Qualmann 2019-09-15 13:50:45 UTC
QLocale reads the environment variables. We have to put these from the real system into the AppImage.

https://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/tools/qlocale_unix.cpp#n75

Maik
Comment 11 caulier.gilles 2019-09-15 14:12:14 UTC
Here export from bash report this from my account:

declare -x LANG="en_US.UTF-8"
declare -x LANGUAGE="en_US.UTF-8:en_GB:en"


So LANG is present, and there is nothing special in RunApp bash script from AppImage bundle to drop these settings.

Also look well the QLocale code : if LANG is not defined, 'C' is taken as workaround value. So don't understand why we need to force "LANG=C /path/to/digikam.appimage" at AppImage startup...

Gilles Caulier
Comment 12 caulier.gilles 2019-09-15 18:44:15 UTC
Maik,

AppImage is re-built with Qt 5.12.5 and ICU support. This time, at startup, the warning on the console is different :

 = "Digikam")
Digikam::DPluginLoader::registerGenericPlugins: Generic plugin named "Video Slideshow" registered to Digikam::DigikamApp(0x20e7860, name = "Digikam")
Digikam::DPluginLoader::registerGenericPlugins: Generic plugin named "YandexFotki" registered to Digikam::DigikamApp(0x20e7860, name = "Digikam")
unknown: Could not create collator: 4
unknown: Could not create collator: 4
unknown: Could not create collator: 4
unknown: Could not create collator: 4
unknown: Could not create collator: 4
unknown: Could not create collator: 4
...

The status code 4 = U_FILE_ACCESS_ERROR 

the warning is reported by this function:

https://github.com/qt/qtbase/blob/5.12/src/corelib/tools/qcollator_icu.cpp#L55

Gilles
Comment 13 caulier.gilles 2019-09-18 05:47:24 UTC
Maik,

I progressed a little bit with AppImage and locale system support.

1/ I recompiled AppImage with last Qt 5.13.1 and problem still reproducible.
2/ When i run the digiKam version installed under the Mageia6 system used to compile AppImage, it run without ICU warnings from Qt (It's the same binary hosted in AppImage bundle).
3/ When i run the AppImage under Mageia6 build system, the problem is reproducible.
4/ When i run AppRun script from the pre-bundle cache directory of AppImage under Mageia6 build system, the problem is also reproducible.

To conclude: the problem is located somewhere in the bundle, aka wrapper script miss something with env. variables, or a run time deps is missing.

Note : the ICU error sound like the locale to use is not found in ICU data. But these data are not missed, as all data files are located in a shared memory for a faster access. I need to hack Qt Locale/Collator to see which locale is detected exactly when digiKam is initialized.

Note2 : as previous AppImage build with Qt 5.11.3 was compile WITHOUT ICU support, this ini problem was never occurs, but Qt fail back to Posix locale support. This is why we need to setup locale variable previously when we start the bundle.

Note3 : I tried to play with other AppImage as Krita or Kate, but first one do not use ICU, and second one is very very old. The difficulty is to know the right deps to setup to build Qt for AppImage.

Gilles Caulier
Comment 14 caulier.gilles 2019-09-19 15:06:07 UTC
Maik,

I advance more and more in locale support problem with AppImage.

Now i can reproduce the problem with the digiKam compiled for the AppImage but installed and executed on the compilation system.

Typically, before to process bundle packaging, whole necessary libraries and programs are compiled and installed. This is true also for digiKam executable. This is the same process than to compile on a target computer, excepted that all is automatized.

So, the /usr/bin/digikam can be started and used as well, _without_ any locale  problem reported by ICU libraries.

And now, i can reproduce the warning from QCollator at run time. Remember the error code :

"Could not create collator for "en" :: ICU return value: 4" 

This string is an adjusted qWarning in QCollator init for icu backend. 4 is for file not found on the system.

The question is : which file is not found?

Well i suspected that configuration files are not parsed, but it's not the case. I can reproduce this warning on the system when i run digiKam, when i rename /usr/share/icu to *.old. In this folder libicu store a large .dat file with all char conversion tables (or something like that). If i read the icu doc, it's clear that this kind of data can be taken from a icu shared library to optimize speed. This one is hosted and even pre loaded in AppImage bundle, but for obscure reasons, it's not used and the .dat file is used instead, and of course not found in the bundle, as i don't include /usr/share/icu in AppImage.

But, if i include this directory at the same place in the bundle, the problem is not solved...

Gilles
Comment 15 Maik Qualmann 2019-09-19 18:52:41 UTC
I have now also played with the paths to the ICU data here and have no solution yet...

Maik
Comment 16 Maik Qualmann 2019-09-19 19:19:41 UTC
If I understand the instructions to compile the libicu correctly, the path to the data must be specified during compilation. It is also possible to disable with an option that the path can be set via an environment variable.

Maik
Comment 17 caulier.gilles 2019-11-07 09:59:38 UTC
*** Bug 413842 has been marked as a duplicate of this bug. ***
Comment 18 caulier.gilles 2022-01-19 19:09:15 UTC
Git commit 00e4d5da2948ccdc101cdeeb23f7b05405812436 by Gilles Caulier.
Committed on 19/01/2022 at 19:04.
Pushed by cgilles into branch 'master'.

Great news : AppImage with Qt 5.15.2 compiled under Mageia 7.1 now support ICU
This require to recompile whole AppImage build system on Continuous Deployement server.
This will be done in a few days. I only tested here on my laptop, and i can confirm that ICU work fine now.
Related: bug 406583, bug 425168, bug 418670, bug 407506, bug 413842

M  +2    -3    project/bundles/3rdparty/ext_qt/5.15/CMakeLists.txt

https://invent.kde.org/graphics/digikam/commit/00e4d5da2948ccdc101cdeeb23f7b05405812436
Comment 19 caulier.gilles 2022-02-05 17:37:57 UTC
Hi,

With the ICU support introduced in next 7.6.0 release, this problem is now fixed.

You can test using 7.6.0 pre-release AppImage bundle available here : https://files.kde.org/digikam/

Best Regards

Gilles Caulier
Comment 20 Maik Qualmann 2022-02-06 18:44:54 UTC
I confirm that the sorting in the AppImage is now correct.

Maik
Comment 21 caulier.gilles 2022-02-06 21:09:55 UTC
Hi Maik,

With the Qt 5.15 LTS (annotated 5.15.3 in help/component dialog), we inherit of qtWebEngine 5.15.8.

Qt detect iconv APi (from glib) and can compile with it but not qtWebEngine as it expect libxml compiled with utf8 support through iconv. 
For that qt is not compiled with iconv support at all, which is different than qt 5.15.2 or qt 5.14.x used previously in AppImage bundle.

Do you know how is used iconv API in Qt ? Perhaps in QString and other container as QByteArray ?  This can have a side-effect in digiKam ?

https://www.man7.org/linux/man-pages/man1/iconv.1.html

I can patch qtWebEngine to disable iconv support as in fact with this Qt component, i'm sure that we don't care at all...

Gilles