https://bugs.kde.org/show_bug.cgi?id=503813

--- Comment #42 from Igor Kushnir <igor...@gmail.com> ---
(In reply to ulterno from comment #39)
> Attempting the same inside QTextEdit::contextMenuEvent, did not reproduce
> the bug. i.e. The menu was not called twice when using the keyboard Menu key.
> So, only able to do it with KTextEditor::View
This is strange. Maybe the bug is that KTextEditor somehow sends the same event
again? Locating such a bug in KTextEditor should be much easier than in Qt:
much less code to consider and much higher-level code, not dealing with
OS-level APIs

> Modifying the function as shown below: ...
>               if (menu == d->currentContextMenu)
>               {
>                       // Of the same thing, then don't delete and don't 
> retry. This line
> depends upon cleanContextMenu working as required
>                       qCDebug (SHELL) << "Pointer of the new context menu is 
> the same... Not
> cleaning, not remaking";
>                       return;
>               }
> ... can be considered a hotfix.
This hotfix seems risky in view of the following info from the commit message
of the aforementioned commit
https://commits.kde.org/kdevelop/d940a88e8bd92031cd7ffce2450cc24f266e6acc :
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).

I don't see how KDevelop can tell whether the old context menu request is
obsolete and the menu should be repopulated, or if the second context menu
event is just an exact duplicate of the currently handled one, and therefore
should be ignored. Comparing the context menu event objects that arrive to
KateViewInternal::contextMenuEvent() should be less risky (especially if the
event pointers match), as I suggested in the previous comment. Maybe the root
cause is even in the short definition of KateViewInternal::contextMenuEvent(),
e.g. the special handling for the Keyboard event reason therein: `if
(e->reason() == QContextMenuEvent::Keyboard)`.

(In reply to ulterno from comment #41)
> > I am willing to take up the task after finishing 3 others I have lined up
> > (and getting good enough in the process) as long as someone can take up
> > testing
There is no dedicated KDevelop QA team. We rely on developer testing and
automatic tests. Whoever makes a change is expected to do the bulk of the
testing and is most responsible for regressions.
> It seems preferable to have this kind of thing work asynchronously, 
> specially if we end up implementing Remote Objects with a Server - Client
> architecture to use KDevelop through ssh.
> Even if the above doesn't happen, it would be good to not have the GUI start
> hanging if someone is maybe using a slower HDD with some plugin having a
> heavy IO load or even if they are opening a project on a USB Flash drive
I agree that asynchronous event handling is much better. The problem is getting
there. I suspect a lot of KDevelop code would have to be changed. And some fair
amount of testing with different plugins, e.g. CTags, would be needed on X11
and Wayland. And the immediate gain seems to be small because this bug is
somewhat obscure and probably can be fixed or worked around much easier in
KTextEditor.

This trouble with nested event loops is similar to the much more impactful
(measured by the age and the length of the CC list) Bug 435235.

-- 
You are receiving this mail because:
You are watching all bug changes.

Reply via email to