Bug 386911 - empty entries in toolbar button menus
Summary: empty entries in toolbar button menus
Status: RESOLVED FIXED
Alias: None
Product: kdevplatform
Classification: Developer tools
Component: shell (show other bugs)
Version: unspecified
Platform: Compiled Sources All
: NOR normal
Target Milestone: ---
Assignee: RJVB
URL: https://phabricator.kde.org/D8954
Keywords: junior-jobs
Depends on:
Blocks:
 
Reported: 2017-11-14 16:50 UTC by RJVB
Modified: 2017-11-22 21:01 UTC (History)
0 users

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


Attachments
empty item in the Problems toolbar menu (18.46 KB, image/png)
2017-11-14 16:50 UTC, RJVB
Details
empty items in the Outline toolbar button (15.79 KB, image/png)
2017-11-14 16:51 UTC, RJVB
Details
empty item in the Search/Replace toolbar button (38.72 KB, image/png)
2017-11-14 16:52 UTC, RJVB
Details
empty outline menu items in the 5.2.0 AppImage (53.50 KB, image/png)
2017-11-22 13:02 UTC, RJVB
Details

Note You need to log in before you can comment on or make changes to this bug.
Description RJVB 2017-11-14 16:50:36 UTC
Created attachment 108859 [details]
empty item in the Problems toolbar menu

I've noticed that certain of the toolbar buttons have empty items in their menus. They don't appear to do anything - where could they come from?
Comment 1 RJVB 2017-11-14 16:51:22 UTC
Created attachment 108860 [details]
empty items in the Outline toolbar button
Comment 2 RJVB 2017-11-14 16:52:18 UTC
Created attachment 108861 [details]
empty item in the Search/Replace toolbar button
Comment 3 RJVB 2017-11-22 13:02:06 UTC
Created attachment 109011 [details]
empty outline menu items in the 5.2.0 AppImage

I was beginning to think this might have something to do with local patches or Qt 5.8.0, but I am seeing exactly the same issue in the latest AppImage (5.2.0, downloaded earlier today).

Font nor widget style have anything to do with it.
Comment 4 Kevin Funk 2017-11-22 14:53:44 UTC
The entries in the menu are equal to the entries in the task bar of the tool views. IOW, the empty action you have in the menu corresponds to the line edit. 

There's simply no visual representation of the line edit in a menu, thus you get that empty space. I have a deja vu -- I remember fixing this long time ago; but I can't find the corresponding commit right now.

Anyhow, the place to look at is likely `IdealDockWidget::contextMenuRequested`.
Comment 5 RJVB 2017-11-22 15:44:15 UTC
Indeed, I just confirmed that with the Outline widget. I had just seen the same thing happening in a demo app using wxQt and fortunately decided to take a look to see if I could understand what was happening :)

For the Outline toolview I found an acceptable fix for the *two* empty lines in the plugin's context menu:
- use the tooltip text as the menu item text for the sort action
- set the filter item text to "Clear filter" and add a lambda function that achieves this. I don't know how useful that is but at least it's sensical.

I planned to go through the other cases to see if they can be handled similarly but I think you're suggesting that the context menu simply skips actions with an empty text?
Comment 6 Kevin Funk 2017-11-22 15:56:00 UTC
> I planned to go through the other cases to see if they can be handled similarly but I think you're suggesting that the context menu simply skips actions with an empty text?

Well, actions which cannot be represented in a menu. As far as I remember there's another way to check this instead of check against a non-empty text. Please investigate.
Comment 7 RJVB 2017-11-22 16:20:02 UTC
There's QAction::isVisible() but that applies to menus and toolbars so isn't useful here. QAction::menuRole() could have been used too but that's a Mac-specific property which doesn't have a "keep me out of menus" setting anyway.

Either way, is there a point in adding textless actions to context menus. It seems to me one should avoid them (to spare the user some potential surprises by flying blind), regardless of whether there are other ways to determine if a QAction can be represented.
Comment 8 Kevin Funk 2017-11-22 21:01:44 UTC
Git commit f4363a0863414e46fd6d91c5690ce6aec39c587f by Kevin Funk, on behalf of René J.V. Bertin.
Committed on 22/11/2017 at 21:01.
Pushed by kfunk into branch '5.2'.

kdevelop: prevent empty dockwidget context menuitems

Summary:
This patch prevents dock widgets (toolviews) from having empty items in their context menus. Most the corresponding actions can probably not be represented in menus anyway and are thus unlikely to do anything when triggered through a menu. The average user doesn't know that though.

The patch achieves this by filtering out QActions that would not have a title in `IdealDockWidget::contextMenuRequested()` but also adds a menu item text to two actions which I think make sense to include in the context menu. (Doing this makes it possible to hide the toolview's toolbar in more situations, gaining a bit of vertical space.)
FIXED-IN: 5.2.1

Test Plan:
Works as intended on Mac and Linux/X11 .

I understand there may be another way to detect QActions that cannot be represented in menus but haven't yet found such an alternative. Such an alternative could be used in addition to the empty text check which I think should be done as argued in the summary.

Individual plugins can also override `IToolViewFactor::contextMenuActions()`.

Reviewers: kfunk

Reviewed By: kfunk

Subscribers: kdevelop-devel

Tags: #kdevelop

Differential Revision: https://phabricator.kde.org/D8954

M  +8    -1    kdevplatform/sublime/idealdockwidget.cpp
M  +3    -0    plugins/contextbrowser/contextbrowserview.cpp
M  +2    -0    plugins/outlineview/outlinewidget.cpp

https://commits.kde.org/kdevelop/f4363a0863414e46fd6d91c5690ce6aec39c587f