Before this commit, Dolphin reserved space for the scrollbar spacing
even when no scrollbar is visible resulting in a ugly gap in the view
when:
1. the theme uses QStyle::SH_ScrollView_FrameOnlyAroundContents and
2. the theme has a positive PM_ScrollView_ScrollBarSpacing.
QtCurve can have both while Oxygen have 1 but not 2.
To reproduce the problem with Oxygen style. Replace the
`width += ....` (which returns -2 or 0 for Oxygen) with `width += 2`.
See more info here:
https://github.com/QtCurve/qtcurve-qt4/issues/9#issuecomment-28630517
CCBUG: 306631
FIXED-IN: 4.11.4
REVIEW: 113902
Before this commit, we stored the translated "Places" in
.kde4/share/apps/kfileplaces/bookmarks.xml. This was not intentional -
it only happened because
PlacesItem::updateBookmarkForRole(const QByteArray& role) always stored
the translated text (returned by PlacesItem::text()) in the KBookmark.
This is be fixed by first checking if the text() is equal to the
translation of the text that is already stored in the KBookmark, and
not updating it in that case.
Note that we have to make sure that all "Places"-related use the same
context "KFile System Bookmarks" to make that work.
Thanks to Burkhard Lück and Albert Astals Cid for helping to fix this
problem!
BUG: 319282
FIXED-IN: 4.12.0
REVIEW: 113850
If we detect that the user pressed the back/forward buttons while
hovering the empty space of the view, such that
DolphinView::slotMouseButtonPressed(int, Qt::MouseButtons) will request
that Dolphin navigates back/forward in the history, handling the mouse
press event should stop. This prevents the possible unexpected selection
of items in the new directory.
BUG: 327412
FIXED-IN: 4.11.4
This is a follow-up to commit 0e9f4a3987,
which tries to speed up sorting the items naturally by their name using
the idea that a fast non-natural pre-sorting already sorts the items
mostly correctly and thus reduces the number of expensive natural
comparisons.
This change only makes sense if the view is really sorted by "Name". In
other cases, the pre-sorting will most likely not be useful.
Thanks to Christoph Feck for pointing this out!
Use KStringHandler and QTextLayout to wrap the text (file name)
into the maximum width of the label "name".
Make use of QFontMetrics to calculate a font size aware tooltip size.
BUG: 287983
FIXED-IN: 4.11.3
REVIEW: 113101
Since Dolphin 2.0, we have stored the selected items in a QSet<int>,
which is neither space-efficient nor particularly fast when inserting
many items which are in a consecutive range.
This commit replaces the QSet<int> by a new class "KItemSet", which
stores the items in a sorted list of ranges. For each range, we only
store the first index and the length of the range, so we need a lot
less memory for most common selection patterns, and we also save quite
a few CPU cycles in many situations, because adding an item to the
KItemSet will in many cases not need a memory allocation at all, and
it's particularly easy when inserting sorted items into the KItemSet in
a row.
KItemSet contains a minimal subset of QSet's API which makes it
suitable as a drop-in replacement for our needs. It also has iterators,
such that the items can be iterated through easily, also with foreach.
One advantage of KItemSet compared to QSet<int> is that the items are
always iterated through in ascending order.
REVIEW: 113488
Sort the items in a folder first according to their name, without doing
a natural/locale-aware sorting. This is very fast, but the order of the
items is then already close to the final order in most cases.
The number of expensive natural comparisons required to sort the items
is thus greatly reduced.
In my experiments with a folder with 100,000 items, the time required
to sort the files was reduced by 63% with this patch.
REVIEW: 113485
In KItemListViewLayouter, we have always stored a QRectF for each item,
which is "the area that the item occupies". However, the size of the
QRectF is already stored in the size hint resolver.
Therefore, it is sufficient to store the position of the top left
corner of the QRectF in a QPointF and construct the QRectF on demand.
This patch reduces the memory usage by 16 bytes for each item in the
view:
* a QRectF is 4 doubles -> 32 byes
* a QPointF contains only 2 doubles -> 16 bytes
REVIEW: 113487
This patch actually does two things when collapsing an expanded folder
with expanded children:
(a) It removes all expanded children (which are removed from the model)
from m_expandedDirs.
(b) It remembers the expanded children and restores them if the
top-level folder is re-expanded.
BUG: 304363
FIXED-IN: 4.12.0
REVIEW: 113293
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.
BUG: 326039
FIXED-IN: 4.11.3
REVIEW: 113290
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.
REVIEW: 113233
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.
BUG: 325517
FIXED-IN: 4.11.3
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.
CCBUG: 318683
REVIEW: 113191
FIXED-IN: 4.11.3
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).
BUG: 324371
BUG: 325359
FIXED-IN: 4.12.0
REVIEW: 113070
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!
BUG: 161385
FIXED-IN: 4.11.3
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.
BUG: 324479
FIXED-IN: 4.11.3
REVIEW: 113071
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.
REVIEW: 112725
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.
This fixes the problem that filtered child items in Details View may
reappear when switching the view mode and the clearing the filter.
BUG: 325344
REVIEW: 112962
FIXED-IN: 4.11.3
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.
REVIEW: 112728
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!
BUG: 322093
FIXED-IN: 4.11.2
REVIEW: 112580
Take the style option vertical/horizontal margin into account
for the calculation of the new scroll offset.
Thanks to Frank for pointing out two other problems with "Page Up/Down" and providing
a better way to fix these problems. :)
BUG: 311099
FIXED-IN: 4.11.2
REVIEW: 112678
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 d70a481180. This patch fixes
it.
BUG: 324713
FIXED-IN: 4.11.2
REVIEW: 112561
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).
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.
REVIEW: 112562
Implemented setActive(bool active), isActive() and activated() signal
for DolphinSearchBox - similar to the KUrlNavigator implementation.
BUG: 296970
FIXED-IN: 4.11.2
REVIEW: 112534
This prevents that the GUI freezes if there are many files inside the
directory, or if the access to the directory is slow for some other
reason.
BUG: 318518
REVIEW: 111920
FIXED-IN: 4.12.0
