DBus Menu

Merge lp://qastaging/~charlesk/libdbusmenu/lp-953509 into lp://qastaging/libdbusmenu/0.6

  1. lp-953509
  2. Merge into trunk.0.6
Proposed by Charles Kerr
Status: Merged
Approved by: Charles Kerr
Approved revision: 389
Merged at revision: 388
Proposed branch: lp://qastaging/~charlesk/libdbusmenu/lp-953509
Merge into: lp://qastaging/libdbusmenu/0.6
Diff against target: 116 lines (+0/-51)
1 file modified
libdbusmenu-gtk/parser.c (+0/-51)
To merge this branch: bzr merge lp://qastaging/~charlesk/libdbusmenu/lp-953509
Reviewer Review Type Date Requested Status
Michael Terry Approve
Lars Karlitski (community) Approve
Review via email: mp+98621@code.qastaging.launchpad.net

Description of the change

Don't listen for "changed" events from the screen's default GtkIconTheme. Fixes lp bug #953509

The crash in bug #953509 was being caused by the last two lines of theme_changed_cb() (a) leaving a dangling handler id in priv.theme_changed_sig, and (b) not remembering the handler id of its own signal connection. However after testing I don't see any reason to keep any of the theme handling code at all.

"But wait," you say. "How will our menu icons follow the theme changes?" It works anyway because we always listen for property changes to our reference GtkImage, **and** we listen for property changes to its GtkImageMenuItem parent so that if the GtkImageMenuItem changes GtkImages we can stop listening to the old one and start listening to the new one.

To post a comment you must log in.
Revision history for this message
Lars Karlitski (larsu) wrote :

Yes!

review: Approve
Revision history for this message
Michael Terry (mterry) wrote :

Seems fine. Thanks for cleaning up a mess I believe I introduced! :)

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

to all changes: