Bug report #5754
Preferences item under "QGIS" menu opens shortcut settings on OSX
Status: | Closed | ||
---|---|---|---|
Priority: | High | ||
Assignee: | - | ||
Category: | GUI | ||
Affected QGIS version: | master | Regression?: | No |
Operating System: | OS X | Easy fix?: | No |
Pull Request or Patch supplied: | No | Resolution: | |
Crashes QGIS or corrupts data: | No | Copied to github as #: | 15260 |
Description
On OSX, a "Preferences" item is available in the "QGIS" menu drop-down. Unfortunately, it opens the shortcut settings rather than the QGIS options dialog. This should be mapped correctly to maintain OSX standards. In applications on OS X, the keyboard shortcut command-, opens program preferences. Currently, this opens the shortcut settings instead for QGIS. This is also mapped to the same "Preferences" menu item, so both of these issues should be resolved at the same time by remapping that item correctly.
History
#1 Updated by Larry Shaffer over 12 years ago
- File prefs-patch-1.8_git.diff added
John, or anyone else with a Mac, I have fixed this on my latest build. Apply attached patch to test.
Here's what's happening:
The mActionConfigureShortcuts's text starts with 'config' and has its MenuRole set to QAction::TextHeuristicRole0. This causes the action to be used for the Preferences... menu action instead of mActionOptions, even though that action's MenuRole is set to QAction::PreferencesRole [1]. Must be that heuristic check overrides, or comes before, the src/ui/qgisapp.ui setting.
I did the following to fix it:
1) Set mActionConfigureShortcuts's MenuRole to QAction::NoRole so it doesn't cause the issue.
2) Added mActionOptionsMac action (basically a copy of mActionOptions) to the Settings menu. The options() action is in two places, like before, with 1.7.4. This was added because, on the Mac, Qt will move the QAction::PreferencesRole and QAction::AboutRole actions under QGIS (application) menu. Not sure when that happens, otherwise mActionOptions could re-added to the Settings menu, which, as a test, can be done in the Python console after launch.
I chose to just use the text of mActionOptions ('Options...'), since there is a string freeze. However, mActionOptionsMac really should be shifted to 'Preferences...' for that full Mac OS experience.
A more deluxe fix would loop all actions and set any action's MenuRole that starts with config, options, setup, settings or preferences, or about.* to QAction::NoRole excepting mActionOptions and mActionAbout specifically. This way, if someone in the future accidentally adds another problem action to src/ui/qgisapp.ui and forgets to set it to QAction::NoRole (TextHeuristicRole is default), it might override the About or Preferences... menus's actions.
If this fix works for you (works on 10.6.8 and 10.7.4 here), I'll set up a pull request tonight, and contact William Kyngesburye as well. Not sure if he has extra steps to deal with this during packaging, but now it might be in master branch.
[0] http://doc.qt.nokia.com/4.7.1/qaction.html#MenuRole-enum
[1] http://doc.qt.nokia.com/4.7.1/qmenubar.html#qmenubar-on-mac-os-x
#2 Updated by John Tull over 12 years ago
Larry: Sorry I was not able to test this sooner. Was this already applied to trunk? If so, it probably should be closed. I wasn't sure if the patch applied or it was already in the code upstream.
#3 Updated by Larry Shaffer over 12 years ago
- File fix-hijacked-menus-patch.diff added
John, this was mostly fixed with:
f01c6ad8c820a68e3d3050921b22781e100209d2
But I have a better fix now that I need to test on Mac 10.6.8 (good to go on 10.7.4) that keeps all third-party plugins from hijacking About or Preferences. It's more of a hack, but it works well. Will send pull request in an hour or two. I have added it here as a diff, if you would like to test.
It keeps the menu items from being hijacked and preserves the menu items in their respective menus: Options... under Settings and About under Help. Before, Qt would move them from there.
The problem here is that this Mac-only Qt menu merge 'feature' is re-assessed (left-to-right) on each menubar change, not just at launch. So, if a third-party plugin is loaded and adds a top-level menu before the Help menu, it will hijack the app's about menu, if it has a menu that starts with the text 'about'. CadTools does, as well as others. This has been an issue for several QGIS versions now.
My little hack adds duplicate menus for About and Options (they are set to AboutRole and PreferencesRole already) to the bottom of the File Menu, which Qt discovers and moves to the appropriate app menus before it runs into any others, regardless of whether they load later, like with plugins.
One small flaw here is if a plugin or C++ dev adds to the File Menu above these with a TextHeuristicRole-triggered 'about' or 'config, options, etc' action. It is doubtful this will happen, though. Otherwise, I can't think of another solution, short of a change to Qt concerning this. It's a bad auto-feature IMHO.
#4 Updated by Larry Shaffer over 12 years ago
- File fix-hijacked-menus_1.7.4-4.png added
- File fix-hijacked-menus-patch_1.8.0.png added
John, the patch supplied in previous post works well under 10.6.8, too. In the attached screen snap for 1.8.0 you can see CadTools and SEXTANTE (Analysis menu) loaded before Help (after Window, though, due to other ordering issue).
In snap for 1.7.4-4 there was no hijacking of the About menu, but only because of the screwed-up menu ordering issue: Help preceded all plugin menus.
Please let me know if you get similar results with patch and only those two plugins loaded. Compare results under 1.7.4-4 and 1.8.0, if possible. Thanks.
#5 Updated by John Tull over 12 years ago
I did get the same result with Sextante plugin. The Analysis menu item is between Windows and Help. This is tested in trunk build only. I don't have time to build 1.7.4 right now, though.
#6 Updated by Larry Shaffer over 12 years ago
Should be reasonably fixed with:
cc9b8983bc5b9fe37a3233d60fe73c564b758f2b
Recommend setting issue to Closed.
#7 Updated by Larry Shaffer over 12 years ago
This should be set to closed. Fixed for 1.8.0+.
#8 Updated by John Tull over 12 years ago
- Status changed from Open to Closed