The fix for bug 161385 (which was about Dolphin still showing an empty
view if a device that had been unmounted earlier was clicked in the
Places Panel) caused a regression: the view state (current item, scroll
position, Details View expansion state) was not restored any more when
going "Back".
The reason is that "m_view->reload()" in
DolphinViewContainer::setUrl(const KUrl& newUrl) was always executed
just after entering a directory, and that command overwrites this
information.
Distinguishing between "change URL" and "reload the view" works better
if it's done in DolphinMainWindow instead of DolphinViewContainer.
Two small simplifications in KItemListViewLayouter
1. Remove the unneeded variable rowCount.
2. Simplify the calculation of the member m_maximumScrollOffset. We can
just use the current value of "y" because this is the offset that
the next row would have.
Fix crash when triggereing the "Compare files" action via D-Bus
If the number of selected items is not two, Dolphin disables this
action. However, it is still possible to trigger it via D-Bus, and this
could cause a crash in DolphinMainWindow::compareFiles() because this
function did not test at all if there are really two items selected.
This patch adds such a check and simplifies the code in that function.
Erik Hahn [Sun, 13 Oct 2013 08:44:13 +0000 (10:44 +0200)]
Files passed as arguments: Ignore unsupported files
With this patch, Dolphin ignores all files passed to it that it can't
Also, archives are now opened inside Dolphin so it can be used as an
archive manager at least for local files. If the user tries to open a
remote archive Dolphin still opens it externally; I have observed that
if it receives one as an argument, it will display a pseudo-folder that
contains only said archive. So having it set as the archive handler is
still broken, but in a less annoying way.
Make the code that removes items from KFileItemModel more robust
When we remove items from the model, we called the function
KFileItemModel::removeItems(const KFileItemList&, RemoveItemsBehavior).
This function then looked up the indexes of the items using the hash
m_items. This is wasteful in the situations when the indexes of the
removed items are known in advance (like when an expanded folder is
collapsed in Details View), and it can cause trouble if one item is
contained in the model multiple times (can happen when searching, and a
file both matches the search and is a child of a folder that matches
the search). Even if expanding folders in the search results list might
not be particularly useful most of the time, it makes sense to make the
model more robust to prevent crashes and other unexpected behavior in
such situations.
This patch makes the following changes to achieve that goal:
* Change the argument of removeItems() from KFileItemList to
KItemRangeList. To make this work, the "look the indexes up in
m_items" code is moved from that function to slotItemsDeleted(). In
the other places where removeItems() is called, the indexes are
calculated directly (which is not more difficult than determining the
removed items as a KFileItemList, if one considers that we needed the
function childItems(KFileItem) for that, which is not needed any more
with this patch).
* Also removeFilteredChildren() takes a KItemRangeList now. Rather than
putting the parent KFileItems into a QSet for O(1) lookup (which
prevents O(N^2) worst case behavior for the entire function), it uses
a QSet<ItemData*> now, which should even be more efficient (hashing a
pointer is cheaper than hashing a KFileItem/KUrl).
Reload the view if a previously unmounted device is mounted again
The problem was that DolphinViewContainer::setUrl(newUrl) was ignored
if newUrl is equal to the URL which is shown in the view already.
The new approach is to reload the view in that method if it is empty, to
make sure that we do not miss that a previously unmounted device has
been re-mounted.
Thanks to Grigoriadis Grigoris for analyzing the root cause of this
issue!
Before this commit, we only added pressed keys to the search string if
they have no other meaning. This means that files containing a Space in
their name could not be searched because Ctrl+Space toggles the
selection state of the current item, and Space alone selects the
current item.
After this commit, Space is added to the search string if
(a) the key press did not have any other effect, i.e., if Ctrl was not
pressed, and the current item is selected already, and
(b) a keyboard search has been started already (to prevent unexpected
effects when pressing Space accidentally - I think that it's rather
uncommon to have files whose names start with a Space - and to make
the unit test simpler).
I modified the unit test of KItemListController, which did not test
keyboard search yet. This uncovered a small problem in
KItemListController::slotChangeCurrentItem() when NoSelection mode is
used. It's not really relevant for anything that is executed inside
Dolphin, but I still fixed it to make the unit test happy.
Save memory and time in KFileItemModel by lazy-loading the "ItemData"
To reduce unnecessary memory comsumption and CPU usage, we only fill the
QHash<QByteArray, QVariant> if the methods data(int) or setData(int) are
called for the corresponding index, or the data is necessary for sorting
the model.
According to my tests, this patch reduces the memory usage when loading
a folder with 100,000 items by 17% in Icons View, and by 26% in Details
View.
Add unit test for the calculation of "name" groups with expanded items
This prevents a possible regression that would have happened with the
first version of https://git.reviewboard.kde.org/r/112725/
The problem was that isChildItem(int index) would return "false"
incorrectly when the QHash for that item was not initialized yet. The
grouping code would then try to read the "text" from the empty QHash,
which yielded an empty QString, and then accessing the first character
of that string caused a crash.
Also factor out the code that transforms a sorted list of ints to a
KItemRangeList. This removes some duplicated code from KFileItemModel.
Note that overriding operator<<() in KItemRangeList was necessary
because it's not a typedef for QList<KItemRange>, but a class derived
from that now, and some code fails to compile if the return type of
that function is QList<KItemRange> and not KItemRangeList.
KFileItemListView notifies KFileItemModelRolesUpdater of changes of the
visible index range and the icon size with a delay, to prevent that
expensive operations are triggered repeatedly, and that scrolling feels
sluggish because the GUI thread is blocked by icon loading.
This patch ensures that the "long" delay of 300 ms is only used when
the zoom level is changed, and the "short" delay if only the visible
index range has changed.
Thanks to Christoph Feck for helping to analyze this problem!
Always sort items correctly when the refreshItems() signal is received
When sorting by, e.g., "Size", and the name is used as a fallback
because there are multiple files with the same size, the refreshItems
signal that is received when a file's name is changed either with the
dialog or outside the current view did not cause the view to be resorted
after commit d70a4811807776966c3241a72121242f4d1eaee8. This patch fixes
it.
The most recent commit from the KDE/4.11 branch (new unit test) had to
be modified slightly due to the changed signal emission when resorting
the model changes only the groups, and not the order of the items
(groupsChaged instead of itemsMoved).
Make expandedParentsCount() work without accessing the data hash
The idea is that we no longer assume that the "expandedParentsCount"
for each item will be stored in the QHash. It is only accessed for
items which are expanded, and which are not top-level items (i.e.,
which have an expandedParentsCount > 1).
Some unit tests are added to improve the coverage of the affected code.
Try to avoid calling the model's data method if only "text" is needed
Currently, KStandardItemListWidgetInformant::itemSizeHint() calls the
model's data(int) method for every single item, but the full data is
actually only needed for the size calculation in Compact View. In
Details View, no data is needed at all to determine the size required
for the item, and in Icons View, only the name is needed.
This patch makes it possible for subclasses of
KStandardItemListWidgetInformant to provide an alternative way to
obtain the "text", and implements this in the subclass
KFileItemListWidgetInformant.
The final goal is to achieve that the QHash which contains all data
for a file item is only created if it is really needed, e.g., because
the view needs access to the data for displaying the item on the
screen.
Replaced all KNewFileMenu usages in DolphinPart by DolphinNewFileMenu.
Removed all signal-slot-connections related to DolphinNewFileMenu->errorMessage(QString)
in DolphinMainWindow and DolphinContextMenu and replaced it by a better solution.
Now we make use of the already existing DolphinNewFileMenuObserver singleton class to achieve a better
error handling, because every newly created DolphinContextMenu instance registers himself by DolphinNewFileMenuObserver
and we use this to connect the errorMessage(QString) signal of every DolphinContextMenu instance to the errorMessage(QString)
signal of the DolphinNewFileMenuObserver singleton class.
So we need only one connection from DolphinNewFileMenuObserver to DolphinMainWindow (or to DolphinPart) to
collect all error messages thrown by every DolphinNewFileMenu instance.
Fix filename trucation issues in Icons View with maximum number of lines
When the name of a file is too long to be shown inside the maximum
number of lines, the last line is elided. However, there were several
problems before this commit:
(a) "lastTextLine", which contains the text to be elided, was not
assigned the complete remaining text, but only the part that would
be put into the last line if there were more lines following. This
may be less than what would fit into the line because we try to not
break the text at random points.
(b) QFontMetrics::elidedText() was not given the width that is available
for the last line (that would be maxWidth), but only the width that
would be occupied by the text if there were more lines following
(line.naturalTextWidth()).
(c) The variable "nameWidth", which is required to calculate the QRectF
that is reserved for the name, was not updated correctly.
The result is that the text was sometimes trucated too early (especially
if there would be a line break early in the text if we had more lines
available), that there may be insufficient space to show the "...", and
that the hover/selection rectangle might be too narrow.
KFileItem::determineMimeType() not only determines the mime type, but
also the icon. For folders, it looks for a .directory file inside the
folder, where a custom icon might be stored. This can take quite a bit
of time and cause the problem that some folder's type still appears to
be "unknown" when the view is shown.
We can work around this problem by caching the folder mime type in a
static QString and applying to to all folders, which can be identified
easily with KFileItem::isDir(),
Make use of the "resort all items timer" in KFileItemModel::slotRefreshItems
to avoid too much expensive resorting calls, in case of many refresh items signals.
Fabio D'Urso [Thu, 15 Aug 2013 17:04:16 +0000 (19:04 +0200)]
dolphin: Show full KFileItem statusbar text with hovered folders too
This patch changes the status bar text dolphin shows when a folder is
hovered. It now shows full folder info, eg:
"mydir (folder, symlink to /opt/mydir")
instead of:
"mydir"
This is consistent with dolphin's behavior on regular files, and
DolphinPart already does that (DolphinPart::slotRequestItemInfo has no
special cases for folders).
Fabio D'Urso [Thu, 15 Aug 2013 16:56:48 +0000 (18:56 +0200)]
dolphin: Show full file info in statusbar when only one file is selected
The purpose of this change is to give the user a chance to see hover
file information if it doesn't fit in the status bar, by allowing to
click on the file and hover on the status bar.
As it's now possible to have status bar texts starting with "<qt>",
DolphinPart::updateStatusBar() must escape strings. Otherwise,
filenames such as "<qt>Tes<font color=red>t" would be rendered as HTML
data in konqueror's status bar when selected.
Make sure that the sort order is correct after renaming
KFileItemModel::setData() should not only cause a resorting when the
sort role is changed. The name is always used as a fallback if the sort
role of multiple files is equal, therefore, renaming a file can change
the correct order of the files even if the files are not sorted by
"name".
Do not store default values in QHash<QByteArray, QVariant>
Storing values which are equivalent to default-constructed QVariants
does not make much sense because QHash::value returns the same value
even if the corresponding key is not found in the hash.
This commit reduces Dolphin's memory consumption in large folders by
up to 7.3% (tested a folder with 100,000 files in Details View) and
reduces the time required for loading a folder.
Fix slow scrolling when hidden files or symbolic links are shown
The problem was that we drawed the overlays using KIconLoader, which can
be very slow, every time an item appeared on the screen. This commit
makes sure that not only the icon, but the icon including overlays is
cached in QPixmapCache. Therefore, the overlay drawing is done just once
for each icon+overlays combination.
For previews, the overlay drawing is done in KFileItemModelRolesUpdater
just after the preview is received.
The problem was that items are removed from m_visibleGroups while
a QMutableHashIterator iterates over this hash, such that the iterator
can become invalid. The solution is to use a QHashIterator instead,
which takes a copy of the hash. Therefore, it is not affected if
m_visibleGroups is modified in any way.
Sometimes when items are renamed, the order of the items in the
directory is not affected, but the groups still change (simple example:
with files a, b, c, e, rename "c" to "d"). At the moment, we always emit
the itemsMoved signal in such a case to make sure that the view is
updated. However, it would be preferable if this signal was not emitted
because it can trigger some quite expensive operations which are not
needed at all.
This commit introduces a new signal groupsChanged and modifies
KFileItemModel and KItemListView such that these classes make use of it.
Some unit tests for the new functionality are included as well.
Fix possible out of range error inKItemListSizeHintResolver::itemsMoved
The function assumes implicitly that the moved range always starts with
the index 0. This is indeed the case at the moment, but it might make
sense to change that in the future. This commit prevents that we get an
out of range problem then.
Thanks to Emmanuel Pescosta for finding this problem, see
https://git.reviewboard.kde.org/r/111808/
Do not allow that panels are dragged out of the main window
This feature might have been useful in some cases, but it's most likely
not useful enough to justify that we accept the bugs that it causes.
Even though some of these bugs are not Dolphin's fault at all, they
cause serious problems for users in some cases.
I'm pushing this commit to master only because I prefer to not push such
behavior changes to 4.11 at this point of the release cycle.
Don't let HTML-like filenames be interpreted as HTML strings
So that filenames that look like HTML don't get fancy-formatted when
we show info about them (i.e. on hover)
This patch fixes the same issue in two places:
- dolphin, by setting Qt::PlainText on the status bar's label
- konqueror, by escaping setStatusBarText strings emitted by
DolphinPart
Fix maximum value for scroll bar when deleting items in Details View
The problem was that the view heigt minus the header height was
subtracted from maximumScrollOffset() to determine the maximum value
of the scroll offset of the top of the view.
However, the top of the view is the part that is hiden behind the
header. Therefore, the full view height must be subtracted from
maximumScrollOffset.
The remaining bits of bug 319951 were fixed by other recent commits.
Thanks to Emmanuel Pescosta for helping to track down the problem!
Prevent that removing items can cause icons to overlap
When items are removed, new items may become visible because of that.
This includes
(a) Items *behind* the removed range. KItemListView may try to create
their widgets at their "imaginary" old positions and move them to
the new position with an animation.
(b) Items *before* the removed range, if the deletion causes the view
to scroll up. In that case, the "imaginary" old position and the new
position was equal, but KItemListView still tried to determine the
"old" position by adding the number of removed items to the index.
The result was that the widgets were created at completely wrong
positions, and no animation was started to fix this.
Thanks to Emmanuel for helping to find the cause of this bug!
Make sure that widgets are initialized when changing the view mode
The problem was that DolphinItemListView overrides the virtual function
onItemLayoutChanged() without calling the base class implementation.
Therefore, KStandardItemListView::updateLayoutOfVisibleItems(), which
calls initializeItemListWidget(), is never called.
This patch refactors the "change item layout"/"supports item expanding"
code a bit to make it more robust and fix the problem that the view
looks "messed up" when switching from Details View without expandable
folders to Icons View.
I'm only pushing this patch to master (going to be KDE 4.12).
The patch is a bit too intrusive for the KDE/4.11 branch for my taste
at this point of the release cycle, and the bug is not a real
showstopper. If it works well in master, one could consider backporting
it to a 4.11.x bug fix release.
Thanks to Emmanuel Pescosta for helping to analyze this issue.
Do not try to smooth-scroll past the end of the view
KItemListSmoothScroller::scrollTo(qreal position) did not check if
'position' is a valid value. Even if the view is scrolled to the bottom
already, it tried to scroll further and activated "smooth scrolling"
when the mouse wheel is used. Because it never got out of the "smooth
scrolling" state then, it got confused when changing the directory, and
restoring the correct scroll offset could fail.
Fix "truncated header" in Details View with non-Oxygen styles
The code for painting the "empty header" was inconsistent with the
headers of the other columns, which is probably the reason why the other
styles got confused
a) No QStyleOptionHeader is used
b) Even if an empty header must be drawn, the last column is drawn with
the option QStyleOptionHeader::End.
According to Christoph, it still doesn't work with the Skulpture style,
but it seems that the patch does at least not make things worse.
Overwrite the changed role value with an empty QVariant,
because the nepomuk roles provider doesn't overwrite it when the property
value list is empty.
Load unknown icons for items just before showing items in the view
Rather than loading many icons (without full mime type determination)
in advance, we make sure that an item has an icon just before it is
shown in the view. This makes sure that no "unknown" icons are shown
unnecessarily, and saves some resources.
Fix O(N^2) complexity issue in KItemListView::slotItemsRemoved()
If many item ranges are removed, KItemListView::slotItemsRemoved()
could take very long because it looped over all items after the first
removed one for every removed range, even if most of these items are
not visible at all.
This commit improves this by just looping over the visible items (whose
number is limited by the window size) for each range.
Make sure that KItemListSizeHintResolver is always consistent
This was the root cause of bug 317827. The assert tried to make sure
that we never access KItemListSizeHintResolver from
KItemListViewLayouter inside the loop over the item ranges. This would
be dangerous because it might be in an inconsistent state - the removed
item ranges were removed step by step, so accessing the item size hints
before the operation was finished could lead to wrong results.
The solution is to insert/remove all item ranges immediately. A nice
side effect is that there are no sources of O(N^2) complexity in
KItemListSizeHintResolver any more if many item ranges are
inserted/removed.
Fix crash when dropping URLs on the URL navigator's drop down menus
The problem was that the files were copied/moved inside the nested event
loop of the drag, which caused problems if the "File exists" dialog was
shown.
The solution is to make the copy/move operation delayed, such that it is
executed in the main event loop.
Note that dropping files on these menus does apparently not work at the
moment when using the Oxygen style (see bug 310016).
Replace QList by QVector if the elements are larger than a pointer
If the elements are larger than a pointer, QList does not store the
elements themselves, but pointers to them in a contiguous block of
memory. This wastes quite a bit of memory. This can be prevented easily
by using QVector instead.
Since m_pendingItemsToInsert is a list of pointers now (and not a list
of KFileItems, as in the 4.10 branch), we have to delete all pointers
when clearing or destroying the model.
I think that no review request is necessary for this small and obvious
change.
Re-enable expandable folders for network top level folders (remote:/)
Added a hash table for target url to url mapping. So when the dir lister
sends us the target url as directory url, we can use the url mapping table
to get the right "Dolphin internal" directory url, which is the non-target url.
Fix the "dropped files are not selected" regression that has
recently been brought up and that have been caused by review 107351 / commit fd65a97b0787b23246c9392fdc34173fb604c9ca
Fix performance regression when loading folders in Details View
When using Details View, only insert all pending items immediately if
new items are inserted which might be children of a pending item. Fixes
the problem that inserting the items in multiple bunches slows down the
folder loading.
Note that the cause of the slowness when inserting in multiple bunches
is that KItemListSizeHintResolver needs O(N^2) time in the worst case
for inserting N items into a model with N existing ones.
projects / dolphin.git / log