GNOME Bugzilla – Bug 156917
Use window-menu from libwnck
Last modified: 2005-01-25 19:21:50 UTC
Why are we building the window menu in gnome-panel when there's a window-menu in libwnck? The code from libwnck is far from complete, but I think we should move the code from gnome-panel there and just use what libwnck gives us.
The code for window-menu is indeed confusing. The other wncklets seem to use gnome-panel/applets/wncklet code for the applet and libwnck for the actual widgets.
What would be gained by doing this, apart from code consistency ? If there's no real gain, why not just delete window-menu.c in libwnck ?
Vincent: it'd make it easier to be consistent with what the tasklist is doing.
I never knew about window-menu in libwnck. Sure, it makes sense to finish and use that.
Vincent: if you'll work on this, please tell me so we don't duplicate efforts :-)
Well I've begun to look into it, but it's trickier than I thought. What I did right now : * I moved all the widget-specific code into libwnck's window-menu. * In libwnck's window-menu, I've made the window-menu a real widget, descending from a GtkMenuBar. It looks like that should work. * I removed all the widget-specific stuff from gnome-panel's window-menu. However, I'm kinda stuck here. Every time I launch the applet, it cannot access the list of opened windows : wnck_screen_get_windows returns nothing. I cannot figure out what's going on - I tried setting the screen number myself etc but nothing works.
Also I've renamed the widget from libwnck's window-menu to a WnckSelector (and used "selector" everywhere else). This was to clarify things (otherwise we would end up with two "window_menu" structures everywhere). The same thing was done for the window-list (the wnck widget is called WnckTasklist).
Created attachment 35977 [details] [review] patch for libwnck Ok, here is the first patch for libwnck. It moves all the menu widget handling into libwnck/selector.c. I have changed the name of the file, as everything was re-written and window-menu.c was never used anyway, so we shouldn't care about the file history. Also, it's more consistent as the widget itself is called WnckSelector. I've also fixed a small bug that was introduced by the icon-dimming patch yesterday :)
Created attachment 35978 [details] [review] Second patch for gnome-panel Here is the second patch for gnome-panel. It removes all the widget handling from gnome-panel/window-menu. Nothing exciting here, except if you're particularly fond of code removal ;-)
Created attachment 35979 [details] [review] Second patch for gnome-panel Here is the second patch for gnome-panel. It removes all the widget handling from gnome-panel/window-menu. Nothing exciting here, except if you're particularly fond of code removal ;-)
Crap. Don't pay any attention to the double-post, please... :P A few notes : * I'm using the patched version of the applet right now, everything looks normal. * Keyboard navigation works * Keyboard focusing works... * I'm sure I screwed up the indentation somewhere :D
Created attachment 35982 [details] [review] patch for libwnck The fix for yesterday's bug was worse than the bug - icons for minimized apps would slowly disappear as you used the applet. This new patch is hopefully correct.
Vincent : do you think this could go into 2.10 ?
Vincent: I'm not sure... It's ok for the gnome-panel side of the problem, but you have to ask Havoc or Mark for the libwnck side.
It doesn't mean the libwnck patch is not ok (it looks ok), but I'm not a libwnck maintainer. I'm just wondering why some static functions are wnck_selector_*, some other are selector_* and some other don't have selector in the name. > * I'm sure I screwed up the indentation somewhere :D Should I mention that the indention rules are not the same in libwnck and in gnome-panel? ;-)
Created attachment 36031 [details] [review] Second patch for libwnck (w/ indent) Here is a new patch for libwnck, that tries to be more consistent regarding the wnck_* prefix. I have also (I hope) followed the libwnck indentation standard (2 spaces by indent, no tabs). I have a newfound respect for the indent command :-) How do we put this bug on libwnck's maintainers radar ?
You should probably send them a mail.
I opened bug 164474 for the libwnck part of this bug.
Vincent : the libwnck part is now committed, is it ok to commit the gnome-panel part ?
Vincent: please wait a bit. I'm releasing the panel today, and it'd be a shame to have an unusable applet (libwnck 2.9.90 was already released) :-)
No problem :)
Comment on attachment 35979 [details] [review] Second patch for gnome-panel Ok to commit now.
I committed an updated version of the patch (due to small changes required by Havoc in bug 164474, namely the use of a "private" structure, meaning the window selector applet does not use anything inside the WnckSelector widget, which is better). I guess now many of the window selector applet bugs will have to be refiled against libwnck ?
Vincent: I'll take care of this. Thanks for your work!