<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/115282/">https://git.reviewboard.kde.org/r/115282/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 24th, 2014, 10:18 a.m. UTC, <b>David Edmundson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/115282/diff/1/?file=235593#file235593line264" style="color: black; font-weight: bold; text-decoration: underline;">telepathy-mpris.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void TelepathyMPRIS::setTrackToPresence()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">264</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">Tp</span><span class="o">::</span><span class="n">Presence</span> <span class="n">requestedPresence</span> <span class="o">=</span> <span class="n">m_globalPresence</span><span class="o">-></span><span class="n"><span class="hl">requested</span>Presence</span><span class="p">();</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">264</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">Tp</span><span class="o">::</span><span class="n">Presence</span> <span class="n">requestedPresence</span> <span class="o">=</span> <span class="n">m_globalPresence</span><span class="o">-></span><span class="n"><span class="hl">current</span>Presence</span><span class="p">();</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think this will cause problems.
If you set yourself to Invisible and have a local-xmpp account which doesn't support invisible. When you listen to a track it will now move all your accounts "Away".
Could you explain the problem we're trying to solve?</pre>
</blockquote>
<p>On January 25th, 2014, 3:15 a.m. UTC, <b>James Smith</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;">#114569 exposed an issue where the status will noticeably flicker in ktp-plasma-applet when the status is set by the AutoAway plugin (from Available state) and subsequent track changes in MPRIS2 change the status without updating the away message, which is set in AutoAway and takes precedence. The status will quickly switch back to Away from Available, returning the Away status message instead of the MPRIS2 status message (and associated Available state).
So, don't even bother updating the MPRIS2 status and message.</pre>
</blockquote>
<p>On January 26th, 2014, 9:16 a.m. UTC, <b>David Edmundson</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;">Why does it go back to available before it goes to whatever MPRIS is set to?</pre>
</blockquote>
<p>On January 26th, 2014, 9:28 a.m. UTC, <b>James Smith</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;">No, I don't think so. Because we're requesting what the presence is RIGHT NOW, removing the current status message and pasting a new one in and relying on the plugin priority to show a change in persistent status or not, the MC plugins shouldn't receive a status change and instead should remain at what the Ktp plugin allowed or explicitly (not persistently) set. The away KTp plugin, being higher priority, won't allow the mpris2 KTp plugin to maintain a changed status or changed status message.
requestedPresence() should (will) return only what the global presence was requested to be set at, not what currentPresence() reported the global presence to actually be (plugins are taken into account by requestedPresence()).
The jump to global Available was caused by requestedPresence() reporting the user-set presence, not the plugin-priority forced Away presence. The mpris2 status message was ignored (properly) by the plugin ordering; however the status flickered while mpris2 KTp plugin set the requested user presence, changed the status message, and then returned (only to have the Away KTp plugin re-take control over global status based on its timer, which caused a reset to away hopefully before anybody noticed what happened by KTp mpris2.)
I don't know if anyone would need mpris2 status messages while the machine is autoway or on screensaver away? Probably bad netiquette.</pre>
</blockquote>
<p>On January 26th, 2014, 9:56 a.m. UTC, <b>David Edmundson</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;">>The jump to global Available was caused by requestedPresence() reporting the user-set presence, not the plugin-priority forced Away presence. The mpris2 status message was ignored (properly) by the plugin ordering;
Why would it be ignored?
- user is playing music
- mpris plugin activates
- presence is [online - "Some Song"]
- user goes away
- autoaway plugin activates
- presence is [away - "I am not here right now"]
- user comes
- auto away plugin deactivates
- presence gets set to that of the mpris plugin [online - "Some song"]
I can imagine a bug in which a track change whilst away sets you to [away - "Some song"] when you come back; but this patch won't fix that, and that's not this flickering you describe.
</pre>
</blockquote>
<p>On January 26th, 2014, 3:41 p.m. UTC, <b>James Smith</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;">Since KTp MPRIS2 doesn't actually change the global status, but does set the status message, it probably belongs in its own class of plugin.
This approach seems proper, since taking the current status and adding a status message is correct when relying on the plugin ordering mechanism to properly set and unset the global presence. Any change in global status unsets the MPRIS2 status message, which is probably great for people who leave both music and IM open while physically away from the machine or with a screensaver, providing a bit more privacy.
Status messages are bound by plugin ordering, just like global presences.
Reordering the plugins with mpris2 before the state-change plugins doesn't work either, causing the away plugin to wait on the track change. I think I might be right in thinking the plugins are ordered the way they are out of necessity? MPRIS2 won't interoperate with Away at all unless its pulling up the rear and only then with #114569 and this patch applied.
Unrelated, but there are currently no checks for hidden status and for preventing a plugin from picking a visible state while manually the global presence is hidden.</pre>
</blockquote>
<p>On January 26th, 2014, 4:06 p.m. UTC, <b>David Edmundson</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;">I still don't understand why the current system breaks. Until I understand that I can't sign off on anything.
Can you explain in steps like above how and why this flicker occurs?</pre>
</blockquote>
<p>On January 28th, 2014, 2:03 p.m. UTC, <b>James Smith</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;">1 run juk on a long playlist
2 open dragon and run a long movie without setting off autoaway
3 watch as between tracks in juk ktp tray jumps to available with new track information and then back to away
4 verify you arent seeing things by watching a tp plugin for presence change in a dbus
I'm working on a more robust fix for these two issues.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I finally got round to trying to reproduce this. (sorry for the delay btw)
Using Tomahawk (not Juk) I didn't see any flicker my setup was:
1) start listening to music
2) set myself to "Now listening to..." (worked correctly)
3) ..waiting
4) went to autoaway
5) on track changes everything stayed as away.
Moving out of autoaway brought me back to NowPlaying. Pausing that brought me back to Online.
Everything seemed ok \o/
Maybe JUK sends super weird MPRIS signals and sends a pause and play. Perhaps we can capture this in bustle?
</pre>
<br />
<p>- David</p>
<br />
<p>On January 24th, 2014, 3:43 a.m. UTC, James Smith wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Telepathy and Xuetian Weng.</div>
<div>By James Smith.</div>
<p style="color: grey;"><i>Updated Jan. 24, 2014, 3:43 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
ktp-kded-module
</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;">Fixes state flicker when away plugin is active and track changes in a media player.</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;">Compile, run.</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>telepathy-mpris.cpp <span style="color: grey">(1c7b98c)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/115282/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>