Merge lp:~vanvugt/ubuntu/raring/ubuntu-settings/fix-1073202 into lp:ubuntu/raring/ubuntu-settings

[Daniel van Vugt]

Don't override show-desktop to Ctrl+Alt+D for new users.
(LP: #1073202)

Or would it make more sense to delete the whole line? That way Compiz should use its hard-coded default for Ubuntu builds (Ctrl+Super+D).

[Didier Roche-Tolomelli (didrocks)]

The real bug is that compiz doesn't overwrite any of the integrated keys over the system one as it should with its own values under some circumstances triggered by autopilot. So, this key is not the only one impacted and I'm afraid that we'll have to set again the same workaround on other keys.

What I would like:
- we find and instrumente why compiz isn't copying its key on startup
- we remove the override in the ubuntu-settings package for any key duplicated from compiz

What do you think?

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

At the moment if Compiz starts with different settings to Gnome then it uses the Gnome one instead of its own. You are suggesting that Compiz should copy its settings over the top of org.gnome.*.

I think that might be a bad idea because org.gnome.* is user-wide and has no sense of profiles. That copy will break the user's Gnome shell/classic settings, and not just apply to Compiz. So I think the current behaviour makes more sense for Compiz to only ever write to its own profile settings, but read from either Gnome (if available) or its own profile.

There might be other reasoning for the current behaviour... Sam?

[Sam Spilsbury (smspillaz)]

> The real bug is that compiz doesn't overwrite any of the integrated keys over
> the system one as it should with its own values under some circumstances
> triggered by autopilot.
> What I would like:
> - we find and instrumente why compiz isn't copying its key on startup

Its not supposed to do that.

The settings-integration code will only /redirect/ reads and writes to and from the nominated integrated keys. It won't actually touch the compiz keys.

If you want to change the value of a gnome key that's integrated into compiz, you need to actually change the value of the key in compizconfig direclty.

Overwriting the integrated keys with the compiz ones when integration is turned on is wrong because the whole point was that we wanted to respect the gnome settings and not the compiz ones.

> So, this key is not the only one impacted and I'm
> afraid that we'll have to set again the same workaround on other keys.
>
> - we remove the override in the ubuntu-settings package for any key duplicated
> from compiz
>
> What do you think?

[Sam Spilsbury (smspillaz)]

> At the moment if Compiz starts with different settings to Gnome then it uses
> the Gnome one instead of its own. You are suggesting that Compiz should copy
> its settings over the top of org.gnome.*.
>
> I think that might be a bad idea because org.gnome.* is user-wide and has no
> sense of profiles. That copy will break the user's Gnome shell/classic
> settings, and not just apply to Compiz. So I think the current behaviour makes
> more sense for Compiz to only ever write to its own profile settings, but read
> from either Gnome (if available) or its own profile.

I'm tying up some loose ends with bug 1063617 which has a lot to do with how we handle the compiz keys in the integrated context, but the changes that need to be made there are not trivial because the integration system was never designed to define values for compiz keys stored in gsettings shadowed by integrated keys.

>
> There might be other reasoning for the current behaviour... Sam?

[Sam Spilsbury (smspillaz)]

At the moment I think this is a correct fix - or at least, the *most* correct fix would be moving <Primary><Super>d to the beginning of the line.

Compiz has no support for multiple keybindings, so it currently just works around that by changing the first one.

So its a +1 from me, but note the above.

review: Approve

[Daniel van Vugt (vanvugt)]

Didier also points out that there are other settings in the override file that we probably want to drop. That's true. However this branch is called fix-1073202 and we hopefully only need to change this one setting to fix bug 1073202.

[Sebastien Bacher (seb128)]

(settings to "Work in progress" so it doesn't sit in the sponsoring queue while blocked on other issues to be resolved, please put it back to needs review once Didier's concerns are addressed)

[Didier Roche-Tolomelli (didrocks)]

So, as said on IRC, this fix will trigger a lot of other side effects.

The real bug is what I explained here: https://bugs.launchpad.net/compiz/+bug/1073202/comments/1, which is a compiz issue.

I'm afraid that we never get to the bottom of this bug (and so the other settings won't be migrated) if we go this way. Then, I'm happy to sync this setting. But some people like pitti are still impacted by this bug (the "compiz doesn't copy the integrated settings the first time I log in, but just on some configuration")

Sam, you told some weeks ago that you knew what was causing this? do you know more about it?

review: Needs Fixing

[Didier Roche-Tolomelli (didrocks)]

@Sam: you are telling that it's not supposed to do that, but it's doing it, clearly. On an ubuntu install, open a guest session, you'll see that the integrated keys (system ones) are copied by compiz from the compiz one, which, for some unknown reason and as discussed on my above command, doesn't happen on some (rare) systems.

[Sam Spilsbury (smspillaz)]

On Tue, Nov 27, 2012 at 10:45 PM, Didier Roche <email address hidden> wrote:
> @Sam: you are telling that it's not supposed to do that, but it's doing it, clearly. On an ubuntu install, open a guest session, you'll see that the integrated keys (system ones) are copied by compiz from the compiz one, which, for some unknown reason and as discussed on my above command, doesn't happen on some (rare) systems.

Right that's a bug and I'm looking into it but its going to take me a
week or two before I have a fully tested solution, sorry.

