Git commit 1b4a23fe5f9df90779a04670c4c07135aefbc390 by Jan Kundr?t. Committed on 08/08/2013 at 10:08. Pushed by jkt into branch 'master'.
GUI: respect Content-Disposition for all MIME types This patch reworks the way how "attachments" are wrapped in the AttachmentView. It's done via the following steps: 1) The wrapping is moved into the PartWidgetFactory::create 2) LoadablePartWidget is changed to delegate actual widget instantiation to the PartWidgetFactory 3) A bunch of changes were needed to make sure that there are no excessive click-through buttons when the attachment was originally hidden, when the network is offline, etc. 4) The "supported MIME types" got changed so that container types (i.e. parts which are made from other parts, and wherefore it doesn't make sense to check their size directly) are handled properly (i.e. don't invoke click-through on them) M +20 -7 src/Gui/LoadablePartWidget.cpp M +8 -9 src/Gui/LoadablePartWidget.h M +3 -2 src/Gui/PartWidget.cpp M +83 -58 src/Gui/PartWidgetFactory.cpp M +8 -4 src/Gui/PartWidgetFactory.h http://commits.kde.org/trojita/1b4a23fe5f9df90779a04670c4c07135aefbc390 diff --git a/src/Gui/LoadablePartWidget.cpp b/src/Gui/LoadablePartWidget.cpp index 3a44fdc..25fe620 100644 --- a/src/Gui/LoadablePartWidget.cpp +++ b/src/Gui/LoadablePartWidget.cpp @@ -29,13 +29,21 @@ namespace Gui { -LoadablePartWidget::LoadablePartWidget(QWidget *parent, Imap::Network::MsgPartNetAccessManager *manager, const QModelIndex &part, - MessageView *messageView, const LoadingTriggerMode mode): - QStackedWidget(parent), manager(manager), partIndex(part), m_messageView(messageView), realPart(0), - loadButton(0), m_loadOnShow(mode == LOAD_ON_SHOW) +LoadablePartWidget::LoadablePartWidget(QWidget *parent, Imap::Network::MsgPartNetAccessManager *manager, const QModelIndex &part, + MessageView *messageView, PartWidgetFactory *factory, int recursionDepth, + const PartWidgetFactory::PartLoadingOptions loadingMode): + QStackedWidget(parent), manager(manager), partIndex(part), m_messageView(messageView), m_factory(factory), + m_recursionDepth(recursionDepth), m_loadingMode(loadingMode), realPart(0), loadButton(0), m_loadOnShow(false) { Q_ASSERT(partIndex.isValid()); - if (mode == LOAD_ON_CLICK) { + + QString mimeType = partIndex.data(Imap::Mailbox::RolePartMimeType).toString().toLower(); + + if ((loadingMode & PartWidgetFactory::PART_IS_HIDDEN) || + (loadingMode & PartWidgetFactory::PART_IGNORE_CLICKTHROUGH) || + partIndex.data(Imap::Mailbox::RoleIsFetched).toBool()) { + m_loadOnShow = true; + } else { loadButton = new QPushButton(tr("Load %1 (%2)").arg(partIndex.data(Imap::Mailbox::RolePartMimeType).toString(), Imap::Mailbox::PrettySize::prettySize(partIndex.data(Imap::Mailbox::RolePartOctets).toUInt())), this); @@ -56,14 +64,19 @@ void LoadablePartWidget::loadClicked() loadButton->deleteLater(); loadButton = 0; } - realPart = new SimplePartWidget(this, manager, partIndex, m_messageView); + + // We have to disable any flags which might cause recursion here + realPart = m_factory->create(partIndex, m_recursionDepth + 1, + (m_loadingMode | PartWidgetFactory::PART_IGNORE_CLICKTHROUGH + | PartWidgetFactory::PART_IGNORE_LOAD_ON_SHOW) ^ PartWidgetFactory::PART_IS_HIDDEN); addWidget(realPart); setCurrentIndex(1); } QString LoadablePartWidget::quoteMe() const { - return realPart ? realPart->quoteMe() : QString(); + AbstractPartWidget *part = dynamic_cast<AbstractPartWidget*>(realPart); + return part ? part->quoteMe() : QString(); } void LoadablePartWidget::showEvent(QShowEvent *event) diff --git a/src/Gui/LoadablePartWidget.h b/src/Gui/LoadablePartWidget.h index 67fc5bd..51db1c5 100644 --- a/src/Gui/LoadablePartWidget.h +++ b/src/Gui/LoadablePartWidget.h @@ -25,8 +25,8 @@ #include <QPersistentModelIndex> #include <QStackedWidget> -#include "AbstractPartWidget.h" -#include "SimplePartWidget.h" +#include "Gui/AbstractPartWidget.h" +#include "Gui/PartWidgetFactory.h" class QPushButton; @@ -46,13 +46,9 @@ class LoadablePartWidget : public QStackedWidget, public AbstractPartWidget { Q_OBJECT public: - /** @short Load when the widget becomes visible, or wait until the user clicks a button? */ - typedef enum { - LOAD_ON_SHOW, /**< @short Load as soon as the widget becomes visible */ - LOAD_ON_CLICK /**< @short Load only after the user has clicked a button */ - } LoadingTriggerMode; LoadablePartWidget(QWidget *parent, Imap::Network::MsgPartNetAccessManager *manager, const QModelIndex &part, - MessageView *messageView, const LoadingTriggerMode mode); + MessageView *messageView, PartWidgetFactory *factory, int recursionDepth, + const PartWidgetFactory::PartLoadingOptions loadingMode); QString quoteMe() const; virtual void reloadContents(); protected: @@ -63,7 +59,10 @@ private: Imap::Network::MsgPartNetAccessManager *manager; QPersistentModelIndex partIndex; MessageView *m_messageView; - SimplePartWidget *realPart; + PartWidgetFactory *m_factory; + int m_recursionDepth; + PartWidgetFactory::PartLoadingOptions m_loadingMode; + QWidget *realPart; QPushButton *loadButton; bool m_loadOnShow; diff --git a/src/Gui/PartWidget.cpp b/src/Gui/PartWidget.cpp index 08d7acb..41f8185 100644 --- a/src/Gui/PartWidget.cpp +++ b/src/Gui/PartWidget.cpp @@ -27,6 +27,7 @@ #include <QTabBar> #include "EnvelopeView.h" +#include "LoadablePartWidget.h" #include "MessageView.h" #include "PartWidgetFactory.h" #include "Imap/Model/ItemRoles.h" @@ -74,8 +75,8 @@ MultipartAlternativeWidget::MultipartAlternativeWidget(QWidget *parent, // I can live with that. QWidget *item = factory->create(anotherPart, recursionDepth + 1, i == preferredIndex ? - PartWidgetFactory::LOAD_IMMEDIATELY : - PartWidgetFactory::LOAD_ON_SHOW); + PartWidgetFactory::PartLoadingOptions() : + PartWidgetFactory::PartLoadingOptions() | PartWidgetFactory::PART_IS_HIDDEN); QString mimeType = anotherPart.data(Imap::Mailbox::RolePartMimeType).toString(); addTab(item, mimeType); } diff --git a/src/Gui/PartWidgetFactory.cpp b/src/Gui/PartWidgetFactory.cpp index 25ed5c7..6808d17 100644 --- a/src/Gui/PartWidgetFactory.cpp +++ b/src/Gui/PartWidgetFactory.cpp @@ -57,7 +57,7 @@ QWidget *PartWidgetFactory::create(const QModelIndex &partIndex) return create(partIndex, 0); } -QWidget *PartWidgetFactory::create(const QModelIndex &partIndex, int recursionDepth, const PartLoadingMode loadingMode) +QWidget *PartWidgetFactory::create(const QModelIndex &partIndex, int recursionDepth, const PartLoadingOptions loadingMode) { using namespace Imap::Mailbox; Q_ASSERT(partIndex.isValid()); @@ -68,9 +68,84 @@ QWidget *PartWidgetFactory::create(const QModelIndex &partIndex, int recursionDe "the top-most thousand items or so are shown."), 0); } + QString mimeType = partIndex.data(Imap::Mailbox::RolePartMimeType).toString().toLower(); + bool isCompoundMimeType = mimeType.startsWith(QLatin1String("multipart/")) || mimeType == QLatin1String("message/rfc822"); + + if (loadingMode & PART_IS_HIDDEN) { + return new LoadablePartWidget(0, manager, partIndex, m_messageView, this, recursionDepth + 1, + loadingMode | PART_IGNORE_CLICKTHROUGH); + } + + // Check whether we can render this MIME type at all + QStringList allowedMimeTypes; + allowedMimeTypes << "text/html" << "text/plain" << "image/jpeg" << + "image/jpg" << "image/pjpeg" << "image/png" << "image/gif"; + bool recognizedMimeType = isCompoundMimeType || allowedMimeTypes.contains(mimeType); + bool isDerivedMimeType = false; + if (!recognizedMimeType) { + // QMimeType's docs say that one shall use inherit() to check for "is this a recognized MIME type". + // E.g. text/x-csrc inherits text/plain. + QMimeType partType = QMimeDatabase().mimeTypeForName(mimeType); + Q_FOREACH(const QString &candidate, allowedMimeTypes) { + if (partType.isValid() && !partType.isDefault() && partType.inherits(candidate)) { + // Looks like we shall be able to show this + recognizedMimeType = true; + // If it's a derived MIME type, it makes sense to not block inline display, yet still make it possible to hide it + // using the regular attachment controls + isDerivedMimeType = true; + manager->registerMimeTypeTranslation(mimeType, candidate); + break; + } + } + } + + const Imap::Mailbox::Model *constModel = 0; + Imap::Mailbox::TreeItemPart *part = dynamic_cast<Imap::Mailbox::TreeItemPart *>(Imap::Mailbox::Model::realTreeItem(partIndex, &constModel)); + Imap::Mailbox::Model *model = const_cast<Imap::Mailbox::Model *>(constModel); + Q_ASSERT(model); + Q_ASSERT(part); + + // Check whether it shall be wrapped inside an AttachmentView + // From section 2.8 of RFC 2183: "Unrecognized disposition types should be treated as `attachment'." + const QByteArray contentDisposition = partIndex.data(Imap::Mailbox::RolePartBodyDisposition).toByteArray().toLower(); + const bool isInline = contentDisposition.isEmpty() || contentDisposition == "inline"; + const bool looksLikeAttachment = !partIndex.data(Imap::Mailbox::RolePartFileName).toString().isEmpty(); + const bool wrapInAttachmentView = !(loadingMode & PART_IGNORE_DISPOSITION_ATTACHMENT) + && (looksLikeAttachment || !isInline || !recognizedMimeType || isDerivedMimeType); + if (wrapInAttachmentView) { + // The problem is that some nasty MUAs (hint hint Thunderbird) would + // happily attach a .tar.gz and call it "inline" + QWidget *contentWidget = 0; + if (recognizedMimeType) { + PartLoadingOptions options = loadingMode | PART_IGNORE_DISPOSITION_ATTACHMENT; + if (!isInline) { + // The widget will be hidden by default, i.e. the "inline preview" will be deactivated. + // If the user clicks that action in the AttachmentView, it makes sense to load the plugin without any further ado, + // without requiring an extra clickthrough + options |= PART_IS_HIDDEN; + } else if (!isCompoundMimeType) { + // This is to prevent a clickthrough when the data can be already shown + part->fetchFromCache(model); + } + if (!model->isNetworkAvailable()) { + // This is to prevent a clickthrough when offline + options |= PART_IGNORE_CLICKTHROUGH; + } + contentWidget = new LoadablePartWidget(0, manager, partIndex, m_messageView, this, recursionDepth + 1, options); + if (!isInline) { + contentWidget->hide(); + } + } + // Previously, we would also hide an attachment if the current policy was "expensive network", the part was too big + // and not fetched yet. Arguably, that's a bug -- an item which is marked online shall not be hidden. + // Wrapping via a clickthrough is fine, though; the user is clearly informed that this item *should* be visible, + // yet the bandwidth is not excessively trashed. + return new AttachmentView(0, manager, partIndex, contentWidget); + } + + // Now we know for sure that it's not supposed to be wrapped in an AttachmentView, cool. bool userPrefersPlaintext = QSettings().value(Common::SettingsNames::guiPreferPlaintextRendering, QVariant(true)).toBool(); - QString mimeType = partIndex.data(Imap::Mailbox::RolePartMimeType).toString(); if (mimeType.startsWith(QLatin1String("multipart/"))) { // it's a compound part if (mimeType == QLatin1String("multipart/alternative")) { @@ -136,65 +211,15 @@ QWidget *PartWidgetFactory::create(const QModelIndex &partIndex, int recursionDe } else if (mimeType == QLatin1String("message/rfc822")) { return new Message822Widget(0, this, partIndex, recursionDepth); } else { - QStringList allowedMimeTypes; - allowedMimeTypes << "text/html" << "text/plain" << "image/jpeg" << - "image/jpg" << "image/pjpeg" << "image/png" << "image/gif"; - - // From section 2.8 of RFC 2183: "Unrecognized disposition types should be treated as `attachment'." - QByteArray contentDisposition = partIndex.data(Imap::Mailbox::RolePartBodyDisposition).toByteArray().toLower(); - bool showInline = contentDisposition.isEmpty() || contentDisposition == "inline"; - - bool recognizedMimeType = allowedMimeTypes.contains(mimeType); - if (!recognizedMimeType) { - // QMimeType's docs say that one shall use inherit() to check for "is this a recognized MIME type". - // E.g. text/x-csrc inherits text/plain. - QMimeType partType = QMimeDatabase().mimeTypeForName(mimeType); - Q_FOREACH(const QString &candidate, allowedMimeTypes) { - if (partType.isValid() && !partType.isDefault() && partType.inherits(candidate)) { - // Looks like we shall be able to show this - recognizedMimeType = true; - manager->registerMimeTypeTranslation(mimeType, candidate); - break; - } - } - } - - const Imap::Mailbox::Model *constModel = 0; - Imap::Mailbox::TreeItemPart *part = dynamic_cast<Imap::Mailbox::TreeItemPart *>(Imap::Mailbox::Model::realTreeItem(partIndex, &constModel)); - Imap::Mailbox::Model *model = const_cast<Imap::Mailbox::Model *>(constModel); - Q_ASSERT(model); - Q_ASSERT(part); part->fetchFromCache(model); - // The problem is that some nasty MUAs (hint hint Thunderbird) would - // happily attach a .tar.gz and call it "inline" - if (showInline && recognizedMimeType && partIndex.data(Imap::Mailbox::RolePartFileName).toString().isEmpty()) { - // showing inline without any decorations whatsoever - - bool showDirectly = loadingMode == LOAD_IMMEDIATELY; - if (!part->fetched()) - showDirectly &= model->isNetworkOnline() || part->octets() <= ExpensiveFetchThreshold; - - QWidget *widget = 0; - if (showDirectly) { - widget = new SimplePartWidget(0, manager, partIndex, m_messageView); - } else { - widget = new LoadablePartWidget(0, manager, partIndex, m_messageView, - loadingMode == LOAD_ON_SHOW && part->octets() <= ExpensiveFetchThreshold ? - LoadablePartWidget::LOAD_ON_SHOW : - LoadablePartWidget::LOAD_ON_CLICK); - } - return widget; + if ((loadingMode & PART_IGNORE_CLICKTHROUGH) || (loadingMode & PART_IGNORE_LOAD_ON_SHOW) || + part->fetched() || model->isNetworkOnline() || part->octets() < ExpensiveFetchThreshold) { + // Show it directly without any fancy wrapping + return new SimplePartWidget(0, manager, partIndex, m_messageView); } else { - QWidget *contentWidget = recognizedMimeType ? - new LoadablePartWidget(0, manager, partIndex, m_messageView, LoadablePartWidget::LOAD_ON_SHOW) : 0; - if (contentWidget && !showInline) { - contentWidget->hide(); - } - if (contentWidget && !part->fetched() && !model->isNetworkOnline() && part->octets() > ExpensiveFetchThreshold) { - contentWidget->hide(); - } - return new AttachmentView(0, manager, partIndex, contentWidget); + return new LoadablePartWidget(0, manager, partIndex, m_messageView, this, recursionDepth + 1, + model->isNetworkAvailable() ? loadingMode : loadingMode | PART_IGNORE_CLICKTHROUGH); } } QLabel *lbl = new QLabel(mimeType, 0); diff --git a/src/Gui/PartWidgetFactory.h b/src/Gui/PartWidgetFactory.h index 0fe0f5e..abdaf2b 100644 --- a/src/Gui/PartWidgetFactory.h +++ b/src/Gui/PartWidgetFactory.h @@ -40,13 +40,17 @@ class PartWidgetFactory public: /** @short Are the parts supposed to be visible immediately, or only after their respective widget is shown? */ typedef enum { - LOAD_IMMEDIATELY, /**< @short Load them immediately */ - LOAD_ON_SHOW /**< @short Load the parts only after they have been shown to the user */ - } PartLoadingMode; + PART_IGNORE_DISPOSITION_ATTACHMENT = 1 << 0, /**< @short Don't wrap the part in an AttachmentView */ + PART_IGNORE_CLICKTHROUGH = 1 << 1, /**< @short Ignore any heuristics which lead to wrapping in an LoadablePartWidget with a clickthrough */ + PART_IGNORE_LOAD_ON_SHOW = 1 << 2, /**< @short Ignore wrapping in a LoadablePartWidget set up to load on first show event */ + PART_IS_HIDDEN = 1 << 3 /**< @short Request wrapping this part in a LoadablePartWidget */ + } PartLoadingFlag; + typedef QFlags<PartLoadingFlag> PartLoadingOptions; PartWidgetFactory(Imap::Network::MsgPartNetAccessManager *manager, MessageView *messageView); QWidget *create(const QModelIndex &partIndex); - QWidget *create(const QModelIndex &partIndex, int recursionDepth, const PartLoadingMode loadingMode = LOAD_IMMEDIATELY); + QWidget *create(const QModelIndex &partIndex, int recursionDepth, const PartLoadingOptions loadingMode = PartLoadingOptions()); + MessageView *messageView() const; private: Imap::Network::MsgPartNetAccessManager *manager;
