Review Request 127971: [KToolBar] Disable toggleViewAction() when actions/options_show_toolbar is restricted

David Faure faure at kde.org
Sun May 22 12:05:39 UTC 2016



> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160522/8fdfee4f/attachment.html>


More information about the Plasma-devel mailing list