<table><tr><td style="">rkflx accepted this revision.<br />rkflx added a comment.<br />This revision is now accepted and ready to land.
</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/D10972">View Revision</a></tr></table><br /><div><div><p>Thanks so much, Friedrich. Latest changes work great, only one more small thing found (but solvable, I guess).</p>

<p>I know I have been asking for a lot in this review, but I think users will appreciate the polish. After editing the commit message, let's land this ;)</p>

<hr class="remarkup-hr" />

<p>There's now also <a href="https://phabricator.kde.org/D10972" style="background-color: #e7e7e7;
          border-color: #e7e7e7;
          border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;">D10972: [RFC] Exposing slideshow to MPRIS controllers</a> where I added some of the things which could be solved later, so they are not forgotten and interested contributors can jump right in. Feel free to add to this (but please keep it concise) or add subtasks.</p></div></div><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/D10972#inline-55643">View Inline</a><span style="color: #4b4d51; font-weight: bold;">mprismediaplayer2.cpp:82</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">    <span style="color: #aa4000">return</span> <span class="n">QGuiApplication</span><span style="color: #aa2211">::</span><span class="n">desktopFileName</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">This is the only part which prevents compiling with Qt 5.6 as found on Leap 42.3 and elsewhere, which would be kinda nice.</p>

<p style="padding: 0; margin: 8px;">Are there any alternatives? Or could we return <tt style="background: #ebebeb; font-size: 13px;">QString()</tt> for older Qts? The basic functionality still seemed fine to me when testing, so perhaps this is not strictly needed?</p>

<p style="padding: 0; margin: 8px;">Otherwise we'd have to bite the bullet and bump the Qt version to 5.7.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R260 Gwenview</div></div></div><br /><div><strong>BRANCH</strong><div><div>addmprisservice</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D10972">https://phabricator.kde.org/D10972</a></div></div><br /><div><strong>To: </strong>kossebau, Gwenview, rkflx<br /><strong>Cc: </strong>mtijink, ngraham, nicolasfella, KDE Connect, rkflx, broulik<br /></div>