GTK+

Merge lp://staging/~3v1n0/gtk/ubuntugtk3-fix-gtkbuilder-menus into lp://staging/~ubuntu-desktop/gtk/ubuntugtk3

  1. ubuntugtk3-fix-gtkbuilder-menus
  2. Merge into ubuntugtk3
Proposed by Marco Trevisan (Treviño)
Status: Merged
Merged at revision: 107
Proposed branch: lp://staging/~3v1n0/gtk/ubuntugtk3-fix-gtkbuilder-menus
Merge into: lp://staging/~ubuntu-desktop/gtk/ubuntugtk3
Diff against target: 260 lines (+66/-31)
2 files modified
debian/changelog (+10/-0)
debian/patches/043_ubuntu_menu_proxy.patch (+56/-31)
To merge this branch: bzr merge lp://staging/~3v1n0/gtk/ubuntugtk3-fix-gtkbuilder-menus
Reviewer Review Type Date Requested Status
Ken VanDine Approve
Ted Gould (community) Approve
Review via email: mp+79448@code.staging.launchpad.net

Description of the change

Fixing bug #849732

I've explained more about this issue on this bug comment: https://bugs.launchpad.net/unity/+bug/849732/comments/25

Read more about the implementation in the bzr commit, by the way I've just added a class function to GtkMenuBar to make it not to disable the accelerators defined in its children when using a GtkMenu (like it's happening with GtkBuilder menus).

To post a comment you must log in.
Revision history for this message
Ted Gould (ted) wrote :

I think that this local variable is unneeded

140 ++ GtkMenuBar *menu_bar;
141 ++

But I think the approach makes sense. I'd describe it slightly differently though. Instead of using the gtk_widget_visible() function we're checking the internal shown variable as that represents the internal state instead of whether it is actually visible to the user.

review: Approve
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Oh, I forgot to remove that variable, sorry... Then I'll update the description.

Thanks for reviewing!

lp://staging/~3v1n0/gtk/ubuntugtk3-fix-gtkbuilder-menus updated
108. By Marco Trevisan (Treviño)

Updated the changelog, and removed a forgotten variable initialization.

Revision history for this message
Ken VanDine (ken-vandine) wrote :

Looks good, thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches