D22424: TextDocument: remove actions from contextmenu on hide already

[Friedrich W. H. Kossebau]

kossebau created this revision.
kossebau added a reviewer: KDevelop.
Herald added a project: KDevelop.
Herald added a subscriber: kdevelop-devel.
kossebau requested review of this revision.

REVISION SUMMARY
  Context menu in KDevelop text documents is shown by the
  KateViewInternal::contextMenuEvent, where KateViewInternal is an internal
  widget class of the actual KTextEditor::View implmenetation
  KTextEditor::ViewPrivate. The KateViewInternal code 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 everytime 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 is 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
  b837392d5f05394794a813afb7ca94e54650fcff. 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  4d63d74c7d189479b752a11df70afab77859a457 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 exaxtly
  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 immediatly 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.

REPOSITORY
  R32 KDevelop

BRANCH
  removecontextmenuactionsonhide

REVISION DETAIL
  https://phabricator.kde.org/D22424

AFFECTED FILES
  kdevplatform/shell/textdocument.cpp
  kdevplatform/shell/textdocument.h

To: kossebau, #kdevelop
Cc: kdevelop-devel, hmitonneau, christiant, glebaccon, domson, antismap, iodelay, alexeymin, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20190712/36b7d511/attachment.html>