> On Sept. 28, 2014, 5:13 p.m., Thomas Lübking wrote: > > kdeui/widgets/kmenu.cpp, line 182 > > <https://git.reviewboard.kde.org/r/120403/diff/1/?file=315609#file315609line182> > > > > try to avoid negated bools unless for performance or readability > > reasons (ie. you'd need "!flag" a lot otherwise)
isNOSXMenuBar? ;) > On Sept. 28, 2014, 5:13 p.m., Thomas Lübking wrote: > > kdeui/widgets/kmenu.cpp, line 189 > > <https://git.reviewboard.kde.org/r/120403/diff/1/?file=315609#file315609line189> > > > > As mentioned before, the parentship is pretty meaningless here. A popup > > *might* be parented by a window that has it in its menubar, but doesn't > > have to at all. Not necessarily at "this moment", but I've only seen menubars attached to windows until now ... > On Sept. 28, 2014, 5:13 p.m., Thomas Lübking wrote: > > kdeui/widgets/kmenu.cpp, line 186 > > <https://git.reviewboard.kde.org/r/120403/diff/1/?file=315609#file315609line186> > > > > parsing associatedWidgets (eg. a QMenu or a QMenuBar or a QToolButton) > > is a good idea, but what you want to do is to search yourself through pot. > > QMenu's until one ::menuAction() is associated to a QMenuBar (ie. "this > > menu can be reached by navigating through a menubar and its menus) I think you've lost me a bit here. Not to appear dumb or lazy, but how does one do that? Are we looking at a recursive function that gets called for each QMenu encountered (starting with 'this', calling itself afterwards) and cycles through all that QMenu's ::menuAction()->associatedWidgets, until we hit a widget for which `qobject_cast<QMenuBar*>(w) != NULL`? (Or is there a better way to check the type of a QWidget?) (AAArrgggh, all this would be much simpler if KDevelop would stop exitting for no apparent reason and without any error message like it just did, again) > On Sept. 28, 2014, 5:13 p.m., Thomas Lübking wrote: > > kdeui/widgets/kmenu.cpp, line 191 > > <https://git.reviewboard.kde.org/r/120403/diff/1/?file=315609#file315609line191> > > > > If I understand the Qt doc correctly, there're basically two kinds of > > menubars: > > 1) per window > > 2) shared > > > > however, they all will end up as global menu? > > > > Even if not: > > how many menubars are not mapped to the global one (the I could think > > of Qt Designer forms) and are those false positives actually worth the > > false negatives (ie. you miss special casing a menu)? For the moment we're talking KDE, not (pure) Qt apps. The only KDE app I know that uses per-window menus even on OS X is konsole. And it doesn't use KMenu::addTitle. What's your point? That in the end I might be better off just emulating the title rendering for all popup menus, even if they actually could render the "official" style? - René J.V. ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120403/#review67559 ----------------------------------------------------------- On Sept. 28, 2014, 2:46 p.m., René J.V. Bertin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120403/ > ----------------------------------------------------------- > > (Updated Sept. 28, 2014, 2:46 p.m.) > > > 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 > >
