fix: external images now actually load via QTextBrowser::loadResource override

Previous fix rewrote HTML to data: URIs with a nested QEventLoop inside
processImages() — fragile and could silently fail on heavy multi-image mails.
Replaced with the canonical Qt approach: an ImageTextBrowser subclass that
overrides loadResource() and downloads http(s) image resources via
QNetworkAccessManager with a QPixmap cache. QTextBrowser calls loadResource
for each <img>, so when images are allowed we just leave the src URLs intact
and setExternalImagesAllowed(true).

- Add ImageTextBrowser (QTextBrowser subclass) in readerview.h
- m_bodyViewer now ImageTextBrowser*, wired to m_network at construction
- sanitizeHtml: on allow, keep http(s) src and set the flag; on block,
  replace with placeholder as before (flag off)
- Remove dead processImages()

Verified against a real Enverus image over HTTPS: downloads 74,692 bytes and
produces a valid QPixmap 680x113. Also verified the button flow end-to-end.
This commit is contained in:
2026-09-05 13:58:24 +02:00
parent 30a425cf8a
commit 75817a6290
2 changed files with 79 additions and 70 deletions
+8 -66
View File
@@ -472,7 +472,8 @@ void ReaderView::setupBodyViewer() {
m_bodyLayout->setContentsMargins(16, 16, 16, 16);
m_bodyLayout->setSpacing(0);
m_bodyViewer = new QTextBrowser();
m_bodyViewer = new ImageTextBrowser();
m_bodyViewer->setNetworkManager(m_network);
m_bodyViewer->setOpenExternalLinks(false); // We'll handle link clicks securely
m_bodyViewer->setFrameStyle(QFrame::NoFrame);
m_bodyViewer->setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Preferred);
@@ -659,67 +660,6 @@ void ReaderView::saveTrustedSenders()
s.sync();
}
QString ReaderView::processImages(QString html, bool allow)
{
if (!allow) return html;
// Find all external (http/https) image URLs in the html.
QRegularExpression re("(?i)<img\\b([^>]*\\bsrc\\s*=\\s*[\"'])(https?://[^\"'>\\s]+)([\"'][^>]*)>");
QStringList urls;
auto mit = re.globalMatch(html);
while (mit.hasNext()) {
auto m = mit.next();
urls << m.captured(2);
}
if (urls.isEmpty()) return html;
// Download each unique URL and substitute with a data: URI (QTextBrowser
// cannot fetch http images itself). Synchronous within reason; a short
// per-request timeout keeps the UI responsive.
QStringList done;
for (const QString &url : urls) {
if (done.contains(url)) continue;
done << url;
QUrl u(url);
if (!u.isValid() || (u.scheme() != "http" && u.scheme() != "https")) continue;
QNetworkRequest req(u);
req.setTransferTimeout(8000);
QNetworkReply *reply = m_network->get(req);
QEventLoop loop;
QTimer timer; timer.setSingleShot(true);
connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit);
connect(reply, &QNetworkReply::finished, &loop, &QEventLoop::quit);
timer.start(8000);
loop.exec();
QString replacement;
if (reply->error() == QNetworkReply::NoError) {
const QByteArray data = reply->readAll();
if (!data.isEmpty()) {
const QByteArray mime = reply->header(QNetworkRequest::ContentTypeHeader)
.toByteArray().split(';').first().trimmed();
const QByteArray b64 = data.toBase64();
replacement = QString("data:%1;base64,%2").arg(QString::fromLatin1(mime.isEmpty() ? "image/png" : mime),
QString::fromLatin1(b64));
}
} else {
qDebug() << "[ReaderView] image load failed:" << url << reply->errorString();
}
reply->deleteLater();
if (!replacement.isEmpty()) {
// Replace this exact src URL (the first occurrence only risk is fine here because
// we iterate unique URLs and each appears verbatim).
html.replace(QUrl::fromPercentEncoding(url.toUtf8()), replacement);
// Also try replacing the raw-encoded form if slightly different.
html.replace(url, replacement);
}
}
return html;
}
QString ReaderView::sanitizeHtml(const QString &html) {
if (html.isEmpty()) return "<div class='email-body'><i style='color:#999'>(Mensaje vacío)</i></div>";
@@ -741,7 +681,10 @@ QString ReaderView::sanitizeHtml(const QString &html) {
// An image is "allowed" if the user clicked "Cargar imágenes", or if this sender is trusted
// ("Mostrar siempre imágenes de este remitente" persisted choice).
const bool trusted = !m_currentSender.isEmpty() && isSenderTrusted(m_currentSender);
if (!m_allowExternalImages && !trusted) {
const bool allowImages = m_allowExternalImages || trusted;
if (m_bodyViewer) m_bodyViewer->setExternalImagesAllowed(allowImages);
if (!allowImages) {
// Match images with http/https/data URLs in src attribute
QRegularExpression extImgRegex("(?i)<img([^>]*src\\s*=\\s*[\"'](?:https?|data):[^\"']*[\"'][^>]*)>");
int beforeCount = result.count("data-blocked=\"true\"");
@@ -756,11 +699,10 @@ QString ReaderView::sanitizeHtml(const QString &html) {
if (m_loadImagesButton) m_loadImagesButton->setVisible(hasBlocked);
if (m_alwaysLoadImagesButton) m_alwaysLoadImagesButton->setVisible(hasBlocked);
} else {
// Images are allowed for this view: hide both buttons and download+render
// the external images as data: URIs (QTextBrowser cannot fetch http itself).
// Images are allowed: leave the http(s) src URLs intact in the HTML.
// ImageTextBrowser::loadResource() will download them via QNetworkAccessManager.
if (m_loadImagesButton) m_loadImagesButton->setVisible(false);
if (m_alwaysLoadImagesButton) m_alwaysLoadImagesButton->setVisible(false);
result = processImages(result, true);
}
// Add dark mode class if needed
+71 -4
View File
@@ -13,10 +13,78 @@
#include <QFrame>
#include <QMenu>
#include <QNetworkAccessManager>
#include <QNetworkReply>
#include <QNetworkRequest>
#include <QEventLoop>
#include <QTimer>
#include <QPixmap>
#include <QHash>
#include <QSet>
#include <QSettings>
#include "core/mailitem.h"
/// QTextBrowser that can fetch external (http/https) images. Qt's QTextBrowser
/// cannot load remote images by itself; overriding loadResource() lets us download
/// them (with a cache) when images are allowed.
class ImageTextBrowser : public QTextBrowser
{
Q_OBJECT
public:
explicit ImageTextBrowser(QWidget *parent = nullptr) : QTextBrowser(parent) {}
void setNetworkManager(QNetworkAccessManager *net) { m_net = net; }
void setExternalImagesAllowed(bool on) { m_allowImages = on; }
bool externalImagesAllowed() const { return m_allowImages; }
protected:
QVariant loadResource(int type, const QUrl &name) override
{
if (!m_allowImages || type != QTextDocument::ImageResource) {
// Not an image we are allowed to fetch; let base handle (usually nothing).
return QTextBrowser::loadResource(type, name);
}
if (!m_net) return QVariant();
const QString key = name.toString();
if (m_cache.contains(key)) return m_cache.value(key);
// Only fetch http(s); skip data:, file:, qrc:, about:blob etc.
const QUrl u(name);
if (u.scheme() != "http" && u.scheme() != "https")
return QTextBrowser::loadResource(type, name);
// Synchronous download with a short timeout (keeps render deterministic).
QNetworkRequest req(u);
req.setTransferTimeout(8000);
QNetworkReply *reply = m_net->get(req);
QEventLoop loop;
QTimer timer; timer.setSingleShot(true);
connect(&timer, &QTimer::timeout, &loop, &QEventLoop::quit);
connect(reply, &QNetworkReply::finished, &loop, &QEventLoop::quit);
timer.start(8000);
loop.exec();
QVariant result;
if (reply->error() == QNetworkReply::NoError) {
const QByteArray data = reply->readAll();
QPixmap pm;
if (!data.isEmpty() && pm.loadFromData(data)) {
result = pm;
m_cache.insert(key, pm);
}
} else {
qDebug() << "[ReaderView] loadResource failed:" << name.toString() << reply->errorString();
}
reply->deleteLater();
return result;
}
private:
QNetworkAccessManager *m_net = nullptr;
bool m_allowImages = false;
QHash<QString, QPixmap> m_cache;
};
class ReaderView : public QWidget {
Q_OBJECT
@@ -73,7 +141,7 @@ private:
// Body
QScrollArea *m_scrollArea;
QTextBrowser *m_bodyViewer;
ImageTextBrowser *m_bodyViewer;
QWidget *m_bodyContainer;
QVBoxLayout *m_bodyLayout;
qreal m_zoomFactor = 1.0;
@@ -114,7 +182,6 @@ private:
static QString trustKey(const QString &sender) { return sender.trimmed().toLower(); }
void loadTrustedSenders();
void saveTrustedSenders();
/// Post-process html: if images allowed (via button or trusted sender), replace
/// external http(s) image src with downloaded data: URIs so QTextBrowser shows them.
QString processImages(QString html, bool allow);
/// Post-process html: if images allowed (via button or trusted sender), keep the
/// http(s) src URLs so ImageTextBrowser::loadResource() downloads them.
};