Fix several bookmark synchronization issues

6 files changed, 64 insertions, 42 deletions

diff --git a/src/panels/places/placesitem.cpp b/src/panels/places/placesitem.cpp
index 19b1d9e9a..1fbc12a7d 100644
--- a/src/panels/places/placesitem.cpp
+++ b/src/panels/places/placesitem.cpp
@@ -121,7 +121,7 @@ void PlacesItem::setBookmark(const KBookmark& bookmark)
     const QString udi = bookmark.metaDataItem("UDI");
     if (udi.isEmpty()) {
         setIcon(bookmark.icon());
-        setText(bookmark.text());
+        setText(bookmark.description());
         setUrl(bookmark.url());
         setDataValue("address", bookmark.address());
         setGroup(i18nc("@item", "Places"));
@@ -139,8 +139,7 @@ KBookmark PlacesItem::bookmark() const
 KBookmark PlacesItem::createBookmark(KBookmarkManager* manager,
                                      const QString& text,
                                      const KUrl& url,
-                                     const QString& iconName,
-                                     PlacesItem* after)
+                                     const QString& iconName)
 {
     KBookmarkGroup root = manager->root();
     if (root.isNull()) {
@@ -148,12 +147,9 @@ KBookmark PlacesItem::createBookmark(KBookmarkManager* manager,
     }
 
     KBookmark bookmark = root.addBookmark(text, url, iconName);
+    bookmark.setDescription(text);
     bookmark.setMetaDataItem("ID", generateNewId());
 
-    if (after) {
-        root.moveBookmark(bookmark, after->bookmark());
-    }
-
     return bookmark;
 }
 
diff --git a/src/panels/places/placesitem.h b/src/panels/places/placesitem.h
index c21a8ca9a..fc6a5f078 100644
--- a/src/panels/places/placesitem.h
+++ b/src/panels/places/placesitem.h
@@ -62,8 +62,7 @@ public:
     static KBookmark createBookmark(KBookmarkManager* manager,
                                     const QString& text,
                                     const KUrl& url,
-                                    const QString& iconName,
-                                    PlacesItem* after = 0);
+                                    const QString& iconName);
     static KBookmark createDeviceBookmark(KBookmarkManager* manager,
                                           const QString& udi);
 
diff --git a/src/panels/places/placesitemmodel.cpp b/src/panels/places/placesitemmodel.cpp
index 7bbd65181..681d9d17e 100644
--- a/src/panels/places/placesitemmodel.cpp
+++ b/src/panels/places/placesitemmodel.cpp
@@ -84,6 +84,16 @@ PlacesItemModel::~PlacesItemModel()
     m_hiddenItems.clear();
 }
 
