-
Friedrich W. H. Kossebau authored
Summary: Fixes a bug of context menu entries being duplicated each time the context menu is shown with another document in same cases. This happened, as each document instance tracks what actions it adds to a context menu, but only removes the actions once the menu is shown again. Which fails if the menu is shown with another document than before, but the same context menu object is reused again, thus still having the actions from the other document. Small recap, as the situation is a bit complex: A context menu in KDevelop text documents is triggered by the method KateViewInternal::contextMenuEvent(), where KateViewInternal is an internal QWidget class of KTextEditor::ViewPrivate, which is the actual KTextEditor::View implementation. That KateViewInternal method asks its container KTextEditor::ViewPrivate for the contextMenu() and calls popup() on it. KTextEditor::ViewPrivate::contextMenu() itself either returns a user-set context menu, or queries the toplevel KXMLGUIClient of the XMLGUI structure it belongs to for a "menu" container by the name "ktexteditor_popup". This container is defined by katepart5ui.rc <Menu name="ktexteditor_popup" noMerge="0">, and KTE plugins can extend this with a respecive <Menu name="ktexteditor_popup" noMerge="1"> in their ui.rc files. For some yet to explore internal reason, if there is a plugin with a "ktexteditor_popup" menu to merge in, then the QMenu object instance is the same across all KTextEditor::View instances, otherwise each view has its own. More, even per view one gets a new QMenu object every time the view becomes the active one again (and thus merging into the app XMLGUI structure is done). Due to this the duplicated entries in the context menu are currently only happening if a plugin also provides a "ktexteditor_popup" menu to merge in, like the CTags one does. In a perfect world we would have a symmetric signal KTextEditor::View::contextMenuAboutToHide for the KTextEditor::View::contextMenuAboutToShow signal, so we could unplug our per-view instance custom menu additions. As fallback (and assuming the QMenu is always show as popup) there is the QMenu::aboutToHide signal for use instead. This was once was used here with by commit b837392d. Where though the connection was made to the contextmenu object at the time of the creation of the view. Which, at least these days, as pointed out above, runs risk to miss the change of the context menu object. At least it was found at the time already that approach was not working out, commit 4d63d74c use a new approach instead to clear the menu from the old actions only right before the/a menu was shown again. Sadly the commit message does not explain where exactly things failed, and just described the symptom: "don't trust Qt's aboutToHide signal as there seemed to be many cases where it didn't fire as I thought it would be.". And points to https://blog.qt.io/blog/2010/02/23/unpredictable-exec/ where though any relation is not immediately obvious, as there should be little chance that the context menu is triggered to be shown again from any actions in the menu. And if the menu object was instead deleted without emitting any aboutToHide signal, some shutdown would be going on which should be caught by other means. This patch reverts to relying to the aboutToHide signal again, but making sure this is done for the respective menu instance currently shown. As well as also catches any intermediate shutdown of the menu instance as well as the document itself. Test Plan: Used with & without CTags plugin, opening and using context menus on different views incl. multiple views on same document with splitted view areas. Reviewers: #kdevelop Subscribers: anthonyfieroni, rjvbb, kdevelop-devel Tags: #kdevelop Differential Revision: https://phabricator.kde.org/D22424
d940a88e