<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/100158/">http://git.reviewboard.kde.org/r/100158/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On November 20th, 2010, 8:08 a.m., <b>Nikhil Shantanu Marathe</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;">Using indices to select actions is not very future proof. If actions are re-ordered or removed this code would have to be changed or it could have potentially tragic consequences. The action name is usually a unique identifier. Afaik the Playlist actions generated from a PlaylistProvider::playlistActions() do not have names. So perhaps you could assign names to them and use those names to choose the right action. Make sure when you fetch the action by name, the action is non-null.</pre>
</blockquote>
<p>On November 20th, 2010, 11:25 a.m., <b>Dennis Francis</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;">By name of an action, do you mean the text attribute of QAction ?</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;">QObject::setObjectName(), objectName()
Set it to something quite unique and then in the actions list returned you can test for objectName() instead of the text.</pre>
<br />
<p>- Nikhil Shantanu</p>
<br />
<p>On November 20th, 2010, 11:20 a.m., Dennis Francis wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.orgrb/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 Amarok.</div>
<div>By Dennis Francis.</div>
<p style="color: grey;"><i>Updated 2010-11-20 11:20:27</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 fixes the bug 250746 which causes a crash while using 'delete' key to delete multiple playlists.
When multiple playlists are selected and deleted using delete key, only the first one in the selectedIndexes() gets row removed properly.
I used an alternative route to fix the issue, by using actionsFor() method for getting the actions for the selected
indexes(selected playlists) and then triggering the last action of the returned ActionList (The last action was found to be the one corresponds to
row remove operation).
QTreeView is expanded to depth 0. This resolves 'delete key press' version of bug 245646. Context menu version of this bug can
also be resolved in a similar way through another patch.
Further, this patch automatically fixes bug 250750 - "Cancel delete of playlist initialised by 'Delete' key not clean". Now when you press cancel,
none of the playlists are touched.</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;">Tested with different possible selection patterns using "CTRL+click". Delete key press deletes only the selected playlists when confirmed, else none of
them are affected. The PlayListBrowserView maintains the same expanded state after deletion.
Things seems to work fine for me.
</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=250746">250746</a>,
<a href="https://bugs.kde.org/show_bug.cgi?id=250750">250750</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>src/browsers/playlistbrowser/PlaylistBrowserView.cpp <span style="color: grey">(2603e2f)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/100158/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>