From f74c72b9652ea4613156cc58b681c9244395cf72 Mon Sep 17 00:00:00 2001 From: Peter Penz Date: Mon, 14 May 2012 17:41:18 +0200 Subject: [PATCH] Fix several bookmark synchronization issues --- src/kitemviews/kstandarditem.cpp | 9 +++++ src/kitemviews/kstandarditemmodel.cpp | 9 +++-- src/kitemviews/kstandarditemmodel.h | 6 ++-- src/panels/places/placesitem.cpp | 10 ++---- src/panels/places/placesitem.h | 3 +- src/panels/places/placesitemmodel.cpp | 48 ++++++++++++++++++++++++++- src/panels/places/placesitemmodel.h | 6 +++- src/panels/places/placespanel.cpp | 31 ++++------------- src/panels/places/placespanel.h | 8 +---- 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 changedRoles; @@ -131,7 +138,9 @@ KStandardItem* KStandardItem::parent() const void KStandardItem::setData(const QHash& values) { + const QHash previous = m_data; m_data = values; + onDataChanged(values, previous); } QHash 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& 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 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(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(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& 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& 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()); 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 #include -class KStandardItem; class KItemListController; class PlacesItemEditDialog; +class PlacesItem; class PlacesItemModel; #ifdef HAVE_NEPOMUK @@ -80,12 +80,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 -- 2.47.3