> On Sept. 28, 2014, 3:13 nachm., Thomas Lübking wrote: > > kdeui/widgets/kmenu.cpp, line 220 > > <https://git.reviewboard.kde.org/r/120403/diff/1/?file=315609#file315609line220> > > > > .... mehhh: branched exits. > > René J.V. Bertin wrote: > you realise that the 2 exits don't return the exact same kind of symbol? > Using 1 exit point would mean copy/casting the QWidgetAction* to a QAction* > or vice versa. > > Thomas Lübking wrote: > QWidgetAction is a QAction, you can just have sth. like > > QAction *action; > ... > if () { > ... > action = widgetAction; > } else { > ... > } > ... > return action; > > though in the particular case, you can also > > if () { > action = new QWidgetAction(.); > ... > static_cast<QWidgetAction*>(action)->setDefaultWidget(titleButton); > ... > } else { > ... > } > > since that seems the only position where the QWidgetAction API is > actually required. > > René J.V. Bertin wrote: > `action = new QAction` in that second excerpt, undoubtedly. > > You'll have noticed that I followed option 1 in the updated diff.
No. This is legit: QAction *action = new QWidgetAction(.); QWidgetAction is *also* a QAction. virtual functions called on "action" will be those from QWidgetAction, otherwise (non virtual/QWidgetAction exclusive) you must explicitly cast to QWidgetAction. Same goes eg. for QObject *o = new QWidget(); - Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120403/#review67559 ----------------------------------------------------------- On Sept. 30, 2014, 8:42 nachm., René J.V. Bertin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120403/ > ----------------------------------------------------------- > > (Updated Sept. 30, 2014, 8:42 nachm.) > > > Review request for KDE Software on Mac OS X, kdelibs and Qt KDE. > > > Repository: kdelibs > > > Description > ------- > > This is a spin-off of RR https://git.reviewboard.kde.org/r/120355/ > > As described in that RR, OS X cannot render menu items that were created > using `KMenu::addTitle`. Without a Qt patch, that function will even provoke > a crash. With a patch, the items are rendered barely legibly once, and then > render as empty space in subsequent menu openings. > The main object of that RR is to present and discuss a workaround at the > client level, emulating `KMenu::addTitle`. In this RR, I present a draft > adaptation of that function itself. > > The main goal as I see it is to modify the function just enough so that it > changes its behaviour for items that will or can be rendered in the Mac's > global menubar, using non-Qt code. Pop-up menus that are not attached to that > structure are rendered through Qt and can show all the style formatting they > can under Linux as long as it's not tied to X11 directly. > It's probably impossible to cater to all possible use cases, so I'd propose > to focus on the situations we can detect from inside `KMenu::addTitle`. That > is, *if* we want to ease the client's burden of obtaining a sensible menu > item *and* we want to maintain support for the intended/expected style in the > menus that can actually support them. (KDevelop's context menu its Project > view is a prime example.) > The other goal (secondary for KDE/Mac for the time being) is to come up with > a patch proposal for Qt5's `QMenu::addSection`, because its current use of > texted separators makes it equivalent to `QMenu::addSeparator` on OS X. (= > you get a separator instead of an item showing the title text.) > > I have not found a way to detect reliably that a menu is attached to the > global menubar. There are functions that are supposed to allow this > (KMenu::isTopLevelMenu, QMenu::macMenu) but they don't work as expected. So > what I propose here is to emulate the styled menu title when adding to a > KMenu that is somehow associated to a KMenu belonging to a KMainWindow that > has a menubar. This can probably lead to false hits, and I have already > learned that it doesn't catch all the intended cases either (e.g. > MessageList->Sorting menu under KMail's View menu is apparently not yet > attached when addTitle is called). So I'm following Thomas's suggestion to > publish the draft for feedback. > > In case anyone wonders about the emulation code (when notMacMenuBar==false): > I'm open for suggestions but this is the only approach I've found to create > an entry that stands out. It's not impossible to to obtain the font OS X uses > for menubar menu items (Lucida Grande 14; requires 2 extra functions), but > changing font attributes (bold, underline, overline etc) has no effect. (The > bold font version file is available, so it might be possible to get the bold > font rendered by specifying it as a full font spec and not as an attribute > but I wouldn't know how to achieve this.) > > > Diffs > ----- > > kdeui/widgets/kmenu.cpp 7dab149 > > Diff: https://git.reviewboard.kde.org/r/120403/diff/ > > > Testing > ------- > > On OS X 10.6.8 with kdelibs git/kde4.14 . Currently the draft does nothing on > other OSes, and the actual "emulation" code (when notMacMenuBar==false) can > go conditional if there are no other platforms where a similar approach could > be desired. > > > Thanks, > > René J.V. Bertin > >
