Fix several bookmark synchronization issues

9 files changed, 83 insertions, 47 deletions

diff --git a/src/kitemviews/kstandarditem.cpp b/src/kitemviews/kstandarditem.cpp
index dc03a0d25..6cb5b049b 100644
--- a/src/kitemviews/kstandarditem.cpp
+++ b/src/kitemviews/kstandarditem.cpp
@@ -103,7 +103,14 @@ QString KStandardItem::group() const
 
 void KStandardItem::setDataValue(const QByteArray& role, const QVariant& value)
 {
+    const QVariant previous = m_data.value(role);
+    if (previous == value) {
+        return;
+    }
+
     m_data.insert(role, value);
+    onDataValueChanged(role, value, previous);
+
     if (m_model) {
         const int index = m_model->index(this);
         QSet<QByteArray> changedRoles;
@@ -131,7 +138,9 @@ KStandardItem* KStandardItem::parent() const
 
 void KStandardItem::setData(const QHash<QByteArray, QVariant>& values)
 {
+    const QHash<QByteArray, QVariant> previous = m_data;
     m_data = values;
+    onDataChanged(values, previous);
 }
 
 QHash<QByteArray, QVariant> KStandardItem::data() const
diff --git a/src/kitemviews/kstandarditemmodel.cpp b/src/kitemviews/kstandarditemmodel.cpp
index fbcda8370..e3d40038d 100644
--- a/src/kitemviews/kstandarditemmodel.cpp
+++ b/src/kitemviews/kstandarditemmodel.cpp
@@ -93,11 +93,13 @@ void KStandardItemModel::removeItem(int index)
         KStandardItem* item = m_items[index];
         m_indexesForItems.remove(item);
         m_items.removeAt(index);
+
+        onItemRemoved(index, item);
+        emit itemsRemoved(KItemRangeList() << KItemRange(index, 1));
+
         delete item;
         item = 0;
 
-        onItemRemoved(index);
-        emit itemsRemoved(KItemRangeList() << KItemRange(index, 1));
         // TODO: no hierarchical items are handled yet
     }
 }
@@ -202,9 +204,10 @@ void KStandardItemModel::onItemChanged(int index, const QSet<QByteArray>& change
     Q_UNUSED(changedRoles);
 }
 
-void KStandardItemModel::onItemRemoved(int index)
+void KStandardItemModel::onItemRemoved(int index, KStandardItem* removedItem)
 {
     Q_UNUSED(index);
+    Q_UNUSED(removedItem);
 }
 
 
diff --git a/src/kitemviews/kstandarditemmodel.h b/src/kitemviews/kstandarditemmodel.h
index be5b7c438..047c1e9b6 100644
--- a/src/kitemviews/kstandarditemmodel.h
+++ b/src/kitemviews/kstandarditemmodel.h
@@ -91,9 +91,11 @@ protected:
 
     /**
      * Is invoked after an item has been removed and before the signal
-     * itemsRemoved() gets emitted.
+     * itemsRemoved() gets emitted. The item \a removedItem has already
+     * been removed from the model and will get deleted after the
+     * execution of onItemRemoved().
      */
-    virtual void onItemRemoved(int index);
+    virtual void onItemRemoved(int index, KStandardItem* removedItem);
 
 private:
     QList<KStandardItem*> m_items;
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