| Summary: | KListViewSearchLine can sometimes show items it shouldn't | ||
|---|---|---|---|
| Product: | [Unmaintained] kdelibs | Reporter: | Richard Smith <kde> |
| Component: | general | Assignee: | Scott Wheeler <wheeler> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | ||
| Priority: | NOR | ||
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Compiled Sources | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
SVN commit 433828 by teske:
KListViewSearchLine didn't unhide the right items.
Fix that with a new algorithm.
Thanks to Richard Smith for his help. He reviewed a earlier patch and
ensured that the algorithm is alot more readable.
BUG: 91476
M +38 -5 klistviewsearchline.cpp
M +1 -1 klistviewsearchline.h
--- trunk/KDE/kdelibs/kdeui/klistviewsearchline.cpp #433827:433828
@@ -353,15 +353,48 @@
}
}
-bool KListViewSearchLine::checkItemParentsVisible(QListViewItem *item)
+#include <kdebug.h>
+
+/** Check whether \p item, its siblings and their descendents should be shown. Show or hide the items as necessary.
+ *
+ * \p item The list view item to start showing / hiding items at. Typically, this is the first child of another item, or the
+ * the first child of the list view.
+ * \p highestHiddenParent The highest (closest to root) ancestor of \p item which is hidden. If 0, all parents of
+ * \p item must be visible.
+ * \return \c true if an item which should be visible is found, \c false if all items found should be hidden. If this function
+ * returns true and \p highestHiddenParent was not 0, highestHiddenParent will have been shown.
+ */
+bool KListViewSearchLine::checkItemParentsVisible(QListViewItem *item, QListViewItem *highestHiddenParent)
{
bool visible = false;
- for(; item; item = item->nextSibling()) {
- if((item->firstChild() && checkItemParentsVisible(item->firstChild())) ||
- itemMatches(item, d->search))
+ QListViewItem * first = item;
+ for(; item; item = item->nextSibling())
+ {
+ //What we pass to our children as highestHiddenParent:
+ QListViewItem * hhp = highestHiddenParent ? highestHiddenParent : item->isVisible() ? 0L : item;
+ bool childMatch = false;
+ if(item->firstChild() && checkItemParentsVisible(item->firstChild(), hhp))
+ childMatch = true;
+ // Should this item be shown? It should if any children should be, or if it matches.
+ if(childMatch || itemMatches(item, d->search))
{
- item->setVisible( true );
visible = true;
+ if (highestHiddenParent)
+ {
+ highestHiddenParent->setVisible(true);
+ // Calling setVisible on our ancestor will unhide all its descendents. Hide the ones
+ // before us that should not be shown.
+ for(QListViewItem *hide = first; hide != item; hide = hide->nextSibling())
+ hide->setVisible(false);
+ highestHiddenParent = 0;
+ // If we matched, than none of our children matched, yet the setVisible() call on our
+ // ancestor unhid them, undo the damage:
+ if(!childMatch)
+ for(QListViewItem *hide = item->firstChild(); hide; hide = hide->nextSibling())
+ hide->setVisible(false);
+ }
+ else
+ item->setVisible(true);
}
else
item->setVisible(false);
--- trunk/KDE/kdelibs/kdeui/klistviewsearchline.h #433827:433828
@@ -190,7 +190,7 @@
* It makes a recursive call to all children. It returns true if at least
* one item in the subtree with the given root item is visible.
*/
- bool checkItemParentsVisible(QListViewItem *item);
+ bool checkItemParentsVisible(QListViewItem *item, QListViewItem *highestHiddenParent = 0);
private slots:
void itemAdded(QListViewItem *item) const;
|
Version: (using KDE Devel) Installed from: Compiled sources Take a nested message thread in KMail. Find a search string which applies to the message at the start of the thread, but none of the rest of the messages in it. Enter that string, and enough random text to hide all messages. Now delete the random text, so that message comes back. All its children appear too. They should not. The reason for this is a bug here: bool KListViewSearchLine::checkItemParentsVisible(QListViewItem *item) { bool visible = false; for(; item; item = item->nextSibling()) { if((item->firstChild() && checkItemParentsVisible(item->firstChild())) || itemMatches(item, d->search)) { item->setVisible( true ); visible = true; } else item->setVisible(false); } return visible; } The trouble is, calling item->setVisible( true ); sets all the children of that item to be visible too. Since the above recursive function shows/hides the children before it processes the parent, the children that should be hidden end up not being hidden. In my case (in Kopete, which has the above code copy-pasted for other reasons), I put another call to checkItemParentsVisible(...) after the setVisible( true ), and I now get the expected results, but this makes the code exponential-time (at least in theory). It can still be done in linear time, but it requires a two-pass approach, first deciding on item visibility recursively, then setting item visibility recursively. I'd fix it myself, but I've got a lot on my plate at the moment. If you need clarification on anything, just ask.