> On Sept. 28, 2014, 3:13 nachm., 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) > > René J.V. Bertin wrote: > isNOSXMenuBar? ;)
reverse logics. "isMacMenuBar = false;" > On Sept. 28, 2014, 3:13 nachm., 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. > > René J.V. Bertin wrote: > Not necessarily at "this moment", but I've only seen menubars attached to > windows until now ... Menubars, yes. QMenus, no. > On Sept. 28, 2014, 3:13 nachm., 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)? > > René J.V. Bertin wrote: > 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? > just emulating the title rendering for all popup menus, even if they actually > could render the "official" style? That. Yes. > On Sept. 28, 2014, 3:13 nachm., 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) > > René J.V. Bertin wrote: > 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) ```cpp static bool isMenubarMenu(QMenu *m) { if (!m) return false; if (!m->menuAction()) return false; foreach (QWidget *w, m->menuAction()->associatedWidgets()) { if (qobject_cast<QMenuBar*>(w)) return true; if (isMenubarMenu(qobject_cast<QMenu*>(w))) return true; } return false; } ``` depending on how silly client codes are, one might have to catch in/direct recursions: ```cpp if (w == m) return false; // direct recursion, menu is submenu of itself if (seenWidgets.contains(w)) continue; // indirect recursion, menu is sub-submenu of itself seenWidgets << w; ``` please notice that the above has sketch-quality only (browsers make crap code editors ;-) - Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120403/#review67559 ----------------------------------------------------------- On Sept. 28, 2014, 12:46 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. 28, 2014, 12:46 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 > >
