<table><tr><td style="">davidre added a comment.
</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/D28107">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D28107#631804" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D28107#631804</a>, <a href="https://phabricator.kde.org/p/kmaterka/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@kmaterka</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><blockquote style="border-left: 3px solid #8C98B8;
color: #6B748C;
font-style: italic;
margin: 4px 0 12px 0;
padding: 8px 12px;
background-color: #F8F9FC;">
<div style="font-style: normal;
padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D28107#630144" style="background-color: #e7e7e7;
border-color: #e7e7e7;
border-radius: 3px;
padding: 0 4px;
font-weight: bold;
color: black;text-decoration: none;">D28107#630144</a>, <a href="https://phabricator.kde.org/p/davidre/" style="
border-color: #f1f7ff;
color: #19558d;
background-color: #f1f7ff;
border: 1px solid transparent;
border-radius: 3px;
font-weight: bold;
padding: 0 4px;">@davidre</a> wrote:</div>
<div style="margin: 0;
padding: 0;
border: 0;
color: rgb(107, 116, 140);"><p>It seems it is possible to do this (removing stuff from the data engine) after all. I will try to work on this in the next time</p></div>
</blockquote>
<p>IMO, ideally <tt style="background: #ebebeb; font-size: 13px;">StatusNotifierItemSource</tt> should just expose most of properties from this specification without any changes:<br />
<a href="https://www.freedesktop.org/wiki/Specifications/StatusNotifierItem/StatusNotifierItem/" class="remarkup-link" target="_blank" rel="noreferrer">https://www.freedesktop.org/wiki/Specifications/StatusNotifierItem/StatusNotifierItem/</a><br />
For some complex types it should be allowed to do conversion etc.</p>
<p>Summary of properties:</p>
<div class="remarkup-table-wrap"><table style="border-collapse: separate;
border-spacing: 1px;
background: #d3d3d3;
margin: 12px 0;">
<tr><td style="background: #ffffff;
padding: 3px 6px;">SNI property</td><td style="background: #ffffff;
padding: 3px 6px;">In specification</td><td style="background: #ffffff;
padding: 3px 6px;">Simple/Complex</td><td style="background: #ffffff;
padding: 3px 6px;">DataSource property</td><td style="background: #ffffff;
padding: 3px 6px;">Comments</td></tr>
<tr><td style="background: #ffffff;
padding: 3px 6px;">Category</td><td style="background: #ffffff;
padding: 3px 6px;">Yes</td><td style="background: #ffffff;
padding: 3px 6px;">Simple</td><td style="background: #ffffff;
padding: 3px 6px;">Category</td><td style="background: #ffffff;
padding: 3px 6px;">Direct copy</td></tr>
<tr><td style="background: #ffffff;
padding: 3px 6px;">Id</td><td style="background: #ffffff;
padding: 3px 6px;">Yes</td><td style="background: #ffffff;
padding: 3px 6px;">Simple</td><td style="background: #ffffff;
padding: 3px 6px;">Id</td><td style="background: #ffffff;
padding: 3px 6px;">Direct copy</td></tr>
<tr><td style="background: #ffffff;
padding: 3px 6px;">Title</td><td style="background: #ffffff;
padding: 3px 6px;">Yes</td><td style="background: #ffffff;
padding: 3px 6px;">Simple</td><td style="background: #ffffff;
padding: 3px 6px;">Title</td><td style="background: #ffffff;
padding: 3px 6px;">Direct copy</td></tr>
<tr><td style="background: #ffffff;
padding: 3px 6px;">Status</td><td style="background: #ffffff;
padding: 3px 6px;">Yes</td><td style="background: #ffffff;
padding: 3px 6px;">Simple</td><td style="background: #ffffff;
padding: 3px 6px;">Status</td><td style="background: #ffffff;
padding: 3px 6px;">Direct copy</td></tr>
<tr><td style="background: #ffffff;
padding: 3px 6px;">WindowId</td><td style="background: #ffffff;
padding: 3px 6px;">Yes</td><td style="background: #ffffff;
padding: 3px 6px;">Simple</td><td style="background: #ffffff;
padding: 3px 6px;">WindowId</td><td style="background: #ffffff;
padding: 3px 6px;">Direct copy</td></tr>
<tr><td style="background: #ffffff;
padding: 3px 6px;">ItemIsMenu</td><td style="background: #ffffff;
padding: 3px 6px;">Yes</td><td style="background: #ffffff;
padding: 3px 6px;">Simple</td><td style="background: #ffffff;
padding: 3px 6px;">ItemIsMenu</td><td style="background: #ffffff;
padding: 3px 6px;">Direct copy</td></tr>
<tr><td style="background: #ffffff;
padding: 3px 6px;">AttentionMovieName</td><td style="background: #ffffff;
padding: 3px 6px;">Yes</td><td style="background: #ffffff;
padding: 3px 6px;">Simple</td><td style="background: #ffffff;
padding: 3px 6px;">AttentionMovieName</td><td style="background: #ffffff;
padding: 3px 6px;">Direct copy</td></tr>
<tr><td style="background: #ffffff;
padding: 3px 6px;">ToolTip</td><td style="background: #ffffff;
padding: 3px 6px;">Yes</td><td style="background: #ffffff;
padding: 3px 6px;">Complex</td><td style="background: #ffffff;
padding: 3px 6px;">ToolTipTitle, ToolTipSubTitle, ToolTipIcon</td><td style="background: #ffffff;
padding: 3px 6px;">Converted to separate properties, additional logic</td></tr>
<tr><td style="background: #ffffff;
padding: 3px 6px;">Menu</td><td style="background: #ffffff;
padding: 3px 6px;">Yes</td><td style="background: #ffffff;
padding: 3px 6px;">Complex</td><td style="background: #ffffff;
padding: 3px 6px;">-</td><td style="background: #ffffff;
padding: 3px 6px;">Converted to QMenu, special handling</td></tr>
<tr><td style="background: #ffffff;
padding: 3px 6px;">IconThemePath</td><td style="background: #ffffff;
padding: 3px 6px;">No!</td><td style="background: #ffffff;
padding: 3px 6px;">Simple</td><td style="background: #ffffff;
padding: 3px 6px;">IconThemePath</td><td style="background: #ffffff;
padding: 3px 6px;">Direct copy</td></tr>
<tr><td style="background: #ffffff;
padding: 3px 6px;">IconName</td><td style="background: #ffffff;
padding: 3px 6px;">Yes</td><td style="background: #ffffff;
padding: 3px 6px;">Simple</td><td style="background: #ffffff;
padding: 3px 6px;">IconName</td><td style="background: #ffffff;
padding: 3px 6px;">Only if IconPixmap is empty</td></tr>
<tr><td style="background: #ffffff;
padding: 3px 6px;">IconPixmap</td><td style="background: #ffffff;
padding: 3px 6px;">Yes</td><td style="background: #ffffff;
padding: 3px 6px;">Complex</td><td style="background: #ffffff;
padding: 3px 6px;">Not available</td><td style="background: #ffffff;
padding: 3px 6px;">Used as part of Icon</td></tr>
<tr><td style="background: #ffffff;
padding: 3px 6px;">OverlayIconName</td><td style="background: #ffffff;
padding: 3px 6px;">Yes</td><td style="background: #ffffff;
padding: 3px 6px;">Simple</td><td style="background: #ffffff;
padding: 3px 6px;">OverlayIconName</td><td style="background: #ffffff;
padding: 3px 6px;">Only if OverlayIconPixmap is empty</td></tr>
<tr><td style="background: #ffffff;
padding: 3px 6px;">OverlayIconPixmap</td><td style="background: #ffffff;
padding: 3px 6px;">Yes</td><td style="background: #ffffff;
padding: 3px 6px;">Complex</td><td style="background: #ffffff;
padding: 3px 6px;">Not available</td><td style="background: #ffffff;
padding: 3px 6px;">Used as part of Icon</td></tr>
<tr><td style="background: #ffffff;
padding: 3px 6px;">-</td><td style="background: #ffffff;
padding: 3px 6px;">-</td><td style="background: #ffffff;
padding: 3px 6px;">Simple</td><td style="background: #ffffff;
padding: 3px 6px;">Icon</td><td style="background: #ffffff;
padding: 3px 6px;">Complicated logic to create it from all Icon properties</td></tr>
<tr><td style="background: #ffffff;
padding: 3px 6px;">AttentionIconName</td><td style="background: #ffffff;
padding: 3px 6px;">Yes</td><td style="background: #ffffff;
padding: 3px 6px;">Simple</td><td style="background: #ffffff;
padding: 3px 6px;">AttentionIconName</td><td style="background: #ffffff;
padding: 3px 6px;">Only if AttentionIconPixmap is empty</td></tr>
<tr><td style="background: #ffffff;
padding: 3px 6px;">AttentionIconPixmap</td><td style="background: #ffffff;
padding: 3px 6px;">Yes</td><td style="background: #ffffff;
padding: 3px 6px;">Not available</td><td style="background: #ffffff;
padding: 3px 6px;">Used as part of AttentionIcon</td><td style="background: #ffffff;
padding: 3px 6px;"></td></tr>
<tr><td style="background: #ffffff;
padding: 3px 6px;">-</td><td style="background: #ffffff;
padding: 3px 6px;">-</td><td style="background: #ffffff;
padding: 3px 6px;">Simple</td><td style="background: #ffffff;
padding: 3px 6px;">AttentionIcon</td><td style="background: #ffffff;
padding: 3px 6px;">Complicated logic to create it from both AttentionIcon properties</td></tr>
<tr></tr>
</table></div>
<p>I skipped: IconsChanged, StatusChanged, TitleChanged, ToolTipChanged, these are not relevant (and not used anymore).</p>
<p>I would suggest to:</p>
<ul class="remarkup-list">
<li class="remarkup-list-item">In <tt style="background: #ebebeb; font-size: 13px;">StatusNotifierItemSource</tt>:<ul class="remarkup-list">
<li class="remarkup-list-item">Always set IconName, OverlayIconName, AttentionIconName, regardless if IconPixmap is set or not</li>
<li class="remarkup-list-item">Add new properties for all pixmaps, convert them to proper type (there is function for that: <tt style="background: #ebebeb; font-size: 13px;">imageVectorToPixmap()</tt>)</li>
</ul></li>
<li class="remarkup-list-item">Icon logic should be in <tt style="background: #ebebeb; font-size: 13px;">StatusNotifierItem.qml</tt><ul class="remarkup-list">
<li class="remarkup-list-item">Ignore "Icon" property</li>
<li class="remarkup-list-item">Implement similar logic in QML, everything is available in <tt style="background: #ebebeb; font-size: 13px;">PlasmaCore.IconItem</tt></li>
<li class="remarkup-list-item">Use icon name first, then pixmap (align to specification: "Visualizations are encouraged to prefer icon names over icon pixmaps if both are available")
<br /><br />
Then, is a separate diff, remove unused properties/logic from StatusNotifierItemSource (in Plasma 6?).
<br /><br />
This ensures backward compatibility. I'm pretty sure <tt style="background: #ebebeb; font-size: 13px;">StatusNotifierItemSource</tt> is not used outside of Plasma, but if it this is part of the public API then needs to stay for now. Presentation logic is moved to correct layer. That should simplify it a little bit, I had hard time understanding it for the first time.</li>
</ul></li>
</ul></div>
</blockquote>
<p>Thanks for the comprehensive write up! I fully agree with you!</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R120 Plasma Workspace</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D28107">https://phabricator.kde.org/D28107</a></div></div><br /><div><strong>To: </strong>davidre, kmaterka, Plasma, broulik, davidedmundson<br /><strong>Cc: </strong>anthonyfieroni, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart<br /></div>