diff --git a/autotests/data/test-invalidmatrixtolink-event.json b/autotests/data/test-invalidmatrixtolink-event.json deleted file mode 100644 index c7b27a5a0..000000000 --- a/autotests/data/test-invalidmatrixtolink-event.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "content": { - "body": "https://matrix.to/#/@alice:example.org", - "msgtype": "m.text" - }, - "event_id": "$validlink1:example.org", - "origin_server_ts": 1432735824654, - "room_id": "!test:example.org", - "sender": "@example:example.org", - "type": "m.room.message", - "unsigned": { - "age": 1234 - } -} diff --git a/autotests/data/test-invalidmxclink-event.json b/autotests/data/test-invalidmxclink-event.json deleted file mode 100644 index dfb19dbbc..000000000 --- a/autotests/data/test-invalidmxclink-event.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "content": { - "body": "mxc://example.org/SEsfnsuifSDFSSEF", - "msgtype": "m.text" - }, - "event_id": "$validlink1:example.org", - "origin_server_ts": 1432735824654, - "room_id": "!test:example.org", - "sender": "@example:example.org", - "type": "m.room.message", - "unsigned": { - "age": 1234 - } -} diff --git a/autotests/data/test-invalidnospacelink-event.json b/autotests/data/test-invalidnospacelink-event.json deleted file mode 100644 index f4e7523a1..000000000 --- a/autotests/data/test-invalidnospacelink-event.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "content": { - "body": "testhttps://kde.org", - "msgtype": "m.text" - }, - "event_id": "$validlink1:example.org", - "origin_server_ts": 1432735824654, - "room_id": "!test:example.org", - "sender": "@example:example.org", - "type": "m.room.message", - "unsigned": { - "age": 1234 - } -} diff --git a/autotests/data/test-validplainlink-event.json b/autotests/data/test-validplainlink-event.json deleted file mode 100644 index 4a326cd11..000000000 --- a/autotests/data/test-validplainlink-event.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "content": { - "body": "https://kde.org", - "msgtype": "m.text" - }, - "event_id": "$validlink1:example.org", - "origin_server_ts": 1432735824654, - "room_id": "!test:example.org", - "sender": "@example:example.org", - "type": "m.room.message", - "unsigned": { - "age": 1234 - } -} diff --git a/autotests/data/test-validplainwwwlink-event.json b/autotests/data/test-validplainwwwlink-event.json deleted file mode 100644 index b586b419f..000000000 --- a/autotests/data/test-validplainwwwlink-event.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "content": { - "body": "www.example.org", - "msgtype": "m.text" - }, - "event_id": "$validlink1:example.org", - "origin_server_ts": 1432735824654, - "room_id": "!test:example.org", - "sender": "@example:example.org", - "type": "m.room.message", - "unsigned": { - "age": 1234 - } -} diff --git a/autotests/data/test-validrichlink-event.json b/autotests/data/test-validrichlink-event.json deleted file mode 100644 index c8e793b80..000000000 --- a/autotests/data/test-validrichlink-event.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "content": { - "body": "[Rich Link](https://kde.org)", - "format": "org.matrix.custom.html", - "formatted_body": "Rich Link", - "msgtype": "m.text" - }, - "event_id": "$validlink1:example.org", - "origin_server_ts": 1432735824654, - "room_id": "!test:example.org", - "sender": "@example:example.org", - "type": "m.room.message", - "unsigned": { - "age": 1234 - } -} diff --git a/autotests/linkpreviewertest.cpp b/autotests/linkpreviewertest.cpp index 7dddc7fe0..4ae445e35 100644 --- a/autotests/linkpreviewertest.cpp +++ b/autotests/linkpreviewertest.cpp @@ -6,12 +6,11 @@ #include "linkpreviewer.h" +#include "utils.h" #include #include #include -#include "utils.h" - #include "testutils.h" using namespace Quotient; @@ -30,6 +29,9 @@ private Q_SLOTS: void linkPreviewsMatch_data(); void linkPreviewsMatch(); + void multipleLinkPreviewsMatch_data(); + void multipleLinkPreviewsMatch(); + void linkPreviewsReject_data(); void linkPreviewsReject(); }; @@ -42,45 +44,59 @@ void LinkPreviewerTest::initTestCase() void LinkPreviewerTest::linkPreviewsMatch_data() { - QTest::addColumn("eventSource"); + QTest::addColumn("inputString"); QTest::addColumn("testOutputLink"); - QTest::newRow("plainHttps") << QStringLiteral("test-validplainlink-event.json") << QUrl("https://kde.org"_ls); - QTest::newRow("richHttps") << QStringLiteral("test-validrichlink-event.json") << QUrl("https://kde.org"_ls); - QTest::newRow("plainWww") << QStringLiteral("test-validplainwwwlink-event.json") << QUrl("www.example.org"_ls); - QTest::newRow("multipleHttps") << QStringLiteral("test-multiplelink-event.json") << QUrl("www.example.org"_ls); + QTest::newRow("plainHttps") << QStringLiteral("https://kde.org") << QUrl("https://kde.org"_ls); + QTest::newRow("richHttps") << QStringLiteral("Rich Link") << QUrl("https://kde.org"_ls); + QTest::newRow("richHttpsLinkDescription") << QStringLiteral("https://kde.org") << QUrl("https://kde.org"_ls); } void LinkPreviewerTest::linkPreviewsMatch() { - QFETCH(QString, eventSource); + QFETCH(QString, inputString); QFETCH(QUrl, testOutputLink); - auto event = TestUtils::loadEventFromFile(eventSource); - auto linkPreviewer = LinkPreviewer(LinkPreviewer::linkPreview(event.get()), connection); + auto link = LinkPreviewer::linkPreviews(inputString)[0]; - QCOMPARE(linkPreviewer.empty(), false); - QCOMPARE(linkPreviewer.url(), testOutputLink); + QCOMPARE(link, testOutputLink); +} + +void LinkPreviewerTest::multipleLinkPreviewsMatch_data() +{ + QTest::addColumn("inputString"); + QTest::addColumn>("testOutputLinks"); + + QTest::newRow("multipleHttps") << QStringLiteral("www.example.org https://kde.org") << QList{QUrl("www.example.org"_ls), QUrl("https://kde.org"_ls)}; + QTest::newRow("multipleHttps1Invalid") << QStringLiteral("www.example.org mxc://example.org/SEsfnsuifSDFSSEF") << QList{QUrl("www.example.org"_ls)}; +} + +void LinkPreviewerTest::multipleLinkPreviewsMatch() +{ + QFETCH(QString, inputString); + QFETCH(QList, testOutputLinks); + + auto links = LinkPreviewer::linkPreviews(inputString); + + QCOMPARE(links, testOutputLinks); } void LinkPreviewerTest::linkPreviewsReject_data() { - QTest::addColumn("eventSource"); + QTest::addColumn("inputString"); - QTest::newRow("mxc") << QStringLiteral("test-invalidmxclink-event.json"); - QTest::newRow("matrixTo") << QStringLiteral("test-invalidmatrixtolink-event.json"); - QTest::newRow("noSpace") << QStringLiteral("test-invalidnospacelink-event.json"); + QTest::newRow("mxc") << QStringLiteral("mxc://example.org/SEsfnsuifSDFSSEF"); + QTest::newRow("matrixTo") << QStringLiteral("https://matrix.to/#/@alice:example.org"); + QTest::newRow("noSpace") << QStringLiteral("testhttps://kde.org"); } void LinkPreviewerTest::linkPreviewsReject() { - QFETCH(QString, eventSource); + QFETCH(QString, inputString); - auto event = TestUtils::loadEventFromFile(eventSource); - auto linkPreviewer = LinkPreviewer(LinkPreviewer::linkPreview(event.get()), connection); + auto links = LinkPreviewer::linkPreviews(inputString); - QCOMPARE(linkPreviewer.empty(), true); - QCOMPARE(linkPreviewer.url(), QUrl()); + QCOMPARE(links.empty(), true); } QTEST_MAIN(LinkPreviewerTest) diff --git a/src/linkpreviewer.cpp b/src/linkpreviewer.cpp index 77406950e..f85913380 100644 --- a/src/linkpreviewer.cpp +++ b/src/linkpreviewer.cpp @@ -89,38 +89,23 @@ bool LinkPreviewer::empty() const return m_url.isEmpty(); } -QUrl LinkPreviewer::linkPreview(const Quotient::RoomMessageEvent *event) +QList LinkPreviewer::linkPreviews(QString string) { - if (event == nullptr) { - return {}; - } - - QString text; - if (event->hasTextContent()) { - auto textContent = static_cast(event->content()); - if (textContent) { - text = textContent->body; - } else { - text = event->plainBody(); - } - } else { - text = event->plainBody(); - } - - auto data = text.remove(TextRegex::removeRichReply); + auto data = string.remove(TextRegex::removeRichReply); auto linksMatch = TextRegex::url.globalMatch(data); + QList links; while (linksMatch.hasNext()) { auto link = linksMatch.next().captured(); - if (!link.contains(QStringLiteral("matrix.to"))) { - return QUrl(link); + if (!link.contains(QStringLiteral("matrix.to")) && !links.contains(QUrl(link))) { + links += QUrl(link); } } - return {}; + return links; } -bool LinkPreviewer::hasPreviewableLinks(const Quotient::RoomMessageEvent *event) +bool LinkPreviewer::hasPreviewableLinks(const QString &string) { - return !linkPreview(event).isEmpty(); + return !linkPreviews(string).isEmpty(); } #include "moc_linkpreviewer.cpp" diff --git a/src/linkpreviewer.h b/src/linkpreviewer.h index b275bcdd5..ed957cc96 100644 --- a/src/linkpreviewer.h +++ b/src/linkpreviewer.h @@ -7,11 +7,6 @@ #include #include -namespace Quotient -{ -class RoomMessageEvent; -} - class NeoChatRoom; /** @@ -71,19 +66,19 @@ public: [[nodiscard]] bool empty() const; /** - * @brief Whether the given event has at least 1 pre-viewable link. + * @brief Whether the given string has at least 1 pre-viewable link. * * A link is only pre-viewable if it is http, https or something starting with www. */ - static bool hasPreviewableLinks(const Quotient::RoomMessageEvent *event); + static bool hasPreviewableLinks(const QString &string); /** - * @brief Return the link to be previewed from the given event. + * @brief Return previewable links from the given string. * * This function is designed to give only links that should be previewed so * http, https or something starting with www. The first valid link is returned. */ - static QUrl linkPreview(const Quotient::RoomMessageEvent *event); + static QList linkPreviews(QString string); private: bool m_loaded; diff --git a/src/models/messagecontentmodel.cpp b/src/models/messagecontentmodel.cpp index 98cc8f9e9..42d56a74c 100644 --- a/src/models/messagecontentmodel.cpp +++ b/src/models/messagecontentmodel.cpp @@ -11,6 +11,7 @@ #include #include +#include #ifndef Q_OS_ANDROID #include @@ -110,11 +111,14 @@ MessageContentModel::MessageContentModel(const Quotient::RoomEvent *event, NeoCh endResetModel(); } }); - connect(m_room, &NeoChatRoom::urlPreviewEnabledChanged, this, &MessageContentModel::updateLinkPreviewer); - connect(NeoChatConfig::self(), &NeoChatConfig::ShowLinkPreviewChanged, this, &MessageContentModel::updateLinkPreviewer); + connect(m_room, &NeoChatRoom::urlPreviewEnabledChanged, this, [this]() { + updateComponents(); + }); + connect(NeoChatConfig::self(), &NeoChatConfig::ShowLinkPreviewChanged, this, [this]() { + updateComponents(); + }); } - updateLinkPreviewer(); updateComponents(); } @@ -197,8 +201,9 @@ QVariant MessageContentModel::data(const QModelIndex &index, int role) const return eventHandler.getReplyMediaInfo(); } if (role == LinkPreviewerRole) { - if (m_linkPreviewer != nullptr) { - return QVariant::fromValue(m_linkPreviewer); + if (component.type == MessageComponentType::LinkPreview) { + return QVariant::fromValue( + dynamic_cast(m_room->connection())->previewerForLink(component.attributes["link"_ls].toUrl())); } else { return QVariant::fromValue(emptyLinkPreview); } @@ -272,13 +277,7 @@ void MessageContentModel::updateComponents(bool isEditing) m_components.append(componentsForType(eventHandler.messageComponentType())); } - if (m_linkPreviewer != nullptr) { - if (m_linkPreviewer->loaded()) { - m_components += MessageComponent{MessageComponentType::LinkPreview, QString(), {}}; - } else { - m_components += MessageComponent{MessageComponentType::LinkPreviewLoad, QString(), {}}; - } - } + addLinkPreviews(); endResetModel(); } @@ -326,41 +325,60 @@ QList MessageContentModel::componentsForType(MessageComponentT } } -void MessageContentModel::updateLinkPreviewer() +MessageComponent MessageContentModel::linkPreviewComponent(const QUrl &link) { - if (m_room == nullptr || m_event == nullptr) { - if (m_linkPreviewer != nullptr) { - m_linkPreviewer->disconnect(this); - m_linkPreviewer = nullptr; - updateComponents(); - } - return; + const auto linkPreviewer = dynamic_cast(m_room->connection())->previewerForLink(link); + if (linkPreviewer == nullptr) { + return {}; } - if (!m_room->urlPreviewEnabled()) { - if (m_linkPreviewer != nullptr) { - m_linkPreviewer->disconnect(this); - m_linkPreviewer = nullptr; - updateComponents(); - } - return; - } - - if (const auto event = eventCast(m_event)) { - if (LinkPreviewer::hasPreviewableLinks(event)) { - m_linkPreviewer = dynamic_cast(m_room->connection())->previewerForLink(LinkPreviewer::linkPreview(event)); - updateComponents(); - - if (m_linkPreviewer != nullptr) { - connect(m_linkPreviewer, &LinkPreviewer::loadedChanged, [this]() { - if (m_linkPreviewer != nullptr && m_linkPreviewer->loaded()) { + if (linkPreviewer->loaded()) { + return MessageComponent{MessageComponentType::LinkPreview, QString(), {{"link"_ls, link}}}; + } else { + connect(linkPreviewer, &LinkPreviewer::loadedChanged, [this, link]() { + const auto linkPreviewer = dynamic_cast(m_room->connection())->previewerForLink(link); + if (linkPreviewer != nullptr && linkPreviewer->loaded()) { + for (auto &component : m_components) { + if (component.attributes["link"_ls].toUrl() == link) { // HACK: Because DelegateChooser can't switch the delegate on dataChanged it has to think there is a new delegate. beginResetModel(); - m_components[m_components.size() - 1].type = MessageComponentType::LinkPreview; + component.type = MessageComponentType::LinkPreview; endResetModel(); } - }); + } + } + }); + return MessageComponent{MessageComponentType::LinkPreviewLoad, QString(), {{"link"_ls, link}}}; + } +} + +void MessageContentModel::addLinkPreviews() +{ + int i = 0; + while (i < m_components.size()) { + const auto component = m_components.at(i); + if (component.type == MessageComponentType::Text || component.type == MessageComponentType::Quote) { + if (LinkPreviewer::hasPreviewableLinks(component.content)) { + const auto links = LinkPreviewer::linkPreviews(component.content); + for (qsizetype j = 0; j < links.size(); ++j) { + if (!m_removedLinkPreviews.contains(links[j])) { + m_components.insert(i + j + 1, linkPreviewComponent(links[j])); + } + }; } } + i++; + } +} + +void MessageContentModel::closeLinkPreview(int row) +{ + if (m_components[row].type == MessageComponentType::LinkPreview || m_components[row].type == MessageComponentType::LinkPreviewLoad) { + beginResetModel(); + m_removedLinkPreviews += m_components[row].attributes["link"_ls].toUrl(); + m_components.remove(row); + m_components.squeeze(); + updateComponents(); + endResetModel(); } } diff --git a/src/models/messagecontentmodel.h b/src/models/messagecontentmodel.h index 235a768ea..7d4638e65 100644 --- a/src/models/messagecontentmodel.h +++ b/src/models/messagecontentmodel.h @@ -88,6 +88,13 @@ public: */ [[nodiscard]] QHash roleNames() const override; + /** + * @brief Close the link preview at the given index. + * + * If the given index is not a link preview component, nothing happens. + */ + Q_INVOKABLE void closeLinkPreview(int row); + private: QPointer m_room; const Quotient::RoomEvent *m_event = nullptr; @@ -95,12 +102,14 @@ private: QList m_components; void updateComponents(bool isEditing = false); - QPointer m_linkPreviewer; ItineraryModel *m_itineraryModel = nullptr; QList componentsForType(MessageComponentType::Type type); + MessageComponent linkPreviewComponent(const QUrl &link); + void addLinkPreviews(); + + QList m_removedLinkPreviews; - void updateLinkPreviewer(); void updateItineraryModel(); bool m_emptyItinerary = false; diff --git a/src/timeline/Bubble.qml b/src/timeline/Bubble.qml index 4ac8ca668..0f2a525a4 100644 --- a/src/timeline/Bubble.qml +++ b/src/timeline/Bubble.qml @@ -166,6 +166,7 @@ QQC2.Control { root.selectedTextChanged(selectedText); } onShowMessageMenu: root.showMessageMenu() + onRemoveLinkPreview: index => root.contentModel.closeLinkPreview(index) } } } diff --git a/src/timeline/LinkPreviewComponent.qml b/src/timeline/LinkPreviewComponent.qml index 4d1af45d5..175c1516a 100644 --- a/src/timeline/LinkPreviewComponent.qml +++ b/src/timeline/LinkPreviewComponent.qml @@ -14,6 +14,11 @@ import org.kde.kirigami as Kirigami QQC2.Control { id: root + /** + * @brief The index of the delegate in the model. + */ + required property int index + /** * @brief The link preview properties. * @@ -32,7 +37,7 @@ QQC2.Control { * When the content of the link preview is larger than this it will be * elided/hidden until maximized. */ - property var defaultHeight: Kirigami.Units.gridUnit * 3 + Kirigami.Units.smallSpacing * 2 + property var defaultHeight: Kirigami.Units.gridUnit * 3 + Kirigami.Units.largeSpacing * 2 property bool truncated: linkPreviewDescription.truncated || !linkPreviewDescription.visible @@ -41,6 +46,11 @@ QQC2.Control { */ property real maxContentWidth: -1 + /** + * @brief Request for this delegate to be removed. + */ + signal remove(int index) + Layout.fillWidth: true Layout.maximumWidth: root.maxContentWidth @@ -110,6 +120,23 @@ QQC2.Control { } } + QQC2.Button { + id: closeButton + anchors.right: parent.right + anchors.top: parent.top + visible: root.hovered + text: i18nc("As in remove the link preview so it's no longer shown", "Remove preview") + icon.name: "dialog-close" + display: QQC2.AbstractButton.IconOnly + + onClicked: root.remove(root.index) + + QQC2.ToolTip { + text: closeButton.text + visible: closeButton.hovered + delay: Kirigami.Units.toolTipDelay + } + } QQC2.Button { id: maximizeButton anchors.right: parent.right @@ -122,7 +149,7 @@ QQC2.Control { QQC2.ToolTip { text: maximizeButton.text - visible: hovered + visible: maximizeButton.hovered delay: Kirigami.Units.toolTipDelay } } diff --git a/src/timeline/LoadComponent.qml b/src/timeline/LoadComponent.qml index a67a9d92f..f4102cb14 100644 --- a/src/timeline/LoadComponent.qml +++ b/src/timeline/LoadComponent.qml @@ -10,9 +10,14 @@ import org.kde.kirigami as Kirigami /** * @brief A component to show a link preview loading from a message. */ -RowLayout { +QQC2.Control { id: root + /** + * @brief The index of the delegate in the model. + */ + required property int index + required property int type /** @@ -28,6 +33,11 @@ RowLayout { */ property real maxContentWidth: -1 + /** + * @brief Request for this delegate to be removed. + */ + signal remove(int index) + enum Type { Reply, LinkPreview @@ -36,24 +46,46 @@ RowLayout { Layout.fillWidth: true Layout.maximumWidth: root.maxContentWidth - Rectangle { - Layout.fillHeight: true - width: Kirigami.Units.smallSpacing - color: Kirigami.Theme.highlightColor - } - QQC2.BusyIndicator {} - Kirigami.Heading { - Layout.fillWidth: true - Layout.minimumHeight: root.defaultHeight - verticalAlignment: Text.AlignVCenter - level: 2 - text: { - switch (root.type) { - case LoadComponent.Reply: - return i18n("Loading reply"); - case LoadComponent.LinkPreview: - return i18n("Loading URL preview"); + contentItem : RowLayout { + spacing: Kirigami.Units.smallSpacing + + Rectangle { + Layout.fillHeight: true + width: Kirigami.Units.smallSpacing + color: Kirigami.Theme.highlightColor + } + QQC2.BusyIndicator {} + Kirigami.Heading { + Layout.fillWidth: true + Layout.minimumHeight: root.defaultHeight + verticalAlignment: Text.AlignVCenter + level: 2 + text: { + switch (root.type) { + case LoadComponent.Reply: + return i18n("Loading reply"); + case LoadComponent.LinkPreview: + return i18n("Loading URL preview"); + } } } } + + QQC2.Button { + id: closeButton + anchors.right: parent.right + anchors.top: parent.top + visible: root.hovered && root.type === LoadComponent.LinkPreview + text: i18nc("As in remove the link preview so it's no longer shown", "Remove preview") + icon.name: "dialog-close" + display: QQC2.AbstractButton.IconOnly + + onClicked: root.remove(root.index) + + QQC2.ToolTip { + text: closeButton.text + visible: closeButton.hovered + delay: Kirigami.Units.toolTipDelay + } + } } diff --git a/src/timeline/MessageComponentChooser.qml b/src/timeline/MessageComponentChooser.qml index 36e209885..f4b3159cb 100644 --- a/src/timeline/MessageComponentChooser.qml +++ b/src/timeline/MessageComponentChooser.qml @@ -60,6 +60,8 @@ DelegateChooser { */ signal showMessageMenu + signal removeLinkPreview(int index) + role: "componentType" DelegateChoice { @@ -199,6 +201,7 @@ DelegateChooser { roleValue: MessageComponentType.LinkPreview delegate: LinkPreviewComponent { maxContentWidth: root.maxContentWidth + onRemove: index => root.removeLinkPreview(index) } } @@ -207,6 +210,7 @@ DelegateChooser { delegate: LoadComponent { type: LoadComponent.LinkPreview maxContentWidth: root.maxContentWidth + onRemove: index => root.removeLinkPreview(index) } }