include/vcl/toolkit/menubtn.hxx | 4 +++- vcl/inc/salvtables.hxx | 2 +- vcl/source/control/managedmenubutton.cxx | 2 +- vcl/source/control/menubtn.cxx | 8 +++++++- vcl/source/window/builder.cxx | 2 +- 5 files changed, 13 insertions(+), 5 deletions(-)
New commits: commit 9351d4d6a131b7e4c05a5762ef74789d37054aa3 Author: Michael Weghorn <m.wegh...@posteo.de> AuthorDate: Tue Dec 10 08:31:24 2024 +0100 Commit: Michael Weghorn <m.wegh...@posteo.de> CommitDate: Tue Dec 10 10:50:30 2024 +0100 tdf#164072 vcl: Let MenuButton dispose its PopupMenu The MenuButton usually owns the PopupMenu that gets set via MenuButton::SetPopupMenu, but so far didn't dispose it, i.e. the PopupMenu was never disposed. As far as I understand, that is a preexisting issue, but since commit 6708246e20ce522e673f539369cd38687d2dd16d Author: Michael Weghorn <m.wegh...@posteo.de> Date: Thu Dec 5 11:06:48 2024 +0000 tdf#164093 tdf#157001 a11y: Improve menu window disposal , not disposing the menu also means not disposing the menu's MenuFloatingWindow (which gets created when the popup menu shows), resulting in warn:legacy.osl:563630:563630:vcl/source/window/window.cxx:307: Window ( 10MenuButton()) with live SystemWindows destroyed: 18MenuFloatingWindow() Window ( 10MenuButton()) with live SystemWindows destroyed: 18MenuFloatingWindow() and an assert getting triggered as mentioned in tdf#164072 comment 16. Fix this by letting the MenuButton take care of disposing the menu if it is the owner. The only case where the MenuButton is assigned a menu without taking ownership is in SalInstanceComboBox::set_item_menu (which currently only gets called from SvxStyleBox_Base::SetupEntry). In that case, the weld::Menu passed to SalInstanceComboBox::set_item_menu has the ownership: SalInstanceBuilder::weld_menu calls the SalInstanceMenu ctor with a bTakeOwnership=true, and the SalInstanceMenu dtor disposes the menu. Add a `bTakeOwnership` bool param to MenuButton::SetPopupMenu to distinguish between the cases where the PopupMenu is responsible for disposing the menu and where it's not. Change-Id: I32766d5084e4826056ef394a587b8c2e3124c4da Reviewed-on: https://gerrit.libreoffice.org/c/core/+/178197 Tested-by: Jenkins Reviewed-by: Michael Weghorn <m.wegh...@posteo.de> (cherry picked from commit 90d9b6e12f0aa9a569958586832bd4abe9561197) Reviewed-on: https://gerrit.libreoffice.org/c/core/+/178200 diff --git a/include/vcl/toolkit/menubtn.hxx b/include/vcl/toolkit/menubtn.hxx index 49056a0084bb..b185fbf1e737 100644 --- a/include/vcl/toolkit/menubtn.hxx +++ b/include/vcl/toolkit/menubtn.hxx @@ -41,6 +41,8 @@ private: VclPtr<Window> mpFloatingWindow; OUString msCurItemIdent; sal_uInt16 mnCurItemId; + // whether the MenuButton is the owner of mpMenu + bool mbOwnPopupMenu = false; bool mbDelayMenu; bool mbStartingMenu; Link<MenuButton*,void> maActivateHdl; @@ -80,7 +82,7 @@ public: //before being shown void SetDelayMenu(bool bDelay) { mbDelayMenu = bDelay; } - void SetPopupMenu(PopupMenu* pNewMenu); + void SetPopupMenu(PopupMenu* pNewMenu, bool bTakeOwnership); PopupMenu* GetPopupMenu() const { return mpMenu; } void SetPopover(Window* pWindow); diff --git a/vcl/inc/salvtables.hxx b/vcl/inc/salvtables.hxx index 534e158b4196..a511f021940d 100644 --- a/vcl/inc/salvtables.hxx +++ b/vcl/inc/salvtables.hxx @@ -911,7 +911,7 @@ public: m_xMenuButton = VclPtr<MenuButton>::Create(m_xComboBox, WB_FLATBUTTON | WB_NOPOINTERFOCUS); - m_xMenuButton->SetPopupMenu(pPopup); + m_xMenuButton->SetPopupMenu(pPopup, false); m_xMenuButton->Show(pPopup != nullptr); m_sMenuButtonRow = rIdent; } diff --git a/vcl/source/control/managedmenubutton.cxx b/vcl/source/control/managedmenubutton.cxx index 62be691c1e70..64320690d9ec 100644 --- a/vcl/source/control/managedmenubutton.cxx +++ b/vcl/source/control/managedmenubutton.cxx @@ -42,7 +42,7 @@ void ManagedMenuButton::dispose() void ManagedMenuButton::PrepareExecute() { if (!GetPopupMenu()) - SetPopupMenu(VclPtr<PopupMenu>::Create()); + SetPopupMenu(VclPtr<PopupMenu>::Create(), true); MenuButton::PrepareExecute(); diff --git a/vcl/source/control/menubtn.cxx b/vcl/source/control/menubtn.cxx index 28417879888d..1cfd0799fe30 100644 --- a/vcl/source/control/menubtn.cxx +++ b/vcl/source/control/menubtn.cxx @@ -166,6 +166,8 @@ void MenuButton::dispose() { mpMenuTimer.reset(); mpFloatingWindow.clear(); + if (mpMenu && mbOwnPopupMenu) + mpMenu->dispose(); mpMenu.clear(); PushButton::dispose(); } @@ -240,12 +242,16 @@ void MenuButton::Select() maSelectHdl.Call( this ); } -void MenuButton::SetPopupMenu(PopupMenu* pNewMenu) +void MenuButton::SetPopupMenu(PopupMenu* pNewMenu, bool bTakeOwnership) { if (pNewMenu == mpMenu) return; + if (mpMenu && mbOwnPopupMenu) + mpMenu->dispose(); + mpMenu = pNewMenu; + mbOwnPopupMenu = bTakeOwnership; } void MenuButton::SetPopover(Window* pWindow) diff --git a/vcl/source/window/builder.cxx b/vcl/source/window/builder.cxx index e181acf7ed21..3322e34d6626 100644 --- a/vcl/source/window/builder.cxx +++ b/vcl/source/window/builder.cxx @@ -760,7 +760,7 @@ VclBuilder::VclBuilder(vcl::Window* pParent, std::u16string_view sUIDir, const O "vcl", "missing elements of button/menu"); if (!pTarget || !pMenu) continue; - pTarget->SetPopupMenu(pMenu); + pTarget->SetPopupMenu(pMenu, true); } //Remove ScrollWindow parent widgets whose children in vcl implement scrolling