D16882: [KDevelop/Shell] prevent duplicate added contextmenu actions

[Friedrich W. H. Kossebau]

kossebau added a comment.


  In D16882#360857 <https://phabricator.kde.org/D16882#360857>, @rjvbb wrote:
  
  > Re-opening because I found an actual flaw in KDevelop after noticing that context menu duplication still occurred when only the active view receives the aboutToShowContextMenu signal.
  >
  > The `addedContextMenu` member exists because `"we want to remove the added stuff when the menu hides"`. This should of course read "when the menu reopens in again, possibly in a different view".
  
  
  Good find, that seems indeed broken.
  
  > The flaw here is that the design forgets that the context menu instance is shared among views. It expects `d->addedContextMenu` to exist and contain the QMenu added by a previous view, but this cannot be the case in the current implementation where the variable is only allocated when the menu is first opened in a given view.
  
  I would see the flaw also in that there is no specification in the KTextEditor API how the context menu is shared/reused. `KTextEditor::View::contextMenu()` talks about "the xmlgui menu" or custom set "context menu object", but no hint/promise whether there is any instance sharing done, e.g. between views for the same document or even across all views in the same process(?).
  
  > If the `addedContextMenu` is to be removed in JIT-fashion before reopening the context menu, it should be a static variable.
  
  That might be a way, yes.
  
  Before though I would like to have it first sorted out with the KTextEditor people what to expect here and whether the API dox could resolve that undefinedness. Given the current implementation kdevelop-side I would not be surprised if KTextEditor changed implementation here, but needs to be explored. The proposed fix relies on the current implementation, which is a bit fragile.
  
  Another option might be to link up to the menu being closed and clean up then, as the comment on the `addedContextMenu` member claimed. (personally preferred to clean up right after use). 
  Also needs to be explored if there once was such an implementation and why it has been removed. Might do in the next days.

REPOSITORY
  R32 KDevelop

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

To: rjvbb, #kdevelop, kossebau
Cc: kossebau, kde-frameworks-devel, kdevelop-devel, glebaccon, antismap, iodelay, vbspam, geetamc, Pilzschaf, akshaydeo, surgenight, arrowd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20181118/c240f146/attachment-0001.html>