<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="http://git.reviewboard.kde.org/r/105059/">http://git.reviewboard.kde.org/r/105059/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 2nd, 2012, 6:10 a.m., <b>Andrea Diamantini</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;">Hi Anton,
sorry for the long waiting about a review here. But I just checked your code and I found it very good. I'm just a bit worried about this "strange" interaction: why should a browser take care of this interaction? The change you requested is "monumental" compared to the "tiny" use case you provided. How many users are using the now playing plasmoid? How many players interact with it (everyone?)? How many browsers interact with it (noone?)? Is all this done just to provide a shortcut to play/stop music? What about other multimedia (non-html5) sources? How can users react to the missing functionality in their management?</pre>
</blockquote>
<p>On June 2nd, 2012, 1:29 p.m., <b>Anton Kreuzkamp</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;">>sorry for the long waiting about a review here.
Thanks for the review ;)
>why should a browser take care of this interaction?
Well, one of the goals of rekonq is good desktop integration, isn't it? Integrating web-mediaplayers into the desktop targets exactly that.
>The change you requested is "monumental" compared to the "tiny" use case you provided.
It adds quite a few new classes, that's right, but they're quite well seperated, so it at least doesn't clutter the code, does it?
>How many users are using the now playing plasmoid?
Well, not many. But the now playing plasmoid is not the only one letting you control media players. E.g. Unity has it integrated more deeply into it's shell and for Plasma it's defenitely to come.
The same way one could control the media-playing with a remote control (e.g. while streaming a film from the internet).
>How many players interact with it (everyone?)?
Yes, nearly everyone
>How many browsers interact with it (noone?)?
Yes, noone, so it would be a special feature of rekonq ;)
>Is all this done just to provide a shortcut to play/stop music?
It's done to integrate webbased mediaplayers as good into the desktop as normal mediaplayers are.
The most relevant feature to me personally (and that's why I implemented it) is to play/pause music, though, that's true.
>What about other multimedia (non-html5) sources?
Technically not possible, as html5 players are the only ones having a standardized API for interaction. But html5 is in the ascending.
>How can users react to the missing functionality in their management?
I don't understand this question, sorry.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Does the task manager (such as the Icon-only task manager) use the mpris interface to display the media controls in the icon hover hint?</pre>
<br />
<p>- Lindsay</p>
<br />
<p>On May 26th, 2012, 8:18 a.m., Anton Kreuzkamp wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for rekonq.</div>
<div>By Anton Kreuzkamp.</div>
<p style="color: grey;"><i>Updated May 26, 2012, 8:18 a.m.</i></p>
<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;">This patch addes a mpris2-compliant dbus-interface to rekonq that allows controling html5-video and -audio elements from e.g. the nowplaying-plasmoid.
This is espacially useful together with youtube's html5-player and the plasmoid PlayControl (which allows to assign global shortcuts to mediaplayer-actions (so you can pause all mediaplayers with the same mediacontrol-keys))
With this patch rekonq aggregates all opened media elements and provides one interface to dbus (providing one interface per media element is not possible with mpris) that contains the data of
and sends commands recieved via dbus to the last active mediaplayer (that was last loaded or changed its playback-status most recently).
The communication to the media element happens via javascript.</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;">Comprehensive tests with different html5 mediaplayers (audio and video) and different mpris-controllers. No problems anymore.</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>src/CMakeLists.txt <span style="color: grey">(1180ad0)</span></li>
<li>src/application.h <span style="color: grey">(36114ae)</span></li>
<li>src/application.cpp <span style="color: grey">(ef6c208)</span></li>
<li>src/dbus/mpris2/Mpris2DBusHandler.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/dbus/mpris2/Mpris2DBusHandler.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/dbus/mpris2/org.mpris.MediaPlayer2.Player.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/dbus/mpris2/org.mpris.MediaPlayer2.root.xml <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/webpage.h <span style="color: grey">(abc9f64)</span></li>
<li>src/webpage.cpp <span style="color: grey">(ce1151d)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105059/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>