Skip to content

Conversation

@tokyo4j
Copy link
Contributor

@tokyo4j tokyo4j commented Aug 14, 2025

This PR allows overwriting the icon of item linking to another menu like below (the icon for krita instead of mpv should be shown):

<openbox_menu>
  <menu id="static-menu" label="Static Menu" icon="mpv" />
  <menu id="root-menu" label="Root">
    <menu id="static-menu" icon="krita" />
  </menu>
</openbox_menu>

For comparison, Openbox seems to ignore icon property in toplevel menu definition in the first place and always follows icon property inside the <menu> that links to a static menu. So when we have following menu.xml:

<openbox_menu>
  <menu id="static-menu" label="Static Menu"
    icon="/usr/share/icons/hicolor/32x32/apps/mpv.png" />
  <menu id="root-menu" label="Root">
    <menu id="static-menu"
      icon="/usr/share/icons/hicolor/22x22/apps/krita.png" />
    <menu id="inline-menu" label="Inline Menu"
      icon="/usr/share/icons/hicolor/32x32/apps/chromium.png" />
  </menu>
</openbox_menu>
Openbox labwc 0.9.0 This PR
20250815_02h59m17s_grim 20250815_03h06m42s_grim 20250815_03h11m59s_grim

So I think this PR enhances compatibility to Openbox.

This PR also fixes my mistake in #2971 (s/parent->icon/menu->icon/) that showed incorrect icon in an item linking to another menu.

Link: #2992 (reply in thread)

Allow overwriting the icon of item linking to another menu like below
(the icon for "krita" should be shown):

<openbox_menu>
  <menu id="static-menu" label="Static Menu" icon="mpv" />
  <menu id="root-menu" label="Root">
    <menu id="static-menu" icon="krita" />
  </menu>
</openbox_menu>

This commit also fixes my mistake in 17d66e5 (s/parent->icon/menu->icon/)
that showed incorrect icon in an item linking to another menu.
@johanmalm
Copy link
Member

Thanks for analysis and fix. Makes sense.

LGTM

@johanmalm johanmalm merged commit 72e1945 into labwc:master Aug 14, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants