Bug 401069 - KTextEditor: aboutToShowContextMenu is emitted for all open documents
Summary: KTextEditor: aboutToShowContextMenu is emitted for all open documents
Status: RESOLVED FIXED
Alias: None
Product: frameworks-ktexteditor
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: Compiled Sources All
: NOR normal
Target Milestone: ---
Assignee: KWrite Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-11-15 14:31 UTC by RJVB
Modified: 2018-11-17 13:40 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
potential fix (1.50 KB, patch)
2018-11-15 15:47 UTC, RJVB
Details

Note You need to log in before you can comment on or make changes to this bug.
Description RJVB 2018-11-15 14:31:09 UTC
SUMMARY
A window (and in general, an application) can only show a single context menu at a time. Yet aboutToShowContextMenu is emitted for all currently open documents, not just the one that will show the menu.

This leads to duplicate menu items in KDevelop, but curiously only when the CTags plugin is loaded.

STEPS TO REPRODUCE
1. build KTextEditor with the following debug traces:
void KTextEditor::ViewPrivate::aboutToShowContextMenu()
{
    QMenu *menu = qobject_cast<QMenu *>(sender());

    if (menu) {
        if (mainWindow()->activeView() == this) {
            qWarning() << Q_FUNC_INFO << "emitting contextMenuAboutToShow for foreground view" << this;
            emit contextMenuAboutToShow(this, menu);
        } else {
            qWarning() << Q_FUNC_INFO << "NOT emitting contextMenuAboutToShow for background view" << this;
        }
    }
}
2. install Kate with the patch from D16779
2. run kate or kdevelop with multiple documents open 

OBSERVED RESULT
In kate I'm seeing "NOT emitting" output for all open documents that aren't active:
void KTextEditor::ViewPrivate::aboutToShowContextMenu() NOT emitting contextMenuAboutToShow for background view KTextEditor::ViewPrivate(0x7fa76e13ae30)
void KTextEditor::ViewPrivate::aboutToShowContextMenu() NOT emitting contextMenuAboutToShow for background view KTextEditor::ViewPrivate(0x7fa76e69a280)
void KTextEditor::ViewPrivate::aboutToShowContextMenu() emitting contextMenuAboutToShow for foreground view KTextEditor::ViewPrivate(0x7fa76e10c0e0)

In KDevelop I'm seeing the same kind of output:
void KTextEditor::ViewPrivate::aboutToShowContextMenu() NOT emitting contextMenuAboutToShow for background view KTextEditor::ViewPrivate(0x7fd5bb2f87e0)
void KTextEditor::ViewPrivate::aboutToShowContextMenu() NOT emitting contextMenuAboutToShow for background view KTextEditor::ViewPrivate(0x7fd5bbb4c470)
void KTextEditor::ViewPrivate::aboutToShowContextMenu() emitting contextMenuAboutToShow for foreground view KTextEditor::ViewPrivate(0x7fd5bb7eb830)

but only for the documents that have been active (and/or have had the context menu open).

See also https://phabricator.kde.org/D16882
Comment 1 RJVB 2018-11-15 14:34:04 UTC
Addendum:

Is this intended behaviour or a bug that was never detected because it never caused visible side-effects?
Comment 2 RJVB 2018-11-15 15:47:45 UTC
Created attachment 116327 [details]
potential fix

2-hunk patch:

- Hunk 2 is a protection against emitting the signal multiple times
- Hunk 1 appears to be a fix based on the following hypothesis:

If the menu obtained from KXMLGUI is always the same (QMenu instance) then its aboutToShow and aboutToHide signals should be disconnected from all receivers before connecting them to  "this". It makes no sense I can see to disconnect from "this" and then reconnect to exactly the same slots.
Comment 3 RJVB 2018-11-17 13:40:53 UTC
Git commit 3f0c617e22ab5d2ae016ef7858c6f451f9cf0ad0 by René J.V. Bertin.
Committed on 17/11/2018 at 13:40.
Pushed by rjvbb into branch 'master'.

disconnect contextmenu from all aboutToXXContextMenu receivers

Context menus are shown in a single view at a time so only the active
KTextEditor::ViewPrivate instance should be signalled when a context
menu is about to show or hide.
Differential Revision: https://phabricator.kde.org/D16927

M  +7    -2    src/view/kateview.cpp

https://commits.kde.org/ktexteditor/3f0c617e22ab5d2ae016ef7858c6f451f9cf0ad0