Merge lp:~tribaal/unity-tweak-tool/fix-show-desktop-core-dump into lp:unity-tweak-tool

[Chris Glass]

Only activate the "Show desktop" option of the "Window spread" section if the option is available.

This fixes the linked bug by only activating the "Show desktop" option of the "Window spread" section (labeled as "Click to access desktop") if the option is actually available.

Pretty straightforward check. While this was not tested on previous ubuntu versions it should work fine on saucy (since the key will be defined there).

[J Phani Mahesh (phanimahesh)]

Hi Chris!

Thanks for the patch. This solves the problem partially. The other bit that is missing is to unsensitize the checkbox, so that the user can not interact with it, when the key is missing. We are good to merge after adding that.

Cheers,
Mahesh.

[Chris Glass (tribaal)]

It's already never activated - that's what set_active() does.

Here's a screenshot with my patch applied. Notice how the tickbox is disabled already.

[Chris Glass (tribaal)]

Oh, of course, I forgot the URL for the screenshot:)

http://imgur.com/FERyoy3

[J Phani Mahesh (phanimahesh)]

I'm looking at it right now. Should be merged in ten minutes if that is the case.

[J Phani Mahesh (phanimahesh)]

No, set_active changes the state of the checkbox, ticked vs unticked. set_sensitive(True|False) should be used to make the widget usable/unusable.

And I just checked the UI definitions, it is set to be sensitive by default. I'll need to hunt down where it is getting unsensitized, and if it is not being unsensitized, will have to add that bit. This is going to take some time..

I'll comment back in an hour or so. If you have some time to spare, try to identify where set_sensitive(False) is being called on the checkbox.

[lp:~tribaal/unity-tweak-tool/fix-show-desktop-core-dump]

179. By Chris Glass on 2014-02-20

Added explicit desensitization.

[Chris Glass (tribaal)]

So, the checkbox cannot be toggled on or off and shows as "grey" (please see screenshot).

I added an explicit desensitization in case the show-desktop key is not found. While it doesn't change the behaviour at all it should be quite clear that this won't break in any way now.

[J Phani Mahesh (phanimahesh)]

Found it. It has been taken out of spaghetti long ago, but the reference has not been deleted from spaghetti. there is a checkbox defined in section/windowmanager.py (L188) which automatically takes care of checking the key, and unsensitizing the widget if it is not found.

So the correct patch would be to delete all references to check_click_desktop from the section/spaghetti/compiz.py
Well, all except where it appears as a dependant.

Our bzr repo is actually a mirror of git repo, which gets rewritten daily by LP bots. Would you like to submit a pull request on github, or mail me a patch? Or would you prefer that I commit the changes myself?

[Chris Glass (tribaal)]

So if submitting these changes to github will make the merge, sure, I'll open a PR in a minute.

However I'm not sure I'll have time and dedication enough to attempt the refactoring you describe. Maybe a middle ground would be to merge this in (via github) now to fix the core dump for current users, then handle the refactoring separately?

[J Phani Mahesh (phanimahesh)]

Well, the refactoring has been done. We missed removing the bits in spaghetti. (the code is duplicated currently)

All you have to do is, in the compiz.py file you were editing, delete the function on_check_click_desktop_toggled (Line 721) and the line you were trying to edit earlier. That fixes the crash.

[J Phani Mahesh (phanimahesh)]

Merged on github.

=== modified file 'UnityTweakTool/section/spaghetti/compiz.py'
--- UnityTweakTool/section/spaghetti/compiz.py	2013-05-13 02:55:19 +0000
+++ UnityTweakTool/section/spaghetti/compiz.py	2014-02-20 13:01:52 +0000
@@ -428,7 +428,10 @@
         else:
             self.ui['check_overlay_emblem'].set_active(False)
 
-        self.ui['check_click_desktop'].set_active(gsettings.scale.get_boolean('show-desktop'))
+        if 'show-desktop' in gsettings.scale.list_keys():
+            self.ui['check_click_desktop'].set_active(gsettings.scale.get_boolean('show-desktop'))
+        else:
+            self.ui['check_click_desktop'].set_sensitive(False)
 
         model = self.ui['list_compiz_windows_spread_accelerators']