Bug 3580 - [patch] Submenu Icons in user-defined menus
Status: RESOLVED FIXED
Alias: None
Product: Openbox
Classification: Unclassified
Component: general
Version: 3.4.6.1
Hardware: All All
: P1 enhancement
Assignee: Dana Jansens
QA Contact:
URL:
: 3331
Depends on:
Blocks: 3657
 
Reported: 2008-03-25 16:41 EDT by KadlSoft
Modified: 2014-10-27 16:07:06 EDT
2 users (show)

See Also:


Attachments
Implements icons in user-defined menus (7.12 KB, patch)
2008-03-25 16:43 EDT, KadlSoft
Menu method for Debian menu with icons (1.79 KB, text/plain)
2008-03-25 16:45 EDT, KadlSoft
Implements icons in user-defined menus (generated by diff -pu) (6.81 KB, patch)
2008-03-25 16:52 EDT, KadlSoft
Support of icons in user-defined menus (for 3.4.7-pre3) (13.97 KB, patch)
2008-04-05 10:35 EDT, KadlSoft
On-demand loading of icons, XML schema updates (6.96 KB, patch)
2008-04-19 07:57 EDT, KadlSoft

Description KadlSoft 2008-03-25 16:41:44 EDT
This patch implements support for icons in user-defined menus into Openbox (the patch is for version 3.4.6.1). Image loading is done using the Imlib2 library. I chose Imlib2 because it's pretty fast, it's easy to use, supports many file formats (tested xpm, gif, jpeg, png) and doesn't introduce too much bloat (it depends :)).


What's new?
Syntax of configuration files (namely rc.xml and menu.xml) has been changed slightly to allow users to associate icons to menu entries. This is done by specifying path to icon file in the new "icon" attribute in "<item>" element, e.g:
<item label="Vim" icon="/usr/share/pixmaps/vim-32.xpm">
  <action name="Execute"><execute>x-terminal-emulator -T Vim -e vim</execute></action>
</item>

If user doesn't want to display any icons in his user-defined menus, he/she can disable icons in rc.xml, inside "<menu>" section:
<menu>
 .
 .
 .
  <showIcons>no</showIcons>
 .
 .
 .
</menu>
Default value is "yes".
(New boolean variable "config_menu_user_show_icons" has been added to source code.)

An icon is loaded (using menu_item_attach_icon()) when a new entry of menu is created. Fortunately, I haven't notice any performance problems because of this :).


The dark side of the patch
* Imlib2 is a new run-time dependency (unfortunately, I don't have enough experience with autoconf to add an option like --disable-icons-in-menus).
* There's no cache for icons. If one icon happens to be in menu 100 times, it will be loaded 100 times (but we don't need to bother with cache coherency :)). However, adding some caching shouldn't be difficult.


How to get icons working in Debian menu
To have icons in your Debian menu, you need to update your /etc/menu-methods/openbox (see attachment for example) file and run "update-menus" as a root.
Comment 1 KadlSoft 2008-03-25 16:43:24 EDT
Created attachment 1709 [details]
Implements icons in user-defined menus
Comment 2 KadlSoft 2008-03-25 16:45:29 EDT
Created attachment 1710 [details]
Menu method for Debian menu with icons
Comment 3 KadlSoft 2008-03-25 16:52:17 EDT
Created attachment 1711 [details]
Implements icons in user-defined menus (generated by diff -pu)
Comment 4 Mikachu 2008-03-25 17:10:35 EDT
dana fyi, i put this in a branch imlib2 in my git repo on icculus too.
Comment 5 KadlSoft 2008-04-05 10:35:20 EDT
Created attachment 1722 [details]
Support of icons in user-defined menus (for 3.4.7-pre3)

I ported the patch to 3.4.7-pre3 and added some enhancements. Caching is much better now, and icons can be disabled at compile time using --disable-imlib2 option.
Please note that before applying this patch, the patch for bug #3589 must be applied first.
Comment 6 KadlSoft 2008-04-19 07:57:30 EDT
Created attachment 1729 [details]
On-demand loading of icons, XML schema updates

This patch updates previous patch (id=1722).
I realized that I had accidentaly forgotten to include updated XML schema (data/menu.xsd, data/rc.xsd) in previous patch. This patch includes the update.
Moreover, icon-handling code i openbox/menu.c has been changed, so icons are loaded when they are needed. This speeds start up of Openbox a bit.
Comment 7 Mikachu 2008-05-20 14:07:28 EDT
*** Bug 3331 has been marked as a duplicate of this bug. ***
Comment 8 Mikachu 2010-01-11 17:57:21 EST
This patch has been applied to git, and should appear in 3.5.
Comment 9 Dana Jansens 2010-01-12 13:56:19 EST
oh the last patch is different from what was in your tree mika.  i'll have to look through it and apply some of the differences.
Comment 10 Dana Jansens 2010-01-12 17:40:31 EST
im not comfortable with on-demand loading blocking the menu appearing.  it should be in a separate thread or else done beforehand.

right now the resizing is still blocking the menu appearing, maybe we should force them to be resized appropriately when loaded, rather than waiting until we want to display it.
Comment 11 Mikachu 2010-04-14 12:12:32 EDT
should we close this now that it's in work? I still want the submenu icons though. :)
Comment 12 Dana Jansens 2010-10-20 15:22:41 EDT
do we have submenu icons yet?
Comment 13 Mikachu 2014-10-27 16:07:06 EDT
yes