<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/127971/">https://git.reviewboard.kde.org/r/127971/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 22nd, 2016, 11:49 a.m. UTC, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</blockquote>
<p>On May 22nd, 2016, 11:59 a.m. UTC, <b>Kai Uwe Broulik</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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 ;-)</p></pre>
<br />
<p>- David</p>
<br />
<p>On May 19th, 2016, 9:37 p.m. UTC, Kai Uwe Broulik wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks and Plasma.</div>
<div>By Kai Uwe Broulik.</div>
<p style="color: grey;"><i>Updated May 19, 2016, 9:37 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kxmlgui
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Comes with a unittest.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I can no longer hide the main toolbar in Dolphin or Gwenview when this action is restricted by simply right-clicking the menu bar.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>autotests/ktoolbar_unittest.cpp <span style="color: grey">(d6c1e05)</span></li>
<li>src/ktoolbar.cpp <span style="color: grey">(8fcb9cb)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127971/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2016/05/19/076745b4-03a2-400f-9506-46999d4ce1e1__toolbartoggle.png">The Menu</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>