Another fix for KItemListControllerTest::testMouseClickActivation()
One one machine, I see that changing the global "single click" setting
fails. I don't know why that is the case, but I think we should better
just skip the test in that case and not hang forever.
Prevent repeated re-layouting of all items while previews are generated
There was some code in KStandardItemListView::itemSizeHintUpdateRequired
already that was supposed to prevent an expensive re-layouting of all
items when a preview is received. However, it didn't quite work as
intended because also the "iconOverlays" role changed.
The new approach is to only re-layout if text of a visible role changes,
because this is the only way how the space needed by an item might
change (see KStandardItemListWidgetInformant::itemSizeHint()).
Improve the recently added test testMouseClickActivation
The method to make sure that the first item is visible turned out to be
less reliable than I thought. This could make the test hang forever.
Moreover, this commit removes some trailing whitespace that had been
added accidentally.
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.
Two small optimizations in KFileItemModel::removeItems()
1. It seems that it really can happen that KFileItems that we get from
the dir lister's itemsDeleted signal are not in the model any more,
e.g., if a folder where hidden files are shown is left and a folder
where hidden files are not shown is entered. There is no need to
output warnings then.
2. Remove the emptiness-check for the KFileItemList at the beginning.
Even in the unlikely event that we do get an empty list, we return
just a few lines later in the code.
a) A logic error in the code that removes the ItemData pointers from
m_itemData that could cause crashes if multiple item ranges are
removed, and there were un-removed items behind the last one.
b) The implicit assumption that any call of removeItems() will actually
result in items being removed in the model. This is incorrect if
the model is first cleared and then the hidden-files setting is
modified, which happens if "Save view properties for each folder" is
enabled, and a folder where hidden files are shown is left. In that
case, the dir lister emits itemsDeleted for the hidden items after
they have been removed from the model due to the folder change.
I'll add a unit test covering these issues soon.
Many thanks to Romário Rios and Hrvoje Senjan for testing!
Performance improvements in KFileItemModel::removeItems()
The performance of this method is improved by:
a) Not removing items one by one, but doing it in a way that minimizes
the number of moves to prevent O(N^2) worst-case complexity.
b) Not sorting the removed items using the potentially extremely slow
KFileItemModel::lessThan. We can get the indexes of the removed items
very easily from the hash m_items, and sorting ints is a lot faster.
c) Preventing repeated rehashing of m_items when removing the deleted
URLs by replacing remove() by erase().
The benchmark executable must be run manually. It is not run
automatically with the other unit tests to prevent waste of CPU cycles
in the not-so-uncommon situation that only test failures attract
attention.
Vishesh Handa [Tue, 22 Jan 2013 11:48:48 +0000 (17:18 +0530)]
KFileItemModelRolesUpdater: Avoid multiple updates for Nepomuk data
The Nepomuk ResourceWatcher emits 3 signals - propertyChanged,
propertyAdded and propertyRemoved. We should only listen to either the
propertyChanged signal or the propertyAdded + Removed signals. There is
no point in listening to all 3 signals. That will just result in
unnecessary updates.
Additionally, we do not need to listen to the resourceCreated signal.
That is only emitted when we are watching for a specific types, which we
are not.
Make the "A folder cannot be dropped into itself" message less intrusive
Showing this message in the KMessageWidget above the view, which means
that the view contents are moved down, can be extremely annoying
according to user feedback. Just showing the message in the status bar
is probably enough.
Remove incorrect comments about the sorting functions
In fact, we could use the sorting functions provided by Qt or the STL.
The reason why we have our own is that we want to support parallel
sorting because sorting many items naturally by name can be expensive.
Change the sort and merge functions to a more generic form.
This might make it easier to reuse the parallel sorting code. Moreover,
some the upperBound/lowerBound functions have been removed because
equivalents are provided by the STL.
Simeon Bird [Sat, 12 Jan 2013 15:37:26 +0000 (10:37 -0500)]
We don't need the mutex guarding m_itemStates in the
UpdateItemStatesThread, because m_itemStates is only accessed by the
when the thread is done, and set before the thread starts.
Also combine the setData function with the constructor.
Simeon Bird [Sun, 13 Jan 2013 18:49:21 +0000 (13:49 -0500)]
A crash occurs if updateItemStates runs between the
UpdateItemStatesThread finishing and the finished() signal being
delivered.
In this case, a new thread was not created, because the old thread
still existed. However, pendingItemStatesUpdate was not set, because the
thread was not running. Instead, the old thread was restarted.
This meant that the finished() signal from the first run could be delivered
while the thread was running for a second time, causing the thread to be
deleted while still running and thus a crash.
Solution: set pendingItemStatesUpdate if the thread is non-null,
even if it is not running, knowing that slotThreadFinished has not yet run,
and will call updateItemStates itself.
Emmanuel has contributed a lot of useful patches to Dolphin. Thank you
very much for that! The Dolphin community is looking forward to further
contributions from you.
Slightly reduce the tinting for selected icons and previews
The intention of the tinting was to make it more obvious in icons view
which icons are selected. However, some icons and previews look quite
ugly with the current tinting value of 1.0 (i.e., the value passed to
KIconEffect::colorize). A slight reduction of this value to 0.8 makes
this a little less ugly. However, the real fix is to remove the tinting
altogether and find something better to indicate which items are
selected.
The reentrant natural comparison of strings is the only really
expensive operation. Other comparison functions are much cheaper and
might not be reentrant at all. Therefore, we disable parallel sorting
when not sorting by name to prevent crashes and other unpleasant
behaviour.
Kai Uwe Broulik [Wed, 9 Jan 2013 21:10:04 +0000 (22:10 +0100)]
Fix information panel interpreting html tags
Only fixes the most obvious file name, the other entries in the table
probably have to be fixed in the FileMetaDataWidget.
REVIEW: 108307
BUG: 262464
(cherry picked from commit 3d7d900d2830c71255ff8c02f5f7439224cd4122)
Kai Uwe Broulik [Wed, 9 Jan 2013 21:10:04 +0000 (22:10 +0100)]
Fix information panel interpreting html tags
Only fixes the most obvious file name, the other entries in the table
probably have to be fixed in the FileMetaDataWidget.
REVIEW: 108307
BUG: 262464
The string was added in ecb2715ead30c2b9d483748074524322e5ecedcf.
According to the soft message freeze policy on techbase, it's OK to fix
previously untranslated strings. Translators, please tell me if I'm
wrong, then I'll remove the string and the surrounding code and re-add
it to master later for KDE 4.11. Sorry for the trouble.
Thanks to David Faure for pointing out this mistake!
Fix Bug 240820 - [Usability Bug] Handling Bad Filenames, dolphin gives unclear error message
Fix Bug 308597 - Regression: Renaming a file/folder to something that contains a "/" will result in several message boxes
Uses the same solution as Dolphin-Rename-Dialog does. (KIO::encodeFileName)
The problem was that we erased an iterator from the list and then
incremented it. This can lead to problems (namely, random crashes) if
the iterator pointed to the last list element.
Added curly brackets around DolphinApplication creation and execution, to make sure it is deleted before "return 0;" is
called. This might prevent random crashes due to the de-allocation chain in Qt at exit.
CCBUG: 311092
projects / dolphin.git / log