Yes.  In particular something is writing out ~/.local/share/applications/preferred-web-browser.desktop which is just:

--- /usr/share/applications/mozilla-firefox.desktop	2009-06-11 20:39:15.000000000 -0400
+++ /home/walters/.local/share/applications/preferred-web-browser.desktop	2008-04-21 12:04:00.000000000 -0400
@@ -72,3 +72,5 @@
 StartupNotify=true
 X-Desktop-File-Install-Version=0.15
 Categories=Network;WebBrowser;
+X-Panel-Monitor=true
+NoDisplay=true


We might be able to look for X-Panel-Monitor and skip?

An how about fixing the program that is creating this file? If we consider that one .desktop file <=> one distinct program, it should not create files for this use, but rather create a symlink to the actual .desktop file for Firefox. Than we could easily find it's a duplicate.

This approach is better for the said program too, since the command to start the browser, its description *and translations* will be updated with firefox, which is not the case when merely re-creating the file.

Created attachment 137808 [details] [review]
Bug 587548 - Whitelist nodisplay desktop files to be included

Currently we only want to pick up a few targeted .desktop files,
namely Evince and Nautilus.  In the future maybe we'll change
the Evince .desktop but continue to special-case nautilus.

+/* This baroque code avoids writable memory; see dsohowto.pdf */
+static const char *nodisplay_whitelist = {
+  "evince.desktop"
+  "gnome-nautilus.desktop"
+};
+
+static const size_t nodisplay_whitelist_indices[] = {
+    0,
+    sizeof (nodisplay_whitelist[0])
+};
+
+static const char *
+nodisplay_index (int idx)
+{
+  return nodisplay_whitelist + nodisplay_whitelist_indices[idx];
+}

I don't see anything exactly like this in the DSO-howto, and I don't see how it works.

 A) Your strings aren't null terminated, so when you find one, it won't be usable in strcmp() easily fixed by adding \0

 B) sizeof (nodisplay_whitelist[0]) is 1, right? Not easily fixed. The techniques in the DSO howto are all either complex or ugly.

We're not actually a shared library that's *shared*, and I just don't see introducing complexity to save two relocations.

So, why did your patch work? I think it's because it didn't add the NoDisplay entries (any of them) to the cache, but it does include them (all of them) when doing shell_app_system_lookup_basename(). 

I'll attach a very, very stripped down version of your patch that seems to work as a fix for now.

Created attachment 137833 [details] [review]
Don't include NoDisplay items, except for window identification

Searching across NoDisplay desktop items can produce weird
results to the user (including duplicates, and items that
aren't really applications at all.) So, don't include them
normally.

But continue including NoDisplay items when we look up the
desktop file for a window, since we want to catch applications
like Evince and Nautilus which are otherwise NoDisplay.

Ah yes, they should have been null terminated.  Well, it avoids writable memory, but yeah not a big deal.

Your patch is certainly far simpler and makes sense to me.

I pushed the patch, but I'll leave this open until we get all the issues here fixed - see bug 587818 - in case we need to do something different here.

*** This bug has been marked as a duplicate of 587818 ***