...because that is more flexible and how it is in openbox.
I have had in mind that we should do this since the original
implementation, and #2994 just jogged my memory.
Sequence of events:
- menu_finish() frees the sub-menu first
- the selection.menu of the parent menu is now dangling
- menu_finish() frees the parent menu
- menu_free() calls menu_close_root() on the parent menu
- menu_close_root() tries to close the (freed) sub-menu
- boom
Extending nullify_item_pointing_to_this_menu() avoids the crash.
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.
...with the same approach as rcxml.c
- `If` actions now works for menus
- `name` argument no longer have to be the first argument of <action>
- `label` argument no longer have to be the first argument of <item>
This is a common practice in C projects, which simply enforces that
each header must compile cleanly without implicit dependencies on
other headers (see also the previous commit).
The lifetime of the "current_" variables (current_menu, current_item,
current_item_action) is very difficult to understand from reading the
code. It appears that e.g. current_menu could still point to a previous
menu when starting to parse a new one, with unpredictable results.
Let's use a context struct when parsing, and consistently initialize
it when beginning to build a new menu.
Lightly tested with:
- default menus (no menu.xml)
- example static menu from labwc.github.io/getting-started.html
- an added "client-list-combined-menu" sub-menu
- pipe menu generated by `labwc-menu-generator -p`
v2: style fix
This fixes the gap between menu items and the menu border in an output
with a fractional scale due to the semantic gap between cairo and
wlroots's position-independent scene renderer.
This patch also changes the semantics of scaled_icon_buffer: rather than
calling scaled_icon_buffer_set_app_id() every time an app_id is set, we
can now call scaled_icon_buffer_set_view() just once so that multiple
scaled_icon_buffers bound to a window are automatically updated when an
app_id is set or new icon is set via xdg-toplevel-icon-v1.
This removes the need to call update_client_list_combined_menu()
and update_client_send_to_menu() every time a root menu is opened.
This commit also fixed the incorrect menu position with following
configuration:
<menu id="foo" label="foo">
<item label="aaaaaa"/>
<item label="bbbbbb"/>
</menu>
<menu id="root-menu">
<menu id="foo" />
<menu id="foo" />
</menu>
Previous commits fixed some unexpected behaviors when ShowMenu action is
executed from menu items, but that was still prone to bugs because when
calling actions_run(), we allow an inconsistent state where all menus are
closed but pipemenus must not be destroyed.
So this commit simply removes ShowMenu actions from menu items on
initialization.
server->menu_current should be cleared before calling actions_run() as
it may internally call menu_open_root(). Clearing it after actions_run()
leads to an inconsistent state where a menu is opened but
server->menu_current is NULL. It even lead to a segfault when the item
opening another menu is contained in a pipemenu, because
menu_open_root() calls destroy_pipemenu() when server->menu_current is
set, which makes accessing item->actions a UAF.
The previous revert fixed the problem of stuck modifier keys with
keybinds in Blender, but made Firefox show its menu bar with Alt-*
keybinds. This is fundamentally inevitable due to the limitation of
wayland protocol, but at least for the default Alt-Tab keybind for
window switcher, we can mitigate this problem by clearing the keyboard
focus when the window switcher is activated. This is what KWin does, and
we decided to follow that.
So in this commit, keyboard and pointer focus are temporarily cleared
while Move/Resize, window switcher and menu interactions and restored
after them. We slightly deviate from KWin as KWin doesn't clear the
keyboard focus while Move/Resize, but it solves our existing problem
that Firefox shows its menu bar after dragging it with default Alt-Drag
mousebind, and this is what Mutter does.
We considered other solutions, but they don't work well:
1. Send wl_keyboard.{leave,enter} every time keybinds/mousebinds are
triggered. This solves the Firefox's menu bar problem, but that
sounds like a workaround and sending unnecessary events every time is
not desirable.
2. Send release events for both modifiers and keys even when they are
bound to keybinds. This is what Mutter is doing, but it looks like an
implementation issue and violates wayland protocol.
This commit delegates the calculation for menu position into wlroots
utilities for xdg_positioner.
Notable functional changes are:
- Slide the menu to fit in the output when it's opened out of the output
(e.g. top-left window menu is opened when the window is overflowing to
the left), rather than not updating the menu at all.
- The horizontal alignment of menus is now determined based on the size of
each (sub)menu alone rather than the total width of entire menu tree.
This means submenus can now overlap with is parents, but this is no
longer a problem since we recently added support for menu borders.
- Fixed that pipemenus always follow the alignment of its parent even when
it overflows from the output.
Arrow signs are specific to submenu items, so they would be more natural
to be handled in menu.c rather than accepting "arrow" in
font_buffer_create().
Also I allowed non-positive numbers for max_width in font_buffer_create(),
in which case the natural font width is used as the buffer width.
- Don't store font/background buffers in menuitem struct since we no
longer dynamically update buffers of existing menuitems.
- Factor out the duplicated codes for creating menu item scenes for each
unselected/selected states.
I missed that `scaled_font_buffer_update()` has `max_width` argument and
the subsequent calls to `scaled_font_buffer_set_max_width()` caused
unnecessary re-renderings.
This fixes use-after-free when there's only 1 desktop and
menu_hide_submenu() is called to delete "Workspaces" submenu in
client-menu before menu scenes are initialized.
As menu_create() and item_create() no longer initialize scenes after
76515316, menu->scene_tree and item->tree should be null-checked.
The deleted lines were dead code that didn't make any sense (even if it
were not dead, it should have recreated the parent of the hidden menu
rather than the hidden menu itself).
Commit 7651531 introduced a regression: `menu_update_scene()` which
re-creates a menu scene was called for all the menus when a pipemenu is
created, so the menus (parent of the pipemenu) were always moved to (0,0)
and hidden, and the pipemenu was incorrectly positioned.
This commit fixes it by calling `menu_update_scene()` only for the
pipemenu when it's created.
