D14199: Simplify the context menu handling functions in FoldersPanel

[David Hallas]

hallas added a comment.


  In D14199#294501 <https://phabricator.kde.org/D14199#294501>, @elvisangelaccio wrote:
  
  > The old code was like that for a reason: https://blogs.kde.org/2009/03/26/how-crash-almost-every-qtkde-application-and-how-fix-it-0
  >
  > Now, the chance this change will lead to a crash is very low, but still I don't see what we gain either.
  
  
  After reading the blog post and looking at the Qt documentation, I think you are absolutely right about this. The TreeViewContextMenu::open function ends up calling QMenu::exec which runs synchronously meaning that the application can close while processing events, what I do not understand about the original code is what good does the
  
    if (contextMenu.data())
  
  do? The whole point of QPointer is to provide memory management of QObjects, also in the case where the managed object is deleted elsewhere, therefore I can't see that it is needed.
  
  So, I think a safe implementation would be something like this:
  
    QPointer<TreeViewContextMenu> contextMenu = new TreeViewContextMenu(this, fileItem);
    contextMenu.data()->open();

REPOSITORY
  R318 Dolphin

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

To: hallas, broulik, elvisangelaccio
Cc: elvisangelaccio, ngraham, broulik, kfm-devel, spoorun, navarromorales, firef, andrebarros, emmanuelp
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20180719/1e292c9e/attachment.htm>