[patch] Konq bookmarks sidebar bug

[Philip Rodrigues-4]

Hi,
The attached patch, to be applied in
kdebase/apps/konqueror/sidebar/trees/bookmark_module/ ,  seems to fix a
couple of bugs that affect the bookmarks in the sidebar. The problem is that
KonqSidebarBookmarkModule::findByAddress, which converts an address in the
bookmarks tree like "/5/6/7" into a pointer to the appropriate list item was
recursing one time too many and so returning the wrong thing.

Can anyone see any problems with the patch? If not, can I commit it?

Regards,
Philip

[bookmark_module.cpp.diff]

---

Index: bookmark_module.cpp
===================================================================
--- bookmark_module.cpp (revision 823267)
+++ bookmark_module.cpp (working copy)
@@ -538,6 +538,8 @@
     Q3ListViewItem * item = m_topLevelItem;
     // The address is something like /5/10/2
     QStringList addresses = address.split( '/');
+    // Remove the first, empty, entry
+    addresses.removeAt(0);
     for ( QStringList::Iterator it = addresses.begin() ; it != addresses.end() ; ++it )
     {
         uint number = (*it).toUInt();

---

On Monday 23 June 2008, Philip Rodrigues wrote:
> Hi,
> The attached patch, to be applied in
> kdebase/apps/konqueror/sidebar/trees/bookmark_module/ ,  seems to fix a
> couple of bugs that affect the bookmarks in the sidebar. The problem is that
> KonqSidebarBookmarkModule::findByAddress, which converts an address in the
> bookmarks tree like "/5/6/7" into a pointer to the appropriate list item was
> recursing one time too many and so returning the wrong thing.
>
> Can anyone see any problems with the patch? If not, can I commit it?

Indeed, looks like a porting error.
QStringList::split should have been ported to QString::split(, QString::SkipEmptyParts);
This might be safer than accessing [0], btw (e.g. if the address is empty for some reason).

--
David Faure, faure@..., sponsored by Trolltech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).

[Philip Rodrigues-4]

Hi,
 
> > Can anyone see any problems with the patch? If not, can I commit it?
>
> Indeed, looks like a porting error.
> QStringList::split should have been ported to
> QString::split(, QString::SkipEmptyParts);
> This might be safer than accessing [0], btw (e.g. if the
> address is empty for some reason).

Ah yes, Pino mentioned to me that QString::skipEmptyParts might be a better
way to do it - I'll try that, and if it works, I'll commit unless someone
says otherwise.

Regards,
Philip