Commit c740a8ed authored by Stefano Crocco's avatar Stefano Crocco

Fix a bug causing being unable to overwrite existing cached data

This bug has been introduced by changing
WebEngineWallet::WebEngineWalletPrivate::saveDataToCache so that it
doesn't delete entries in pendingSaveRequests anymore but leaves this
task to its caller. However, the caller can't know whether the data has
actually been saved or whether the saveFormDataRequested signal has been
emitted because the user needs to decide what to do. As a consequence,
the entry in pendingSaveRequests will always be removed, so that when
the user decides to save it, it won't be there anymore.

To avoid this issue, saveDataToCache now returns a bool telling whether
the data has been saved (and the entry must be remvoed) or not.
parent 5fcb0f22
...@@ -343,8 +343,10 @@ void WebEngineWallet::fillFormDataFromCache(const QList<QUrl> &urlList) ...@@ -343,8 +343,10 @@ void WebEngineWallet::fillFormDataFromCache(const QList<QUrl> &urlList)
void WebEngineWallet::saveFormDataToCache(const QString &key) void WebEngineWallet::saveFormDataToCache(const QString &key)
{ {
if (d->wallet) { if (d->wallet) {
d->saveDataToCache(key); bool remove = d->saveDataToCache(key);
d->pendingSaveRequests.remove(key); if (remove) {
d->pendingSaveRequests.remove(key);
}
return; return;
} }
d->openWallet(); d->openWallet();
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
* This file is part of the KDE project. * This file is part of the KDE project.
* *
* Copyright (C) 2020 Dawit Alemayehu <adawit@kde.org> Stefano Crocco <stefano.crocco@alice.it> * Copyright (C) 2020 Dawit Alemayehu <adawit@kde.org> Stefano Crocco <stefano.crocco@alice.it>
* *
* This library is free software; you can redistribute it and/or * This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public * modify it under the terms of the GNU Library General Public
* License as published by the Free Software Foundation; either * License as published by the Free Software Foundation; either
...@@ -56,7 +56,21 @@ public: ...@@ -56,7 +56,21 @@ public:
bool hasAutoFillableFields(const WebFormList &forms) const; bool hasAutoFillableFields(const WebFormList &forms) const;
void fillDataFromCache(WebEngineWallet::WebFormList &formList, bool custom); void fillDataFromCache(WebEngineWallet::WebFormList &formList, bool custom);
void saveDataToCache(const QString &key);
/**
* @brief Saves form data to the wallet
*
* If the wallet already contains data for this key, the data won't be saved immediately. Instead, the
* WebEngineWallet::saveFormDataRequested signal will be emitted, which will cause the WebEnginePart to
* ask the user what to do. In this case, this function will do nothing after emitting the signal.
*
* @note Callers of this function @b must remove the key from #pendingRemoveRequests if this function returns @b true.
*
* @param key the key corresponding to the data to save in #pendingRemoveRequests
* @return @b true if the data was saved immediately and @a key must be removed from #pendingRemoveRequests and @b false
* otherwise
*/
bool saveDataToCache(const QString &key);
void removeDataFromCache(const WebFormList &formList); void removeDataFromCache(const WebFormList &formList);
void openWallet(); void openWallet();
...@@ -118,7 +132,7 @@ WebEngineWallet::WebFormList WebEngineWallet::WebEngineWalletPrivate::parseFormD ...@@ -118,7 +132,7 @@ WebEngineWallet::WebFormList WebEngineWallet::WebEngineWalletPrivate::parseFormD
field.type = WebForm::fieldTypeFromTypeName(elementMap[QL1S("type")].toString().toLower()); field.type = WebForm::fieldTypeFromTypeName(elementMap[QL1S("type")].toString().toLower());
if (field.type == WebForm::WebFieldType::Other) { if (field.type == WebForm::WebFieldType::Other) {
continue; continue;
} }
field.id = elementMap[QL1S("id")].toString(); field.id = elementMap[QL1S("id")].toString();
field.name = elementMap[QL1S("name")].toString(); field.name = elementMap[QL1S("name")].toString();
field.readOnly = elementMap[QL1S("readonly")].toBool(); field.readOnly = elementMap[QL1S("readonly")].toBool();
...@@ -203,16 +217,16 @@ void WebEngineWallet::WebEngineWalletPrivate::fillDataFromCache(WebEngineWallet: ...@@ -203,16 +217,16 @@ void WebEngineWallet::WebEngineWalletPrivate::fillDataFromCache(WebEngineWallet:
} }
} }
void WebEngineWallet::WebEngineWalletPrivate::saveDataToCache(const QString &key) bool WebEngineWallet::WebEngineWalletPrivate::saveDataToCache(const QString &key)
{ {
// Make sure the specified keys exists before acting on it. See BR# 270209. // Make sure the specified keys exists before acting on it. See BR# 270209.
if (!pendingSaveRequests.contains(key)) { if (!pendingSaveRequests.contains(key)) {
return; return false;
} }
if (!wallet) { if (!wallet) {
qCWarning(WEBENGINEPART_LOG) << "NULL Wallet instance!"; qCWarning(WEBENGINEPART_LOG) << "NULL Wallet instance!";
return; return false;
} }
bool success = false; bool success = false;
...@@ -236,7 +250,7 @@ void WebEngineWallet::WebEngineWalletPrivate::saveDataToCache(const QString &key ...@@ -236,7 +250,7 @@ void WebEngineWallet::WebEngineWalletPrivate::saveDataToCache(const QString &key
}; };
if (std::any_of(form.fields.constBegin(), form.fields.constEnd(), fieldChanged)) { if (std::any_of(form.fields.constBegin(), form.fields.constEnd(), fieldChanged)) {
emit q->saveFormDataRequested(key, url); emit q->saveFormDataRequested(key, url);
return; return false;
} }
// If we got here it means the new credential is exactly // If we got here it means the new credential is exactly
// the same as the one already cached ; so skip the // the same as the one already cached ; so skip the
...@@ -264,6 +278,7 @@ void WebEngineWallet::WebEngineWalletPrivate::saveDataToCache(const QString &key ...@@ -264,6 +278,7 @@ void WebEngineWallet::WebEngineWalletPrivate::saveDataToCache(const QString &key
} }
emit q->saveFormDataCompleted(url, success); emit q->saveFormDataCompleted(url, success);
return true;
} }
void WebEngineWallet::WebEngineWalletPrivate::openWallet() void WebEngineWallet::WebEngineWalletPrivate::openWallet()
...@@ -320,10 +335,17 @@ void WebEngineWallet::WebEngineWalletPrivate::_k_openWalletDone(bool ok) ...@@ -320,10 +335,17 @@ void WebEngineWallet::WebEngineWalletPrivate::_k_openWalletDone(bool ok)
} }
// Do pending save requests... // Do pending save requests...
for (QHash<QString, WebFormList>::key_iterator it = pendingSaveRequests.keyBegin(); it != pendingSaveRequests.keyEnd(); ++it) { //NOTE: don't increment the iterator inside the for because it's done inside the cycle, depending on the value returned
saveDataToCache(*it); //by saveDataToCache
for (QHash<QString, WebFormList>::iterator it = pendingSaveRequests.begin(); it != pendingSaveRequests.end();) {
bool removeEntry = saveDataToCache(it.key());
//Only remove the entry if it could be saved without user confirmation
if (removeEntry) {
it = pendingSaveRequests.erase(it);
} else {
++it;
}
} }
pendingSaveRequests.clear();
// Do pending remove requests... // Do pending remove requests...
if (!pendingRemoveRequests.isEmpty()) { if (!pendingRemoveRequests.isEmpty()) {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment