! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
Patch: Filter out KDE screensaver files from the menu
Status:
RESOLVED: FIXED
Product:
Xfdesktop
Component:
General

Comments

Description Olivier Fourdan editbugs 2006-06-16 12:42:37 CEST
User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.0.4) Gecko/20060608 Ubuntu/dapper-security Firefox/1.5.0.4
Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.0.4) Gecko/20060608 Ubuntu/dapper-security Firefox/1.5.0.4

As Benny was not in favour of applying my previous patch for libxfce4util (see bug #1894) but was suggesting a fix in the menu instead, here follows a fix that filter out the KDE screensaver (and also xscreensaver) entries in the menu.

It does so by allowing (very simple) wildcards in the blacklist, and adding "*.kss" to the list of blacklisted applications.

For implementation reasons, the hash table has been replaced with a list because the wildcard cannot be used by the hash table functions anyway (anyway, the list of blacklisted apps is so small -less than 10 entries- that a hash table here is a bit superfluous IMHO).

Reproducible: Always

Steps to Reproduce:
Comment 1 Olivier Fourdan editbugs 2006-06-16 12:43:45 CEST
Created attachment 609 
Patch to filter out kss entries from menu
Comment 2 Olivier Fourdan editbugs 2006-06-24 13:06:03 CEST
Created attachment 611 
Patch to filter out kss entries from menu

Same, but don't blacklist xscreensaver
Comment 3 Brian J. Tarricone (not reading bugmail) 2006-06-24 16:10:16 CEST
Why is the blacklist testing only done against TryExec?  This could break the blacklist for the other files I have in there.

Also, it looks like g_find_program_in_path() is used for both TryExec and Exec now.  Doesn't this totally negate your patch in bug 1895?

Or I could be misunderstanding... it's very early and I'm watching soccer ^_~.
Comment 4 Brian J. Tarricone (not reading bugmail) 2006-06-24 16:19:24 CEST
Ok, half-nevermind.  I see you're checking the blacklist for Exec as well, but why are you calling g_find_program_in_path() for both TryExec and Exec.

I'd also prefer to just check the blacklist against Exec, and not TryExec as well, for performance reasons:

1) Items with both TryExec and Exec are checked against the blacklist twice.  No reason for this.

2) Items that actually end up being blacklisted should be the vast minority of items.  Taking the "extra time" to check only Exec against the blacklist after already doing g_find_program_in_path() in TryExec is more acceptable to me.

Actually, there's another solution that I like even better:

1) Extract the Exec field, bail if it doesn't exist.
2) Check Exec against the blacklist, bail if it's in the blacklist.
3) Extract the TryExec field.
4) If TryExec exists, do g_find_program_in_path() on it, bail if it fails.

The downside here is that we "waste" the time extracting the Exec field even when the TryExec test fails.  But that should be a minority situation, I should think.  For performance modeling, I'd think we should assume that the TryExec test will usually succeed.
Comment 5 Olivier Fourdan editbugs 2006-06-24 16:50:40 CEST
Yes, that patch reverts the bug 1895 because g_find_program_in_path() rejects quite a few items that we don't want.

The standards is ambiguous, as it states that if TryExec is given, then search for the TryExec program in path. It doesn't say that if there is no TryExec, the Exec should not be searched too. And gnome impl checks for both.

Moreover, I see no perf. difference, it's still as long as 45 secs. in every case.
Comment 6 Olivier Fourdan editbugs 2006-06-25 12:15:39 CEST
TryExec and Exec rarely point to the same program. That's why both TryExec and Exec are checked again the blaklist.

As for the waste, it's really nothing compared to I/O time.
Comment 7 Olivier Fourdan editbugs 2006-06-25 12:24:46 CEST
Created attachment 616 
That patch does the sequence you've presented

Ok, the patch does what you asked. It also checks for the presence of the Exec program in path too.
Comment 8 Brian J. Tarricone (not reading bugmail) 2006-07-11 20:20:51 CEST
This one should be committed already too...

Bug #1928

Reported by:
Olivier Fourdan
Reported on: 2006-06-16
Last modified on: 2009-07-14

People

Assignee:
Brian J. Tarricone (not reading bugmail)
CC List:
1 user

Version

Attachments

Additional information