+PlacesItem* PlacesItemModel::createPlacesItem(const QString& text,
+                                              const KUrl& url,
+                                              const QString& iconName)
+{
+    const KBookmark bookmark = PlacesItem::createBookmark(m_bookmarkManager, text, url, iconName);
+    PlacesItem* item = new PlacesItem(bookmark);
+    item->setGroup(groupName(url));
+    return item;
+}
+
 PlacesItem* PlacesItemModel::placesItem(int index) const
 {
     return dynamic_cast<PlacesItem*>(item(index));
@@ -276,6 +286,21 @@ void PlacesItemModel::requestTeardown(int index)
 
 void PlacesItemModel::onItemInserted(int index)
 {
+    const PlacesItem* insertedItem = placesItem(index);
+    if (insertedItem) {
+        // Take care to apply the PlacesItemModel-order of the inserted item
+        // also to the bookmark-manager.
+        const KBookmark insertedBookmark = insertedItem->bookmark();
+
+        const PlacesItem* previousItem = placesItem(index - 1);
+        KBookmark previousBookmark;
+        if (previousItem) {
+            previousBookmark = previousItem->bookmark();
+        }
+
+        m_bookmarkManager->root().moveBookmark(insertedBookmark, previousBookmark);
+    }
+
     if (index == count() - 1) {
         // The item has been appended as last item to the list. In this
         // case assure that it is also appended after the hidden items and
@@ -305,8 +330,14 @@ void PlacesItemModel::onItemInserted(int index)
 #endif
 }
 
-void PlacesItemModel::onItemRemoved(int index)
+void PlacesItemModel::onItemRemoved(int index, KStandardItem* removedItem)
 {
+    PlacesItem* placesItem = dynamic_cast<PlacesItem*>(removedItem);
+    if (placesItem) {
+        const KBookmark bookmark = placesItem->bookmark();
+        m_bookmarkManager->root().deleteBookmark(bookmark);
+    }
+
     const int removeIndex = hiddenIndex(index);
     Q_ASSERT(!m_hiddenItems[removeIndex]);
     m_hiddenItems.removeAt(removeIndex);
@@ -321,6 +352,21 @@ void PlacesItemModel::onItemRemoved(int index)
 
 void PlacesItemModel::onItemChanged(int index, const QSet<QByteArray>& changedRoles)
 {
+    const PlacesItem* changedItem = placesItem(index);
+    if (changedItem) {
+        // Take care to apply the PlacesItemModel-order of the inserted item
+        // also to the bookmark-manager.
+        const KBookmark insertedBookmark = changedItem->bookmark();
+
+        const PlacesItem* previousItem = placesItem(index - 1);
+        KBookmark previousBookmark;
+        if (previousItem) {
+            previousBookmark = previousItem->bookmark();
+        }
+
+        m_bookmarkManager->root().moveBookmark(insertedBookmark, previousBookmark);
+    }
+
     if (changedRoles.contains("isHidden")) {
         const PlacesItem* shownItem = placesItem(index);
         Q_ASSERT(shownItem);
diff --git a/src/panels/places/placesitemmodel.h b/src/panels/places/placesitemmodel.h
index bfc845e41..00bdb1c02 100644
--- a/src/panels/places/placesitemmodel.h
+++ b/src/panels/places/placesitemmodel.h
@@ -52,6 +52,10 @@ public:
     explicit PlacesItemModel(QObject* parent = 0);
     virtual ~PlacesItemModel();
 
+    PlacesItem* createPlacesItem(const QString& text,
+                                 const KUrl& url,
+                                 const QString& iconName);
+
     PlacesItem* placesItem(int index) const;
 
     void setHiddenItemsShown(bool show);
@@ -85,7 +89,7 @@ signals:
 
 protected:
     virtual void onItemInserted(int index);
-    virtual void onItemRemoved(int index);
+    virtual void onItemRemoved(int index, KStandardItem* removedItem);
     virtual void onItemChanged(int index, const QSet<QByteArray>& changedRoles);
 
 private slots:
diff --git a/src/panels/places/placespanel.cpp b/src/panels/places/placespanel.cpp
index 75f765812..4c0162dc4 100644
--- a/src/panels/places/placespanel.cpp
+++ b/src/panels/places/placespanel.cpp
@@ -312,17 +312,17 @@ void PlacesPanel::addEntry()
     dialog->setAllowGlobal(true);
     dialog->setUrl(url);
     if (dialog->exec() == QDialog::Accepted) {
-        KStandardItem* item = createStandardItemFromDialog(dialog);
+        PlacesItem* item = m_model->createPlacesItem(dialog->text(), dialog->url(), dialog->icon());
 
         // Insert the item as last item of the corresponding group.
         int i = 0;
-        while (i < m_model->count() && m_model->item(i)->group() != item->group()) {
+        while (i < m_model->count() && m_model->placesItem(i)->group() != item->group()) {
             ++i;
         }
 
         bool inserted = false;
         while (!inserted && i < m_model->count()) {
-            if (m_model->item(i)->group() != item->group()) {
+            if (m_model->placesItem(i)->group() != item->group()) {
                 m_model->insertItem(i, item);
                 inserted = true;
             }
@@ -348,14 +348,11 @@ void PlacesPanel::editEntry(int index)
     dialog->setUrl(data.value("url").value<KUrl>());
     dialog->setAllowGlobal(true);
     if (dialog->exec() == QDialog::Accepted) {
-        KStandardItem* oldItem = m_model->item(index);
+        PlacesItem* oldItem = m_model->placesItem(index);
         if (oldItem) {
-            KStandardItem* item = createStandardItemFromDialog(dialog);
-            // Although the user might have changed the URL of the item in a way
-            // that another group should be assigned, we still apply the old
-            // group to keep the same position for the item.
-            item->setGroup(oldItem->group());
-            m_model->changeItem(index, item);
+            oldItem->setText(dialog->text());
+            oldItem->setUrl(dialog->url());
+            oldItem->setIcon(dialog->icon());
         }
     }
 
@@ -371,20 +368,6 @@ void PlacesPanel::selectClosestItem()
     selectionManager->setSelected(index);
 }
 
-KStandardItem* PlacesPanel::createStandardItemFromDialog(PlacesItemEditDialog* dialog) const
-{
-    Q_ASSERT(dialog);
-
-    const KUrl newUrl = dialog->url();
-    KStandardItem* item = new KStandardItem();
-    item->setIcon(dialog->icon());
-    item->setText(dialog->text());
-    item->setDataValue("url", newUrl);
-    item->setGroup(m_model->groupName(newUrl));
-
-    return item;
-}
-
 KUrl PlacesPanel::convertedUrl(const KUrl& url)
 {
     KUrl newUrl = url;
diff --git a/src/panels/places/placespanel.h b/src/panels/places/placespanel.h
index 780d3f09f..4156cfea0 100644
--- a/src/panels/places/placespanel.h
+++ b/src/panels/places/placespanel.h
@@ -26,9 +26,9 @@
 #include <KUrl>
 #include <panels/panel.h>
 
-class KStandardItem;
 class KItemListController;
 class PlacesItemEditDialog;
+class PlacesItem;
 class PlacesItemModel;
 
 #ifdef HAVE_NEPOMUK
@@ -81,12 +81,6 @@ private:
     void selectClosestItem();
 
     /**
-     * @return New instance of a KStandardItem containing the properties that have
-     *         been set in the places-dialog.
-     */
-    KStandardItem* createStandardItemFromDialog(PlacesItemEditDialog* dialog) const;
-
-    /**
      * @return Converts the URL, which contains "virtual" URLs for system-items like
      *         "search:/documents" into a Nepomuk-Query-URL that will be handled by
      *         the corresponding IO-slave. Virtual URLs for bookmarks are used to