Review Request: Fix Bug 294445 - Renaming currently viewed folder in Folders sidebar doesn't update main area

[Frank Reininghaus]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107070/#review21265
-----------------------------------------------------------

Ship it!


Thanks, looks very nice :-) 

Please push to 4.9 (4.9.3 tagging is tomorrow, November 1, AFAIK, would be nice if that fix makes it into the packages).

Just a small note: I always try to keep the order of the methods equal in the .h and .cpp files (such that disconnectRoleEditingSignals() would be after hasSiblingSuccessor(int index)). A few times, I had the impression that it's a bit uncomfortable if the sorting of the methods is different in both files because it took me a few seconds more to find the method, but I'll leave it up to you if you want to invest a minute more to make that change - it's not a big issue.

About the root cause of bug 294445: I'll try to come up with a draft for a unit test for KDirLister that documents the issue and then ask David if this can be fixed in KDirLister or if we are really doing something wrong in Dolphin's code.

Thanks again for the good investigation, this multiple connection issue was really not easy to find!

- Frank Reininghaus


On Oct. 31, 2012, 7:29 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107070/
> -----------------------------------------------------------
> 
> (Updated Oct. 31, 2012, 7:29 p.m.)
> 
> 
> Review request for Dolphin and Frank Reininghaus.
> 
> 
> Description
> -------
> 
> Fix Bug 294445 - Renaming currently viewed folder in Folders sidebar doesn't update main area
> 
> Especially the error, while renaming a folder in the folderview panel. (Error-dialog appears when you rename the same folder several times)
> 
> 
> Question:
> 
> void KItemListView::editRole(int index, const QByteArray& role)
> {
>     KItemListWidget* widget = m_visibleItems.value(index);
>     if (!widget || m_editingRole) {
>         return;
>     }
> 
>     m_editingRole = true;
>     widget->setEditedRole(role);
> 
>     connect(widget, SIGNAL(roleEditingCanceled(int,QByteArray,QVariant)),
>             this, SLOT(slotRoleEditingCanceled(int,QByteArray,QVariant)));
>     connect(widget, SIGNAL(roleEditingFinished(int,QByteArray,QVariant)),
>             this, SLOT(slotRoleEditingFinished(int,QByteArray,QVariant)));
> }
> 
> Should we disconnect the signals roleEditingCanceled and roleEditingFinished from KItemListWidget* widget, before we connect the signals? When you rename the same folder/file several times, the slots are called multiple times ...
> 
> 1. Rename -> 1 Slot call
> 2. Rename -> 2 Slot calls
> 3. Rename -> 3 Slot calls 
> ...
> 
> Solution:
> 
> widget->disconnect(SIGNAL(roleEditingCanceled(int,QByteArray,QVariant), this);
> widget->disconnect(SIGNAL(roleEditingFinished(int,QByteArray,QVariant), this);
> 
> 
> This addresses bugs 294445 and 309338.
>     http://bugs.kde.org/show_bug.cgi?id=294445
>     http://bugs.kde.org/show_bug.cgi?id=309338
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kitemlistview.h 5723b9a 
>   dolphin/src/kitemviews/kitemlistview.cpp 72b3fd8 
> 
> Diff: http://git.reviewboard.kde.org/r/107070/diff/
> 
> 
> Testing
> -------
> 
> Renaming a folder in the folderview-panel works without problems. 
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20121031/718706bb/attachment.htm>