From abfcfb5af00987252ea9151fbe3be8ed44d1c37b Mon Sep 17 00:00:00 2001 From: "Vladimir Golovnev (Glassez)" Date: Mon, 24 Apr 2017 21:05:27 +0300 Subject: [PATCH 1/4] Fix possible null pointer dereference --- src/gui/rss/rsswidget.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gui/rss/rsswidget.cpp b/src/gui/rss/rsswidget.cpp index 6ffdb33fd..05fab9438 100644 --- a/src/gui/rss/rsswidget.cpp +++ b/src/gui/rss/rsswidget.cpp @@ -224,7 +224,7 @@ void RSSWidget::askNewFolder() destItem = destItem->parent(); } // Consider the case where the user clicked on Unread item - RSS::Folder *rssDestFolder = ((destItem == m_feedListWidget->stickyUnreadItem()) + RSS::Folder *rssDestFolder = ((!destItem || (destItem == m_feedListWidget->stickyUnreadItem())) ? RSS::Session::instance()->rootFolder() : qobject_cast(m_feedListWidget->getRSSItem(destItem))); @@ -234,7 +234,7 @@ void RSSWidget::askNewFolder() QMessageBox::warning(this, "qBittorrent", error, QMessageBox::Ok); // Expand destination folder to display new feed - if (destItem != m_feedListWidget->stickyUnreadItem()) + if (destItem && (destItem != m_feedListWidget->stickyUnreadItem())) destItem->setExpanded(true); // As new RSS items are added synchronously, we can do the following here. m_feedListWidget->setCurrentItem(m_feedListWidget->mapRSSItem(RSS::Session::instance()->itemByPath(newFolderPath))); From 71814885aa46adbd27bc34380c85714f3a587234 Mon Sep 17 00:00:00 2001 From: "Vladimir Golovnev (Glassez)" Date: Tue, 25 Apr 2017 13:09:25 +0300 Subject: [PATCH 2/4] Fix ArticleListWidget doesn't clear properly --- src/gui/rss/articlelistwidget.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gui/rss/articlelistwidget.cpp b/src/gui/rss/articlelistwidget.cpp index f102f322a..e31a76a41 100644 --- a/src/gui/rss/articlelistwidget.cpp +++ b/src/gui/rss/articlelistwidget.cpp @@ -55,6 +55,7 @@ void ArticleListWidget::setRSSItem(RSS::Item *rssItem, bool unreadOnly) { // Clear the list first clear(); + m_rssArticleToListItemMapping.clear(); if (m_rssItem) m_rssItem->disconnect(this); From 6764de8ef069b12d45232a8a0f3f9810b2cfe2c2 Mon Sep 17 00:00:00 2001 From: "Vladimir Golovnev (Glassez)" Date: Tue, 25 Apr 2017 13:15:11 +0300 Subject: [PATCH 3/4] Fix ArticleListWidget adds new articles in wrong order --- src/gui/rss/articlelistwidget.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/rss/articlelistwidget.cpp b/src/gui/rss/articlelistwidget.cpp index e31a76a41..76d04bdc6 100644 --- a/src/gui/rss/articlelistwidget.cpp +++ b/src/gui/rss/articlelistwidget.cpp @@ -77,7 +77,7 @@ void ArticleListWidget::setRSSItem(RSS::Item *rssItem, bool unreadOnly) void ArticleListWidget::handleArticleAdded(RSS::Article *rssArticle) { if (!(m_unreadOnly && rssArticle->isRead())) - addItem(createItem(rssArticle)); + insertItem(0, createItem(rssArticle)); } void ArticleListWidget::handleArticleRead(RSS::Article *rssArticle) From a78a1a9c649d88d97dcb6f3dacbaf91325ec71d2 Mon Sep 17 00:00:00 2001 From: "Vladimir Golovnev (Glassez)" Date: Wed, 26 Apr 2017 08:09:09 +0300 Subject: [PATCH 4/4] Add invariant checking in ArticleListWidget --- src/gui/rss/articlelistwidget.cpp | 44 ++++++++++++++++++++++--------- src/gui/rss/articlelistwidget.h | 3 ++- 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/src/gui/rss/articlelistwidget.cpp b/src/gui/rss/articlelistwidget.cpp index 76d04bdc6..bce26f1b4 100644 --- a/src/gui/rss/articlelistwidget.cpp +++ b/src/gui/rss/articlelistwidget.cpp @@ -38,6 +38,8 @@ ArticleListWidget::ArticleListWidget(QWidget *parent) { setContextMenuPolicy(Qt::CustomContextMenu); setSelectionMode(QAbstractItemView::ExtendedSelection); + + checkInvariant(); } RSS::Article *ArticleListWidget::getRSSArticle(QListWidgetItem *item) const @@ -61,23 +63,32 @@ void ArticleListWidget::setRSSItem(RSS::Item *rssItem, bool unreadOnly) m_unreadOnly = unreadOnly; m_rssItem = rssItem; - if (!m_rssItem) return; + if (m_rssItem) { + connect(m_rssItem, &RSS::Item::newArticle, this, &ArticleListWidget::handleArticleAdded); + connect(m_rssItem, &RSS::Item::articleRead, this, &ArticleListWidget::handleArticleRead); + connect(m_rssItem, &RSS::Item::articleAboutToBeRemoved, this, &ArticleListWidget::handleArticleAboutToBeRemoved); - m_rssItem = rssItem; - connect(m_rssItem, &RSS::Item::newArticle, this, &ArticleListWidget::handleArticleAdded); - connect(m_rssItem, &RSS::Item::articleRead, this, &ArticleListWidget::handleArticleRead); - connect(m_rssItem, &RSS::Item::articleAboutToBeRemoved, this, &ArticleListWidget::handleArticleAboutToBeRemoved); - - foreach (auto article, rssItem->articles()) { - if (!(m_unreadOnly && article->isRead())) - addItem(createItem(article)); + foreach (auto article, rssItem->articles()) { + if (!(m_unreadOnly && article->isRead())) { + auto item = createItem(article); + addItem(item); + m_rssArticleToListItemMapping.insert(article, item); + } + } } + + checkInvariant(); } void ArticleListWidget::handleArticleAdded(RSS::Article *rssArticle) { - if (!(m_unreadOnly && rssArticle->isRead())) - insertItem(0, createItem(rssArticle)); + if (!(m_unreadOnly && rssArticle->isRead())) { + auto item = createItem(rssArticle); + insertItem(0, item); + m_rssArticleToListItemMapping.insert(rssArticle, item); + } + + checkInvariant(); } void ArticleListWidget::handleArticleRead(RSS::Article *rssArticle) @@ -90,14 +101,22 @@ void ArticleListWidget::handleArticleRead(RSS::Article *rssArticle) item->setData(Qt::ForegroundRole, QColor("grey")); item->setData(Qt::DecorationRole, QIcon(":/icons/sphere.png")); } + + checkInvariant(); } void ArticleListWidget::handleArticleAboutToBeRemoved(RSS::Article *rssArticle) { delete m_rssArticleToListItemMapping.take(rssArticle); + checkInvariant(); } -QListWidgetItem *ArticleListWidget::createItem(RSS::Article *article) +void ArticleListWidget::checkInvariant() const +{ + Q_ASSERT(count() == m_rssArticleToListItemMapping.count()); +} + +QListWidgetItem *ArticleListWidget::createItem(RSS::Article *article) const { Q_ASSERT(article); QListWidgetItem *item = new QListWidgetItem; @@ -113,6 +132,5 @@ QListWidgetItem *ArticleListWidget::createItem(RSS::Article *article) item->setData(Qt::DecorationRole, QIcon(":/icons/sphere2.png")); } - m_rssArticleToListItemMapping.insert(article, item); return item; } diff --git a/src/gui/rss/articlelistwidget.h b/src/gui/rss/articlelistwidget.h index dec2fb90f..6c35892b0 100644 --- a/src/gui/rss/articlelistwidget.h +++ b/src/gui/rss/articlelistwidget.h @@ -56,7 +56,8 @@ private slots: void handleArticleAboutToBeRemoved(RSS::Article *rssArticle); private: - QListWidgetItem *createItem(RSS::Article *article); + void checkInvariant() const; + QListWidgetItem *createItem(RSS::Article *article) const; RSS::Item *m_rssItem = nullptr; bool m_unreadOnly = false;