From 58dc22b6ca28710a1f38a21e843f2f83540c482c Mon Sep 17 00:00:00 2001 From: varjolintu Date: Wed, 28 Jun 2023 09:58:50 +0300 Subject: [PATCH 1/2] Add warning for duplicate URLs with Additional URLs list --- share/translations/keepassxc_en.ts | 4 ++++ src/gui/entry/EditEntryWidget.cpp | 5 ++-- src/gui/entry/EntryURLModel.cpp | 37 ++++++++++++++++++++---------- src/gui/entry/EntryURLModel.h | 6 ++++- 4 files changed, 37 insertions(+), 15 deletions(-) diff --git a/share/translations/keepassxc_en.ts b/share/translations/keepassxc_en.ts index 3b5ae5f4a8..79e756fccd 100644 --- a/share/translations/keepassxc_en.ts +++ b/share/translations/keepassxc_en.ts @@ -3919,6 +3919,10 @@ Error: %1 Invalid URL + + Duplicate URL + + EntryView diff --git a/src/gui/entry/EditEntryWidget.cpp b/src/gui/entry/EditEntryWidget.cpp index af28bab680..6e3a606b24 100644 --- a/src/gui/entry/EditEntryWidget.cpp +++ b/src/gui/entry/EditEntryWidget.cpp @@ -326,17 +326,18 @@ void EditEntryWidget::insertURL() { Q_ASSERT(!m_history); - QString name("KP2A_URL"); + QString name(BrowserService::ADDITIONAL_URL); int i = 1; while (m_entryAttributes->keys().contains(name)) { - name = QString("KP2A_URL_%1").arg(i); + name = QString("%1_%2").arg(BrowserService::ADDITIONAL_URL, QString::number(i)); i++; } m_entryAttributes->set(name, tr("")); QModelIndex index = m_additionalURLsDataModel->indexByKey(name); + m_additionalURLsDataModel->setEntryUrl(m_entry->url()); m_browserUi->additionalURLsView->setCurrentIndex(index); m_browserUi->additionalURLsView->edit(index); diff --git a/src/gui/entry/EntryURLModel.cpp b/src/gui/entry/EntryURLModel.cpp index 3222c7d308..d5b79eaad5 100644 --- a/src/gui/entry/EntryURLModel.cpp +++ b/src/gui/entry/EntryURLModel.cpp @@ -1,6 +1,6 @@ /* + * Copyright (C) 2023 KeePassXC Team * Copyright (C) 2012 Felix Geyer - * Copyright (C) 2019 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -18,6 +18,7 @@ #include "EntryURLModel.h" +#include "browser/BrowserService.h" #include "core/EntryAttributes.h" #include "core/Tools.h" #include "gui/Icons.h" @@ -68,13 +69,21 @@ QVariant EntryURLModel::data(const QModelIndex& index, int role) const const auto value = m_entryAttributes->value(key); const auto urlValid = Tools::checkUrlValid(value); - if (role == Qt::BackgroundRole && !urlValid) { + // Check for duplicate URLs in the attribute list + auto customKeys = m_entryAttributes->customKeys(); + customKeys.removeOne(key); + const auto values = m_entryAttributes->values(customKeys); + const auto duplicateUrl = values.contains(value) || value == m_entryUrl; + + if (role == Qt::BackgroundRole && (!urlValid || duplicateUrl)) { StateColorPalette statePalette; return statePalette.color(StateColorPalette::ColorRole::Error); - } else if (role == Qt::DecorationRole && !urlValid) { + } else if (role == Qt::DecorationRole && (!urlValid || duplicateUrl)) { return m_errorIcon; } else if (role == Qt::DisplayRole || role == Qt::EditRole) { return value; + } else if (role == Qt::ToolTipRole && duplicateUrl) { + return tr("Duplicate URL"); } else if (role == Qt::ToolTipRole && !urlValid) { return tr("Invalid URL"); } @@ -82,17 +91,21 @@ QVariant EntryURLModel::data(const QModelIndex& index, int role) const return {}; } +void EntryURLModel::setEntryUrl(const QString& entryUrl) +{ + m_entryUrl = entryUrl; +} + bool EntryURLModel::setData(const QModelIndex& index, const QVariant& value, int role) { if (!index.isValid() || role != Qt::EditRole || value.type() != QVariant::String || value.toString().isEmpty()) { return false; } - const int row = index.row(); - const QString key = m_urls.at(row).first; - const QString oldValue = m_urls.at(row).second; + const auto row = index.row(); + const auto key = m_urls.at(row).first; - if (EntryAttributes::isDefaultAttribute(key) || m_entryAttributes->containsValue(value.toString())) { + if (EntryAttributes::isDefaultAttribute(key)) { return false; } @@ -113,7 +126,7 @@ QModelIndex EntryURLModel::indexByKey(const QString& key) const } if (row == -1) { - return QModelIndex(); + return {}; } else { return index(row, 0); } @@ -122,7 +135,7 @@ QModelIndex EntryURLModel::indexByKey(const QString& key) const QString EntryURLModel::keyByIndex(const QModelIndex& index) const { if (!index.isValid()) { - return QString(); + return {}; } else { return m_urls.at(index.row()).first; } @@ -133,9 +146,9 @@ void EntryURLModel::updateAttributes() clear(); m_urls.clear(); - const QList attributesKeyList = m_entryAttributes->keys(); - for (const QString& key : attributesKeyList) { - if (!EntryAttributes::isDefaultAttribute(key) && key.contains("KP2A_URL")) { + const auto attributesKeyList = m_entryAttributes->keys(); + for (const auto& key : attributesKeyList) { + if (!EntryAttributes::isDefaultAttribute(key) && key.contains(BrowserService::ADDITIONAL_URL)) { const auto value = m_entryAttributes->value(key); m_urls.append(qMakePair(key, value)); diff --git a/src/gui/entry/EntryURLModel.h b/src/gui/entry/EntryURLModel.h index f9ffa48288..2801f9ffe6 100644 --- a/src/gui/entry/EntryURLModel.h +++ b/src/gui/entry/EntryURLModel.h @@ -1,6 +1,6 @@ /* + * Copyright (C) 2023 KeePassXC Team * Copyright (C) 2012 Felix Geyer - * Copyright (C) 2019 KeePassXC Team * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -19,6 +19,7 @@ #ifndef KEEPASSXC_ENTRYURLMODEL_H #define KEEPASSXC_ENTRYURLMODEL_H +#include "core/Entry.h" #include #include @@ -43,8 +44,10 @@ class EntryURLModel : public QStandardItemModel public: explicit EntryURLModel(QObject* parent = nullptr); + void setEntryAttributes(EntryAttributes* entryAttributes); void insertRow(const QString& key, const QString& value); + void setEntryUrl(const QString& entryUrl); bool setData(const QModelIndex& index, const QVariant& value, int role = Qt::EditRole) override; QVariant data(const QModelIndex& index, int role) const override; QModelIndex indexByKey(const QString& key) const; @@ -57,6 +60,7 @@ private slots: QList> m_urls; EntryAttributes* m_entryAttributes; QIcon m_errorIcon; + QString m_entryUrl; }; #endif // KEEPASSXC_ENTRYURLMODEL_H From 5f640c38df21b160f2d317abe638ebd831ac6fdf Mon Sep 17 00:00:00 2001 From: varjolintu Date: Fri, 30 Jun 2023 13:20:44 +0300 Subject: [PATCH 2/2] Cleanup --- src/gui/entry/EntryURLModel.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/gui/entry/EntryURLModel.cpp b/src/gui/entry/EntryURLModel.cpp index d5b79eaad5..d43c8cb16d 100644 --- a/src/gui/entry/EntryURLModel.cpp +++ b/src/gui/entry/EntryURLModel.cpp @@ -54,7 +54,7 @@ void EntryURLModel::setEntryAttributes(EntryAttributes* entryAttributes) endResetModel(); } - +#include QVariant EntryURLModel::data(const QModelIndex& index, int role) const { if (!index.isValid()) { @@ -69,11 +69,10 @@ QVariant EntryURLModel::data(const QModelIndex& index, int role) const const auto value = m_entryAttributes->value(key); const auto urlValid = Tools::checkUrlValid(value); - // Check for duplicate URLs in the attribute list - auto customKeys = m_entryAttributes->customKeys(); - customKeys.removeOne(key); - const auto values = m_entryAttributes->values(customKeys); - const auto duplicateUrl = values.contains(value) || value == m_entryUrl; + // Check for duplicate URLs in the attribute list. Excludes the current key/value from the comparison. + auto customAttributeKeys = m_entryAttributes->customKeys(); + customAttributeKeys.removeOne(key); + const auto duplicateUrl = m_entryAttributes->values(customAttributeKeys).contains(value) || value == m_entryUrl; if (role == Qt::BackgroundRole && (!urlValid || duplicateUrl)) { StateColorPalette statePalette;