Merge lp:~a-j-buxton/xfpanel-switch/orphan-plugins into lp:~xfpanel-switch-dev/xfpanel-switch/trunk

Proposed by Alistair Buxton
Status: Merged
Merged at revision: 146
Proposed branch: lp:~a-j-buxton/xfpanel-switch/orphan-plugins
Merge into: lp:~xfpanel-switch-dev/xfpanel-switch/trunk
Diff against target: 84 lines (+39/-7)
1 file modified
xfpanel-switch/panelconfig.py (+39/-7)
To merge this branch: bzr merge lp:~a-j-buxton/xfpanel-switch/orphan-plugins
Reviewer Review Type Date Requested Status
Xfce4 Panel Profiles Team Pending
Review via email: mp+343664@code.launchpad.net

Commit message

Fix (LP: #1765565): Don't crash on corrupted panel configs.

1. Fix the problem directly:

Catch the exception if the .desktop file is missing and return false.
This will remove launchers with missing .desktop (not just the binary).

2. Fix the problem more generally:

Ignore orphan plugin configurations. This is a plugin configuration
section which is not refered to by any panel and therefore is not used.
Some old versions of xfce4-panel seem to create these, but I can't
reproduce with the latest version.

These orphans are completely ignored by the panel. Xfpanel-switch also
ignores them, unless they are launchers with missing .desktop files.
The second part of this patch cleans them out anyway.

Description of the change

This is tricky to test as it requires you to have a previously unidentified corruption in your panel config. However, the reporter reported that it worked on IRC. I have not heavily tested it, but it appears to have saved my configuration okay too.

In order to test it you'll have to intentionally break the panel config: add a bunch of launchers, plugins etc, then delete them from panels/panel-N/plugin-ids using settings editor so that they become orphans. Then save the configuration and load it again. The orphans should be gone.

Next add a launcher and delete the .desktop from ~/.config/xfce4/panel/. Again save and load and thebroken launcher should be gone.

In neither case should it crash. :)

To post a comment you must log in.
143. By Alistair Buxton

Be careful about which plugins are deleted.

If the key /plugins/plugin-2 is deemed to be bad, we should not delete
/plugins/plugin-20 for example. So check that the keys to be deleted
either exactly match, or start with the pattern + '/'.

144. By Alistair Buxton

Check for orphans also when loading a config.

Because of the way the configuration copying is used, sometimes orphans
can persist in configs (because they don't go through xfconf.)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'xfpanel-switch/panelconfig.py'
--- xfpanel-switch/panelconfig.py 2018-04-13 21:34:06 +0000
+++ xfpanel-switch/panelconfig.py 2018-04-20 05:14:55 +0000
@@ -61,6 +61,7 @@
6161
62 pc.properties[pp] = pv62 pc.properties[pp] = pv
6363
64 pc.remove_orphans()
64 pc.find_desktops()65 pc.find_desktops()
6566
66 pc.source = None67 pc.source = None
@@ -80,12 +81,40 @@
80 except:81 except:
81 pass82 pass
8283
84 pc.remove_orphans()
83 pc.find_desktops()85 pc.find_desktops()
8486
85 return pc87 return pc
8688
89 def remove_orphans(self):
90 plugin_ids = set()
91 rem_keys = []
92
93 for pp, pv in self.properties.items():
94 path = pp.split('/')
95 if len(path) == 4 and path[0] == '' and path[1] == 'panels' and \
96 path[2].startswith('panel-') and path[3] == 'plugin-ids':
97 plugin_ids.update(pv)
98
99 for pp, pv in self.properties.items():
100 path = pp.split('/')
101 if len(path) == 3 and path[0] == '' and path[1] == 'plugins' and \
102 path[2].startswith('plugin-'):
103 number = path[2].split('-')[1]
104 try:
105 if int(number) not in plugin_ids:
106 rem_keys.append('/plugins/plugin-' + number)
107 except ValueError:
108 pass
109
110 self.remove_keys(rem_keys)
111
87 def check_desktop(self, path):112 def check_desktop(self, path):
88 bytes = self.get_desktop_source_file(path).read()113 try:
114 bytes = self.get_desktop_source_file(path).read()
115 except FileNotFoundError:
116 # If the .desktop file does not exist at all return False
117 return False
89118
90 # Check if binary exists119 # Check if binary exists
91 keyfile = GLib.KeyFile.new()120 keyfile = GLib.KeyFile.new()
@@ -99,7 +128,7 @@
99 return False128 return False
100129
101 def find_desktops(self):130 def find_desktops(self):
102 remove_keys = []131 rem_keys = []
103132
104 for pp, pv in self.properties.items():133 for pp, pv in self.properties.items():
105 path = pp.split('/')134 path = pp.split('/')
@@ -114,13 +143,16 @@
114 if self.check_desktop(desktop_path):143 if self.check_desktop(desktop_path):
115 self.desktops.append(desktop_path)144 self.desktops.append(desktop_path)
116 else:145 else:
117 remove_keys.append('/plugins/plugin-' + number)146 rem_keys.append('/plugins/plugin-' + number)
118147
148 self.remove_keys(rem_keys)
149
150 def remove_keys(self, rem_keys):
119 keys = list(self.properties.keys())151 keys = list(self.properties.keys())
120 for param in keys:152 for param in keys:
121 for bad_plugin in remove_keys:153 for bad_plugin in rem_keys:
122 if param.startswith(bad_plugin):154 if param == bad_plugin or param.startswith(bad_plugin+'/'):
123 self.properties.pop(param, None)155 del self.properties[param]
124156
125 def get_desktop_source_file(self, desktop):157 def get_desktop_source_file(self, desktop):
126 if getattr(self, 'source', None) is None:158 if getattr(self, 'source', None) is None:

Subscribers

People subscribed via source and target branches