Review Request: Wrong icons selected after pasting files and renaming them (because there exists items with it's names)

[Frank Reininghaus]

> On Nov. 17, 2012, 7:50 p.m., Frank Reininghaus wrote:
> > Thanks for the patch! The approach looks good from my point of view, but I'm not sure if we can actually change the return type of functions in lib/konq. This might not be binary compatible. @David: do we guarantee binary compatibility for lib/konq?
> > 
> > About the Dolphin side of the patch: I'm wondering if we could do it without the new member and just append() the URL 'to' to m_selectedUrls instead. This would have the added benefit that the first successfully pasted items are selected already when the dialog is shown.
> 
> Emmanuel Pescosta wrote:
>     > About the Dolphin side of the patch: I'm wondering if we could do it without the new member and just append() the URL 'to' to m_selectedUrls instead.
>     I already tried this approach, but it crashed when I pasted already existing files.
> 
> Frank Reininghaus wrote:
>     Hm, I just tried it myself. What I did was to replace both "m_copiedUrls.append(to)" by "m_selectedUrls.append(to)" in your patch and comment out the code in DolphinView::slotCopyingResult(KJob* job). That does not cause crashes here, no matter if I replace the existing files or if I rename the new files. I am curious to see how you made it crash and what backtrace you get - maybe there is a hidden bug in the existing code that we haven't seen yet?
> 
> Emmanuel Pescosta wrote:
>     > That does not cause crashes here, no matter if I replace the existing files or if I rename the new files.
>     How many files did you use for your test? With 2 files it worked for me too but with 20 files Dolphin always crashed.
>     
>     > maybe there is a hidden bug in the existing code that we haven't seen yet?
>     I will start a debug session in the evening ;)

I haven't seen any crashes yet, even with more files, but maybe I haven't tried hard enough... In any case, I'd like to see the backtrace of such a crash!

One more thing that I didn't notice earlier: the same issue applies to dropping files in the view. Maybe the patch can be improved to also include that case. And we should also make sure that the previous selection gets cleared when items are pasted/dropped, see my related patch https://git.reviewboard.kde.org/r/107389/.


- Frank


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


On Nov. 16, 2012, 11:05 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107351/
> -----------------------------------------------------------
> 
> (Updated Nov. 16, 2012, 11:05 p.m.)
> 
> 
> Review request for Dolphin and Frank Reininghaus.
> 
> 
> Description
> -------
> 
> Bug 233335 - Wrong icons selected after pasting files and renaming them (because there exists items with it's names)
> 
> Dolphin 2.1 selects the existing items instead of the newly pasted + renamed items.
> 
> 
> This addresses bug 233335.
>     http://bugs.kde.org/show_bug.cgi?id=233335
> 
> 
> Diffs
> -----
> 
>   dolphin/src/views/dolphinview.h 1feaf0f 
>   dolphin/src/views/dolphinview.cpp df49634 
>   lib/konq/konq_operations.h b82feb4 
>   lib/konq/konq_operations.cpp e0f1ade 
> 
> Diff: http://git.reviewboard.kde.org/r/107351/diff/
> 
> 
> Testing
> -------
> 
> 1. Open a test folder with Dolphin.
> 2. Create the test files:
>     2.1. Open Dolphin’s terminal (F4).
>     2.2. Run “mkdir folder ; touch a b folder/b c d”.
>     2.3. Close Dolphin’s terminal (F4).
> 3. In the current folder, select the four empty files (a, b, c, d) and copy them (Ctrl+C).
> 4. Enter the “folder” folder.
> 5. Paste the files (Ctrl+V).
>     5.1. When asked for a new name for “b”, enter “b2” and hit Enter to continue.
> 
> Result:
> Items “a”, “b2”, “c”, “d” are selected
> 
> Btw. Thanks to Adrián Chaves Fernández for the easy way to test this bug ;)
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

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