> --
> https://code.launchpad.net/~vanvugt/ubuntu/raring/ubuntu-settings/fix-1073202/+merge/134424
> You are reviewing the proposed merge of lp:~vanvugt/ubuntu/raring/ubuntu-settings/fix-1073202 into lp:ubuntu/ubuntu-settings.

--
Sam Spilsbury

[Sam Spilsbury (smspillaz)]

On Tue, Nov 27, 2012 at 10:44 PM, Didier Roche <email address hidden> wrote:
> Review: Needs Fixing
>
> So, as said on IRC, this fix will trigger a lot of other side effects.

For those of us who weren't on IRC, can you discuss what those side
effects were?

>
> The real bug is what I explained here: https://bugs.launchpad.net/compiz/+bug/1073202/comments/1, which is a compiz issue.
>
> I'm afraid that we never get to the bottom of this bug (and so the other settings won't be migrated) if we go this way. Then, I'm happy to sync this setting. But some people like pitti are still impacted by this bug (the "compiz doesn't copy the integrated settings the first time I log in, but just on some configuration")

Can you elaborate?

The fact that we have the /wrong value/ set for the gsettings key that
the tests need to read gives a pretty good indication as to why they
fail. Why would compiz looking at its own keys have anything to do
with those tests failing, especially when its own key is actually the
correct one in this circumstance.

>
> Sam, you told some weeks ago that you knew what was causing this? do you know more about it?
> --
> https://code.launchpad.net/~vanvugt/ubuntu/raring/ubuntu-settings/fix-1073202/+merge/134424
> You are reviewing the proposed merge of lp:~vanvugt/ubuntu/raring/ubuntu-settings/fix-1073202 into lp:ubuntu/ubuntu-settings.

--
Sam Spilsbury

[Didier Roche-Tolomelli (didrocks)]

The side effect is that we will only fix that one, but not the other ones. The idea is that we remove the override in the end and let compiz copying the keys.

Here, we are a good test case where we see that compiz doesn't copy them. So, on the long term, having a sane default value is important, but it will be good to understand why compiz doesn't copy them. That's all :)

[Sam Spilsbury (smspillaz)]

On Tue, Nov 27, 2012 at 11:56 PM, Didier Roche <email address hidden> wrote:
> The side effect is that we will only fix that one, but not the other ones. The idea is that we remove the override in the end and let compiz copying the keys.
>
> Here, we are a good test case where we see that compiz doesn't copy them. So, on the long term, having a sane default value is important, but it will be good to understand why compiz doesn't copy them. That's all :)

I can reproduce this programmatically, we don't need to intentionally
keep a bug around for the sake of reminding ourselves that there is an
underlying bug.

> --
> https://code.launchpad.net/~vanvugt/ubuntu/raring/ubuntu-settings/fix-1073202/+merge/134424
> You are reviewing the proposed merge of lp:~vanvugt/ubuntu/raring/ubuntu-settings/fix-1073202 into lp:ubuntu/ubuntu-settings.

--
Sam Spilsbury

[Didier Roche-Tolomelli (didrocks)]

the bug is affecting *real systems*, and doesn't only impact this particular key but others (all the integrated keys) as well.

And so, if I drop the override, as it's what we should do (having the value at only one place, compiz and not two), we'll still have the bug as the default is <default>[]</default>. So this won't fix it.

[Sam Spilsbury (smspillaz)]

Hm, ok.

Well I'll try and get a fix out for the read/write to incorrect places
as soon as I can (its non-trivial so it will take me a while like I
said), but we should keep this in the pipeline - having multiple
values for the key in the override makes no sense anyways as compiz
just takes the first one and uses that.

On Wed, Nov 28, 2012 at 12:11 AM, Didier Roche <email address hidden> wrote:
> the bug is affecting *real systems*, and doesn't only impact this particular key but others (all the integrated keys) as well.
>
> And so, if I drop the override, as it's what we should do (having the value at only one place, compiz and not two), we'll still have the bug as the default is <default>[]</default>. So this won't fix it.
> --
> https://code.launchpad.net/~vanvugt/ubuntu/raring/ubuntu-settings/fix-1073202/+merge/134424
> You are reviewing the proposed merge of lp:~vanvugt/ubuntu/raring/ubuntu-settings/fix-1073202 into lp:ubuntu/ubuntu-settings.

--
Sam Spilsbury

[Didier Roche-Tolomelli (didrocks)]

Thanks Sam!

And yeah, I'm keeping an eye on that one, once you'll get it fixed, I'll direclty remove the override :)

Unmerged revisions

4. By Daniel van Vugt on 2012-11-15

Don't override show-desktop to Ctrl+Alt+D for new users.
(LP: #1073202)

=== modified file 'debian/ubuntu-settings.gsettings-override'
--- debian/ubuntu-settings.gsettings-override	2012-10-29 08:32:31 +0000
+++ debian/ubuntu-settings.gsettings-override	2012-11-15 10:01:23 +0000
@@ -25,7 +25,7 @@
 toggle-maximized = ['<Primary><Alt>KP_5']
 toggle-shaded = ['<Primary><Alt>s']
 unmaximize = ['<Super>Down','<Alt>F5','<Primary><Super>Down']
-show-desktop = ['<Primary><Alt>d','<Primary><Super>d','<Super>d']
+show-desktop = ['<Primary><Super>d']
 
 [org.gnome.desktop.wm.preferences]
 button-layout='close,minimize,maximize:'