----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120149/#review102459 -----------------------------------------------------------
this is a kdelibs4 patch, not relevant anymore - Marco Martin On Sept. 25, 2014, 2:03 p.m., René J.V. Bertin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120149/ > ----------------------------------------------------------- > > (Updated Sept. 25, 2014, 2:03 p.m.) > > > Review request for KDE Software on Mac OS X, kdelibs, KDEPIM, Qt KDE, Marco > Martin, and Olivier Goffart. > > > Repository: kdelibs > > > Description > ------- > > This review is for 2 sets of changes; an initial one to the way "system tray" > are rendered, and a newer set that protects the Preferences menu from getting > linked to any action with an appropriate title. > > -- the system tray: > Until now, "system tray" menus had some rendering issues on Mac OS X: > > - The menu title, the 1st menu item that on Linux shows the application name, > remained empty > - Menu items that can (in principle, potentially) show an icon always showed > the icon > > Point 1 was resolved by emulating the Linux addTitle/setTitle action in > `KStatusNotifierItemPrivate::init()` : the menu title is implemented as a > deactive standard menuitem followed by a separator. This makes the item stand > out on a GUI that doesn't support the kind of formatting in menus as used in > the Linux implementation. > > Point 2 was identified as a Qt issue: `isIconVisibleInMenu` is ignored for > systray menus. It was resolved by adding `KMenu::addAction` methods that > overload the ones from QMenu that were hitherto inherited unchanged by KMenu. > The only different method is `addAction(QAction*)` which removes the icon > from the `QAction` if `isIconVisibleInMenu()` is false. The other `addAction` > methods are "overloaded with themselves" with `using QMenu::addAction;` in > the header file. > > -- the Preferences menu item > This is a menu item living in the Application menu, a menu that sits in the > menubar between the Apple (?) menu and the File menu. This menu also contains > the Quit command. > KDE and Qt applications typically do not set up their menus in this fashion, > so Qt provides an automatic way to put relevant menu items (actions) in the > Application menu, using Apple's naming. The algorithm is described under > QMenuBar in the Qt documentation: for the Preferences action, it will > consider any action that has a text containing `config`, `options`, > `settings` or `preferences`, and put it under the Preferences label if its > menu role is set to `heuristic` (which appears to be the default). > In practice, many applications provide a series of menu actions with names > that trigger this method, and they do not always create their own > preferences/settings/configuration menu first. Yet it is the first menu > action that matches that will be installed under the Preferences menu, with > the Command-, shortcut. A good example is KDevelop: it will have a > Preferences menu that activates the `Configure Selection` action - which does > not open a settings dialog but launches the configure or cmake procedure for > the selected project ... > > My proposed solution overrides this Qt behaviour. On OS X, the `KAction(const > QString &text, QObject *parent)` constructor calls a modified (static) > function `setTextWithCorrectMenuRole` which checks the text against the > patterns Qt will consider for `PreferencesRole`. If it finds a match, it will > force the role to `NoRole`, unless it is a perfect match with the standard > KDE configuration action for the application (`"&Configuration appName..."`) > in which case it sets the role to `PreferencesRole`. This latter > consideration allows kdelibs to "catch" the configuration menu for > applications like KMail, which appear not to be created using > KStandardActions. > This approach can be extended to other menu actions that end up incorrectly > in the OS X Application menu. > > Applications that create menu actions using QAction or a different KAction > constructor will see no change (and should use `setMenuRole` selectively on > OS X). > > > Diffs > ----- > > kdeui/actions/kaction.cpp 9e8f7fb > kdeui/notifications/kstatusnotifieritem.cpp 1b15d40 > > Diff: https://git.reviewboard.kde.org/r/120149/diff/ > > > Testing > ------- > > Testing was done with kdelibs git/master and KDE/MacPorts on OS X 10.6.8 . > The modified code is in compile-time conditional blocks used only on OS X, so > no regressions are to be expected on other platforms. > > KF5 is not production ready on OS X, so I am not currently able to port these > modifications beyond KDE4. However, I did see that Qt5 has a new approach to > adding titles to menus, which can be described as a "labelled separator". > Backporting that function from the Qt5 source to kdelibs gave menu items that > had the separator but not the text (title) label. It is thus likely that some > kind of emulation will also be required with KF5, on OS X. > > I considered doing the addTitle/setTitle emulation in kmenu.cpp, but decided > against that for now. Menu titles are rendered as under Linux in menus that > are not attached to the OS X toplevel menubar (say in context menus). Without > knowing how to distinguish the kind of menu in KMenu methods the emulation > will have to remain in the client code. > > The Preferences menu protection should carry over easily to KF5, supposing > Qt5 uses the same heuristics to place relevant menu actions under the OS X > application menu, and supposing `KAction` has made the transition to KF5. > > > Thanks, > > René J.V. Bertin > >
