<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/115425/">https://git.reviewboard.kde.org/r/115425/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">If the kded is used to enable / disable nowplaying using the contactlist to additionally enable / disable nowplaying or enabling nowplaying while set on a custom presence the contactlist selector completely borks.
The kded module in master now defaults status + message plugins highest priority followed by user-specified custom presences and then message plugin only messages. This was the better approach eg fallback benefits of user over automatic, automatic over empty status messages, force the user to disable plugins for empty status message availability, custom status over message plugin for user-take-control, automatic message over empty automatic presence + message etc. Also the addition of new status message plugins will require rethinking nowplaying's prominence and enable / disable mechanisms.
This approach had the additional benefit of solving a number of status-sticking bugs in the contact list selector, but exposed bugs in the nowplaying activation handling when changing from a custom user presence by starting nowplaying. Nowplaying activation now needs a consistency between the contact list and the kded module and the kconfig to properly function.
There are 2 approaches:
1) Modify the kded code to pick up the old behavior i.e. all state-sticky bugs and functioning nowplaying interface in the contact list.
problems - sometimes returns from busy-away status after track change to the wrong status message, depending on in what state now playing was activated from. Sometimes sticks on custom presences when clearing to available status.
2) Change the now playing engaging behavior in the contactlist, to also lessen the visual priority like what kded module now does internally.
problems - contact list won't activate nowplaying from a custom user presence, to get an empty available status message, all custom status message plugins must be disabled.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 15th, 2014, 1:38 p.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/115425/diff/3/?file=244160#file244160line345" style="color: black; font-weight: bold; text-decoration: underline;">global-presence-chooser.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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 GlobalPresenceChooser::onUserActivatedComboChange(int index)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">345</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">i18n</span><span class="p">(</span><span class="s">"Do you want to disable this plugin?"</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;">Are you on our mailing list?
We're in a string freeze so this can't go in 0.8.
Why do we want want to show a prompt when you chose something else?</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;">The nowplaying selector was partially broken by changes to the kded module. The prompt toggles activation / deactivation. The plugin requires deactivation of kded module plugin to reset to an empty status message. I'd rather a short generic prompt for enabling, so multiple plugins can reuse the same string. The status message plugin stack has a priority-based approach identical to the status plugin stack and enabling a plugin doesn't guarantee that it's status message will be at the top of the stack. The string freeze might require reverting #114569 to ship a functional contact list. If one doesn't mind changing status to a preset with a blank presence message in order to turn on now playing, it should function fine until more work can be done on the contact list.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 15th, 2014, 1:38 p.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/115425/diff/3/?file=244160#file244160line372" style="color: black; font-weight: bold; text-decoration: underline;">global-presence-chooser.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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 GlobalPresenceChooser::onUserActivatedComboChange(int index)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">360</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QDBusMessage</span> <span class="n">message</span> <span class="o">=</span> <span class="n">QDBusMessage</span><span class="o">::</span><span class="n">createSignal</span><span class="p">(</span><span class="n">QLatin1String</span><span class="p">(</span><span class="s">"/Telepathy"</span><span class="p">),</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></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;">So the summary of this patch is that you're removing the activateNowPlaying signals and instead relying on changing the config and sending a configChanged method.
The slots still exist in the kded so this is very half finished.
It seems like all of these changes are just going round in circles changing things rather than coming up with a understanding of how things are meant to work and coming up with a solid design.
Martin: now playing is your area, can you please comment. There have been several people randomly hacking on this now and things are still broken. Can you please comment and fix this mess.
What is the purpose of the config and the nowPlaying signals? How is it /meant/ to work?</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;">As long as the kded dbus interface isn't used elsewhere it's probably redundant code originally there to trigger the first status update after enabling the nowplaying and essentially provide control over a semi-autonomous process thereafter.
The kconfig method (functionally) duplicates this transparently enough where nowplaying will enable when the config entry is updated and self-terminate when respectively disabled. The slots in the kded don't set the nowplaying config either, so its a fairly overlapping implementation. Can we remove the dbus logic or at least properly set the kconfig from it? We can have kconfig write the current enable status from activatenowplaying / deactivatenowplaying but that still requires kconfig also to a) check whether the plugin is enabled, and b) also to write out enabled status to kconfig, while still using dbus to activate / deactivate the plugin.</pre>
<br />
<p>- James</p>
<br />
<p>On February 15th, 2014, 1:25 p.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 Martin Klapetek.</div>
<div>By James Smith.</div>
<p style="color: grey;"><i>Updated Feb. 15, 2014, 1:25 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
ktp-contact-list
</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;">Enables / disables Now Playing in systemsettings every time it is enabled / disabled in the contact list.
Fixes systemsettings kcm showing nowplaying enabled while the contact list has disabled its functionality.</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>global-presence-chooser.cpp <span style="color: grey">(2047473)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/115425/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>