Merge lp://qastaging/~attente/unity-gtk-module/gtk-enable-mnemonics into lp://qastaging/unity-gtk-module/14.04

PASSED: Continuous integration, rev:315
http://jenkins.qa.ubuntu.com/job/unity-gtk-module-ci/23/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-amd64-ci/9
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-armhf-ci/9
        deb: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-armhf-ci/9/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-gtk-module-ci/23/rebuild

trying to dynamically handle the settings changing at runtime is maybe a bit insane.... but assuming that indeed you do want to do this:

unity_gtk_menu_shell_handle_item_notify() only uses the pspec to get the name -- why not just modify this function to take the name directly, removing the need to lookup the pspec...

keeping a gtksettings per menuitem is definitely overkill -- it's exceptionally unlikely that we will ever see more than one gtksettings per program. just use the global one. cache it in a static if you really don't trust that it will never change...

since you will just use the global gtksettings, no need to remove/readd the signal handler every time. also: don't bother with handler ids: they don't really make much of an improvement in performance. just disconnect by funcs/data.

finally: you shouldn't try to do this for every single menu item: only for toplevel ones in menubars.

one unrelated note: your underscore removal algorithm's attempt to be correct on non-ascii utf8 strings is not really necessary. all non-ascii utf8 sequences are composed entirely of non-ascii characters (ie: every byte is > 0x80). it is therefore impossible for '_' to appear in the middle of the sequence. just do the naive thing here and it will be correct.

about the strdup: the strlen() + 1 on sized new is not necessary with gstring -- it adds the nul internally. maybe skip GString here entirely since you know that the result string will definitely be smaller than the input.

also: with respect to my previous comments, strchr() is your friend.

I added the signal handler connection/disconnection in the unity_gtk_menu_shell_set_menu_shell () function because sometimes the GtkMenuShell can be set to NULL, and it makes sense to no longer watch once that happens. If you're worried about it being connected/disconnected more than once, in practice that function is never called with a non-NULL GtkMenuShell except at initialization.

PASSED: Continuous integration, rev:318
http://jenkins.qa.ubuntu.com/job/unity-gtk-module-ci/24/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-amd64-ci/10
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-armhf-ci/10
        deb: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-armhf-ci/10/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-gtk-module-ci/24/rebuild

This is better, but still not exactly what I had in mind. I wouldn't block on further review bickering at this point, though.

What I was thinking of (and did a relative poor job of explaining) is one global default GtkSettings watch (using gtk_settings_get_default) for the entire thing. When it fires, then you can iterate the toplevel items and make the tweak.

PASSED: Continuous integration, rev:319
http://jenkins.qa.ubuntu.com/job/unity-gtk-module-ci/25/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-amd64-ci/11
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-armhf-ci/11
        deb: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-armhf-ci/11/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-gtk-module-ci/25/rebuild

PASSED: Continuous integration, rev:320
http://jenkins.qa.ubuntu.com/job/unity-gtk-module-ci/26/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-amd64-ci/12
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-armhf-ci/12
        deb: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-armhf-ci/12/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-gtk-module-ci/26/rebuild

Keeping the global list is really strange -- particularly considering that in practice there will only ever be exactly one menubar. It would have been better to just hook directly up to each (of the one and only) toplevel shell instead of bothering with all of this register stuff...

This is totally fine as-is, though...

PASSED: Continuous integration, rev:321
http://jenkins.qa.ubuntu.com/job/unity-gtk-module-ci/29/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-amd64-ci/15
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-armhf-ci/15
        deb: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-armhf-ci/15/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-gtk-module-ci/29/rebuild

Just one more nag: this check will never fail:

+ if (gtk_enable_mnemonics_name == NULL)
282 + gtk_enable_mnemonics_name = g_intern_static_string ("gtk-enable-mnemonics");
283 +
284 + if (g_param_spec_get_name (pspec) == gtk_enable_mnemonics_name)
285 + {

because you already have a detailed signal connection.

PASSED: Continuous integration, rev:322
http://jenkins.qa.ubuntu.com/job/unity-gtk-module-ci/30/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-amd64-ci/16
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-armhf-ci/16
        deb: http://jenkins.qa.ubuntu.com/job/unity-gtk-module-trusty-armhf-ci/16/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-gtk-module-ci/30/rebuild

Seems like that's ready to land, thanks guys for the review/iterations!