Bug 344614 - Dolphin (frameworks branch) doesn’t find filelight
Summary: Dolphin (frameworks branch) doesn’t find filelight
Status: RESOLVED FIXED
Alias: None
Product: frameworks-kservice
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Gregor Mi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-02-27 08:34 UTC by Philipp A.
Modified: 2015-04-10 14:21 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Philipp A. 2015-02-27 08:34:49 UTC
$ locate filelight.desktop
/usr/share/applications/org.kde.filelight.desktop

but clicking on the free space indicator says
Filelight [not installed]
KDiskFree [not installed]

Reproducible: Always
Comment 1 Gregor Mi 2015-02-27 10:19:36 UTC
Hi I can reproduce the behaviour.

Preliminary analysis:
The Filelight service is found via `KService::serviceByDesktopName("org.kde.filelight")`

On my openSUSE 13.2 I got
locate filelight.desktop
/home/gregor/dev/kf5/usr/share/applications/org.kde.filelight.desktop
/usr/share/applications/kde4/filelight.desktop

So there is a difference between the installed desktop file names.

If I change the call to `KService::serviceByDesktopName("filelight")` is works for me but this will probably not fix it in your situation. I'll look into it.
Comment 2 Gregor Mi 2015-02-27 10:32:12 UTC
This patch would solve it for my case:

    fix to respect old filelight desktop file name

diff --git a/dolphin/src/statusbar/spaceinfotoolsmenu.cpp b/dolphin/src/statusbar/spaceinfotoolsmenu.cpp
index bce5ba1..55dfc58 100644
--- a/dolphin/src/statusbar/spaceinfotoolsmenu.cpp
+++ b/dolphin/src/statusbar/spaceinfotoolsmenu.cpp
@@ -34,7 +34,11 @@ SpaceInfoToolsMenu::SpaceInfoToolsMenu(QWidget* parent, QUrl url)
 
     // find service
     //
-    const auto filelightService = KService::serviceByDesktopName("org.kde.filelight");
+    auto filelightService = KService::serviceByDesktopName("org.kde.filelight");
+    const auto filelightServiceOld = KService::serviceByDesktopName("filelight"); // the old desktop file is named like this
+    if (!filelightService && filelightServiceOld) {
+        filelightService = filelightServiceOld;
+    }
     if (filelightService && filelightService->isApplication()) {
         const auto filelightIcon = QIcon::fromTheme(filelightService->icon());
Comment 3 Gregor Mi 2015-02-27 10:52:09 UTC
I uninstalled my system filelight, now only this one is left:
/home/gregor/dev/kf5/usr/share/applications/org.kde.filelight.desktop

```
KService::Ptr KServiceFactory::findServiceByDesktopName(const QString &_name)
{
    if (!m_nameDict) {
        return KService::Ptr();    // Error!
    }

    // Warning : this assumes we're NOT building a database
    // KBuildServiceFactory reimplements it for the case where we are building one

    int offset = m_nameDict->find_string(_name); 
    if (!offset) {
        return KService::Ptr();    // Not found    <---------------
    }
...
```

KService does not find the org.kde.filelight.desktop.
Comment 4 Gregor Mi 2015-03-04 11:33:39 UTC
I did `kbuildsycoca5` and `eval `dbus-launch`` but no success. @faure: Are we missing something obvious?
Comment 5 Gregor Mi 2015-03-14 09:43:09 UTC
I debugged kbuildsycoca5:
```
KBuildSycoca::build()
...
            Q_FOREACH (const KSycocaEntry::Ptr &entry, list) {
                qDebug() << entry->name() << entry->storageId();
...
```

It looks like the org.kde.filelight.desktop file is indeed picked up.

The harder part is to debug `KService::Ptr KServiceFactory::findServiceByDesktopName(const QString &_name)` since it uses KSycocoaDict which seems to be some kind of custom hash map implementation which has no method to get all keys. So it is hard to get an overview of all registered entries.
Comment 6 Gregor Mi 2015-03-14 09:59:09 UTC
Interesting: KServiceFactory::findServiceByDesktopName has got this call:

int offset = m_nameDict->find_string(_name);

It returns a non-zero 6 digit number for kate (which is found by KService) and also for org.kde.filelight (which is not found). Even if the input name is the same, the number varies from call to call.

The offset is then used to do this: KService::Ptr newService(createEntry(offset));

Then a check is performed:
if (newService && (newService->desktopEntryName() != _name)) { ... }

For kate `newService->desktopEntryName()` it always returns "kate"
For org.kde.filelight it always returns "plasmanetworkmanagement_openswanui". This mismatch case is an expected one as the code comment says "// Check whether the dictionary was right.". So there seem tobe cases where the dictionary is wrong. Any idea what can be wrong or how it can be fixed?
Comment 7 Gregor Mi 2015-03-14 10:16:15 UTC
Two further findings:

1) With KServiceFactory::findServiceByDesktopName("filelight") the offset is 0 which is ok because the KDE4 version of Filelight is not installed

