<table><tr><td style="">kossebau added inline comments.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D28765">View Revision</a></tr></table><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D28765#inline-166595">View Inline</a><span style="color: #4b4d51; font-weight: bold;">dfaure</span> wrote in <span style="color: #4b4d51; font-weight: bold;">kcmoduleinfo.h:131</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">It's complicated.</p>

<ol class="remarkup-list">
<li class="remarkup-list-item">If you use the QString constructor, you know service() is usable. That's the case for all users of KCModuleInfo except KCModuleLoader. [Not that there are many]</li>
</ol>

<ol class="remarkup-list" start="2">
<li class="remarkup-list-item">Even KCModuleLoader calls service(), to test for noDisplay().</li>
</ol>

<p style="padding: 0; margin: 8px;">The whole concept of NoDisplay only makes sense for desktop files, unless we want to have that in JSON metadata as well. I suppose we do, but this currently seems to be completely missing (e.g. from KPluginMetaData, if we want this in all plugins, not just KCMs). We'd have to duplicate the logic currently in KService::noDisplay().</p>

<p style="padding: 0; margin: 8px;">Any volunteers to look into this? :-)</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">As I just ran into this code, a short reminder: it would be a regression not to have a noDisplay check in the case of pure JSON metadata, as the current code of KService::;noDisplay() also asks KAuth whether the page should be shown (<tt style="background: #ebebeb; font-size: 13px;">KAuthorized::authorizeControlModule(storageId())</tt>), which some KIOSK systems or admins might rely on, but also <tt style="background: #ebebeb; font-size: 13px;">showInCurrentDesktop()</tt> & <tt style="background: #ebebeb; font-size: 13px;">showOnCurrentPlatform()</tt>, which also still make sense, for what I can tell.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R295 KCMUtils</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D28765">https://phabricator.kde.org/D28765</a></div></div><br /><div><strong>To: </strong>dfaure, pino, broulik, mart, davidedmundson, ngraham, svuorela<br /><strong>Cc: </strong>kossebau, svuorela, cblack, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns<br /></div>