Summary: | Crash when renaming folder to hidden if it has subfolders, expanded in panel | ||
---|---|---|---|
Product: | [Applications] dolphin | Reporter: | Georgios Varisteas <yorgos.v> |
Component: | panels: folders | Assignee: | Dolphin Bug Assignee <dolphin-bugs-null> |
Status: | RESOLVED FIXED | ||
Severity: | crash | Keywords: | investigated |
Priority: | NOR | ||
Version: | 2.1 | ||
Target Milestone: | --- | ||
Platform: | Gentoo Packages | ||
OS: | Linux | ||
Latest Commit: | http://commits.kde.org/kde-baseapps/b80384caae111e5c9145ada293922acafe5fa085 | Version Fixed In: | 4.11 |
Sentry Crash Report: |
Description
Georgios Varisteas
2012-12-19 18:45:52 UTC
Thanks for the report and for the precise steps, I could reproduce the crash. Might be related to bug 294616, bug 305283. After debugging this for a while, I think that I see what the problem is: Normally, when a directory is deleted (or becomes invisible because it is renamed to something with a dot at the beginning), KFileItemModel receives the itemsDeleted signal from KDirLister, tries to find all children of that directory, and removes everything from the model (and hence, also from the view). However, in the described use case, the following happens: 1. KDirLister tells us via its refreshItems signal that the name of "tmp_s1/tmp_s2" has changed to ".tmp_s1/tmp_s2". It does not tell us about the name change of "tmp_s1" itself, though, because... 2. KDirLister then tells us that "tmp_s1" has been deleted because it is invisible now. KFileItemModel tries to find its children, but cannot find any because technically, "tmp_s1" does not look like the parent of ".tmp_s1/tmp_s2". (To be precise: KFileItemModel doesn't look at the names at all to determine the children, it just takes all items *after* the removed one which have a greater expansion level as children, but because of the renaming, ".tmp_s1/tmp_s2" now appears *before* the removed "tmp_s1".) Now we have an item which has no valid parent -> trying to dereference the dangling "parent" causes a crash. I see two possibilities to fix this: a) Prevent that KDirLister emits the refreshItems signal for an item whose parent is reported as "deleted". b) Move the entire "filter hidden files" logic into KFileItemModel. The second one is not straightforward to implement, not sure about the first one. Just for the record: the reason why this only happens when renaming via the Folders Panel is that DolphinView::slotRoleEditingFinished changes the URL of the renamed item inside the model *before* actually renaming the directory. Therefore, the 'itemsDeleted' signal for the old name does not have any effect there. Hi Frank, I'm not sure what you mean by "renaming via the Folders Panel" but in my case I rename inline using the main part of dolphin. Never actually tried renaming through the panel. However, dolphin crashes only when the folder panel is visible and the subtree expanded (parentless children as you explained why in a previous post). cheers Git commit b80384caae111e5c9145ada293922acafe5fa085 by Frank Reininghaus. Committed on 10/02/2013 at 18:09. Pushed by freininghaus into branch 'master'. Re-organize the code that compares expanded items The previous approach, which was based on comparing the URLs as strings, was not only very complex, but also could lead to inconsistencies in the model, namely, that not all children were removed from the model when the dir lister reported the parent as deleted. Later on, this could even lead to a crash. FIXED-IN: 4.11 REVIEW: 108766 M +29 -87 dolphin/src/kitemviews/kfileitemmodel.cpp M +0 -16 dolphin/src/kitemviews/kfileitemmodel.h M +52 -0 dolphin/src/tests/kfileitemmodeltest.cpp http://commits.kde.org/kde-baseapps/b80384caae111e5c9145ada293922acafe5fa085 I've found a way to fix the problem. Unfortunately, it required rewriting much of the code that determines in which order the expanded children of a folder appear in the view. The new code is much simpler and also faster, but it's a very large change, and I think that it is therefore not suitable for a bug fix release in the KDE 4.10 series. We have seen much smaller patches cause ugly regressions, so I feel that it's better that this new code gets thorough testing. Therefore, the fix is only in the master branch, which will become KDE 4.11. Thanks again for your help and the detailed and precise steps to reproduce the crash. Excellent bug reports like this one really make our lives easier :-) |