2) With KServiceFactory::findServiceByDesktopName("org.kde.filelight") the offset is non-zero. So it seems to be registered in some way. I brute-force iterated through the offsets from 0..2.000.000 to look if newService->desktopEntryName() contains the string "filelight" but it does not.
Comment 8 Gregor Mi 2015-03-14 11:25:01 UTC
In KBuildServiceFactory::addEntry there is

const KService::Ptr service(static_cast<KService*>(newEntry.data()));

service->desktopEntryName() returns only "org" for the filelight desktop file.
Comment 9 Gregor Mi 2015-03-14 11:57:08 UTC
kservice.cpp: void KServicePrivate::load(QDataStream &s)

    s >> ...
      >> m_strDesktopEntryName
    ...

This deserializes "org" instead of "org.kde.filelight"
Comment 10 Gregor Mi 2015-03-14 13:47:01 UTC
What I currently could not find out where and how the QDataStream is initially written to. I don't think that the deserialize method somehow splits and the wrong dot. So it must be when the QDataStream data is created. But where? Any idea?
Comment 11 Philipp A. 2015-03-19 13:23:47 UTC
(In reply to Gregor Mi from comment #2)
> This patch would solve it for my case:
> 
>     fix to respect old filelight desktop file name
> 
> ...

for the record: this patch doesn’t affect this bug, since i and others indeed have org.kde.filelight.desktop (the new name), but it’s still not found.
Comment 12 Gregor Mi 2015-03-19 19:20:01 UTC
Thanks for confirming the fact. I can also say that the dolphin patch from comment #2 is not fixing the problem at all. As shown in my latest comments there is a bug in frameworks-kservice (the filename of filelight's desktopfile is split at the wrong dot). I changed the product of this bug accordingly.
Comment 13 Andreas Hartmetz 2015-04-01 15:12:17 UTC
Gregor: It looks to me like the value is written in ksycocadict.cpp line 535:
str << (*dup)->keyStr;                // Key (QString)

Now the question is how does it get there - it's hopefully not too hard to "trace back" the bad value with some debug output.
Comment 14 Andreas Hartmetz 2015-04-01 15:21:33 UTC
Actually, by simply grepping for '.' I found this:
kservice.cpp line 154 ff:
    pos = _name.indexOf(QLatin1Char('.'));
    if (pos != -1) {
        _name.truncate(pos);
    }

... yeah, that might be not entirely correct.
I haven't tested it at all or verified that this is the place where things break.
Comment 15 Gregor Mi 2015-04-02 10:55:02 UTC
Thanks for looking. I already had some debug statements around that code but somehow I ruled it out at that time. I tried again and saw that it does indeed does the splitting wrong. I'll provide a patch.

Currently, I cannot fully test it with dolphin because for every item in the menu I get an error message:
- Service '/usr/share/applications/kde4/kdf.desktop' is malformatted.
- Service '/home/gregor/dev/kf5/usr/share/applications/org.kde.filelight.desktop' is malformatted.

But at least filelight is found.
Comment 16 Gregor Mi 2015-04-02 11:48:11 UTC
Here is the patch: https://git.reviewboard.kde.org/r/123223/
Comment 17 Gregor Mi 2015-04-10 14:21:11 UTC
Git commit 1782880ce2586d9d8633abfeb05c290907e26658 by Gregor Mi.
Committed on 10/04/2015 at 14:19.
Pushed by gregormi into branch 'master'.

Fix wrong splitting of entry path

REVIEW: 123223

M  +15   -0    autotests/kservicetest.cpp
M  +2    -0    autotests/kservicetest.h
M  +4    -9    src/services/kservice.cpp
M  +1    -0    src/services/kservice_p.h
A  +50   -0    src/services/kserviceutil_p.h     [License: LGPL (v2)]

http://commits.kde.org/kservice/1782880ce2586d9d8633abfeb05c290907e26658