> On May 22, 2016, 11:49 a.m., David Faure wrote:
> > Looks fine to me, except for the reasoning in the commit log. It's 
> > perfectly fine to reimplement an existing virtual method from a base class 
> > into a derived class, this doesn't break ABI.
> 
> Kai Uwe Broulik wrote:
>     But Abi wiki page says you cannot if it's a "non leaf class ie. one that 
> is meant to be subclassed" which KMainWindow is. Regardless of whether we 
> provide our own menu the action should be disabled either way though.

You can, it's just that existing apps might not call your reimplementation 
until they get recompiled too. Which would only mean they don't get the fix, in 
this particular case, it doesn't mean they would get a new breakage. But yeah, 
orthogonal discussion. Just don't remove this part from the commit log, if it's 
irrelevant anyway ;-)


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127971/#review95698
-----------------------------------------------------------


On May 19, 2016, 9:37 p.m., Kai Uwe Broulik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127971/
> -----------------------------------------------------------
> 
> (Updated May 19, 2016, 9:37 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: kxmlgui
> 
> 
> Description
> -------
> 
> While KXmlGui makes sure to prevent the user from hiding toolbars when not 
> allowed, right-clicking an empty space in a QMainWindow, such as the menu 
> bar, will yield a menu created by Qt, which knows nothing about Kiosk 
> restrictions, listing all toolbar toggle actions of this window.
> 
> 
> Diffs
> -----
> 
>   autotests/ktoolbar_unittest.cpp d6c1e05 
>   src/ktoolbar.cpp 8fcb9cb 
> 
> Diff: https://git.reviewboard.kde.org/r/127971/diff/
> 
> 
> Testing
> -------
> 
> Comes with a unittest.
> 
> I can no longer hide the main toolbar in Dolphin or Gwenview when this action 
> is restricted by simply right-clicking the menu bar.
> 
> 
> File Attachments
> ----------------
> 
> The Menu
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/05/19/076745b4-03a2-400f-9506-46999d4ce1e1__toolbartoggle.png
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to