<table><tr><td style="">kossebau created this revision.<br />kossebau added a reviewer: KDevelop.<br />Herald added a project: KDevelop.<br />Herald added a subscriber: kdevelop-devel.<br />kossebau requested review of this revision.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D22424">View Revision</a></tr></table><br /><div><strong>REVISION SUMMARY</strong><div><p>Context menu in KDevelop text documents is shown by the<br />
KateViewInternal::contextMenuEvent, where KateViewInternal is an internal<br />
widget class of the actual KTextEditor::View implmenetation<br />
KTextEditor::ViewPrivate. The KateViewInternal code asks its container<br />
KTextEditor::ViewPrivate for the contextMenu() and calls popup() on it.</p>

<p>KTextEditor::ViewPrivate::contextMenu() itself either returns a user-set<br />
context menu, or queries the toplevel KXMLGUIClient of the XMLGUI structure<br />
it belongs to for a "menu" container by the name "ktexteditor_popup".<br />
This container is defined by katepart5ui.rc<br />
<Menu name="ktexteditor_popup" noMerge="0">, and KTE plugins can extend this<br />
with a respecive <Menu name="ktexteditor_popup" noMerge="1"> in their ui.rc<br />
files.</p>

<p>For some yet to explore internal reason, if there is a plugin with a<br />
"ktexteditor_popup" menu to merge in, then the QMenu object instance is the<br />
same across all KTextEditor::View instances, otherwise each view has its own.<br />
More, even per view one gets a new QMenu object everytime the view becomes the<br />
active one again (and thus merging into the app xmlgui structure is done).<br />
Due to this the duplicated entries in the context menu is currently only<br />
happening if a plugin also provides a "ktexteditor_popup" menu to merge in,<br />
like the CTags one does.</p>

<p>In a perfect world we would have a symmetric signal<br />
KTextEditor::View::contextMenuAboutToHide for the<br />
KTextEditor::View::contextMenuAboutToShow signal, so we could unplug our<br />
per-view instance custom menu additions. As fallback (and assuming the QMenu<br />
is always show as popup) there is the QMenu::aboutToHide signal for use<br />
instead. This was once was used here with by commit<br />
b837392d5f05394794a813afb7ca94e54650fcff. Where though the connection was<br />
made to the contextmenu object at the time of the creation of the view.<br />
Which, at least these days, as pointed out above, runs risk to miss the<br />
change of the context menu object.<br />
At least it was found at the time already that approach was not working out,<br />
commit  4d63d74c7d189479b752a11df70afab77859a457 use a new approach instead<br />
to clear the menu from the old actions only right before the/a menu was<br />
shown again. Sadly the commit message does not explain where exaxtly<br />
things failed, and just described the symptom: "don't trust Qt's aboutToHide<br />
signal as there seemed to be many cases where it didn't fire as I thought<br />
it would be.". And points to<br />
<a href="https://blog.qt.io/blog/2010/02/23/unpredictable-exec/" class="remarkup-link" target="_blank" rel="noreferrer">https://blog.qt.io/blog/2010/02/23/unpredictable-exec/</a> where though any<br />
relation is not immediatly obvious, as there should be little chance that<br />
the context menu is triggered to be shown again from any actions in the<br />
menu. And if the menu object was instead deleted without emitting any<br />
aboutToHide signal, some shutdown would be going on which should be caught<br />
by other means.</p>

<p>This patch reverts to relying to the aboutToHide signal again, but making<br />
sure this is done for the respective menu instance currently shown. As well<br />
as also catches any intermediate shutdown of the menu instance as well as<br />
the document itself.</p></div></div><br /><div><strong>TEST PLAN</strong><div><p>Used with & without CTags plugin, opening and using context menus on<br />
different views incl. multiple views on same document with splitted view<br />
areas.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R32 KDevelop</div></div></div><br /><div><strong>BRANCH</strong><div><div>removecontextmenuactionsonhide</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D22424">https://phabricator.kde.org/D22424</a></div></div><br /><div><strong>AFFECTED FILES</strong><div><div>kdevplatform/shell/textdocument.cpp<br />
kdevplatform/shell/textdocument.h</div></div></div><br /><div><strong>To: </strong>kossebau, KDevelop<br /><strong>Cc: </strong>kdevelop-devel, hmitonneau, christiant, glebaccon, domson, antismap, iodelay, alexeymin, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd<br /></div>