diff --git a/.gitmodules b/.gitmodules index 46aa7d0b..b06ae33b 100644 --- a/.gitmodules +++ b/.gitmodules @@ -21,7 +21,7 @@ ignore = dirty [submodule "3rdparty/md4c"] path = 3rdparty/md4c - url = https://github.com/mity/md4c.git + url = https://github.com/fsimonfc/md4c.git ignore = dirty [submodule "3rdparty/tidy-html5"] path = 3rdparty/tidy-html5 diff --git a/3rdparty/md4c b/3rdparty/md4c index ad8d4112..635f2967 160000 --- a/3rdparty/md4c +++ b/3rdparty/md4c @@ -1 +1 @@ -Subproject commit ad8d41127b94e2f0633ad14b3787f0bc4613a689 +Subproject commit 635f29673582c55fb0b897b2b03236dd9f86c1ed diff --git a/src/app/messageparser.cpp b/src/app/messageparser.cpp index b6222405..01aa65fa 100644 --- a/src/app/messageparser.cpp +++ b/src/app/messageparser.cpp @@ -167,38 +167,6 @@ MessageParser::preprocessMarkdown(QString& markdown) markdown += block.second; } } -const QRegularExpression linkRegex(R"((http[s]?://[^\s]*[\+\-_][^\s]*|www\.[^\s]*[\+\-_][^\s]*))"); -const QString anchorTagTemplate("%1"); - -QByteArray -MessageParser::postprocessHTML(const QByteArray& html) -{ - // HACK : https://github.com/mity/md4c/issues/251 - // Check for any links that haven't been properly detected by md4c. - // We can do this by checking for any text that starts with "http" or "www" and contains a + or - // a -. If we find any, we insert a tag around the link. - // This function could become obsolete if the md4c library fixes the issue it's addressing. - - QByteArray processedHtml = html; - QRegularExpressionMatchIterator it = linkRegex.globalMatch(html); - - int offset = 0; - while (it.hasNext()) { - QRegularExpressionMatch match = it.next(); - QString link = match.captured(0); - int pTagIndex = link.indexOf("

"); - QString anchorTag = anchorTagTemplate.arg(pTagIndex != -1 ? link.left(pTagIndex) : link); - if (pTagIndex != -1) { - anchorTag += "

"; - } - processedHtml.replace(match.capturedStart(0) + offset, - match.capturedLength(0), - anchorTag.toUtf8()); - offset += anchorTag.toUtf8().length() - match.capturedLength(0); - } - - return processedHtml; -} QString MessageParser::markdownToHtml(const char* markdown) @@ -211,6 +179,5 @@ MessageParser::markdownToHtml(const char* markdown) } QByteArray array; const int result = md_html(markdown, MD_SIZE(data_len), &htmlChunkCb, &array, md_flags, 0); - array = postprocessHTML(array); return result == 0 ? QString::fromUtf8(array) : QString(); } diff --git a/src/app/messageparser.h b/src/app/messageparser.h index 46ca9ba8..99ce1589 100644 --- a/src/app/messageparser.h +++ b/src/app/messageparser.h @@ -60,9 +60,6 @@ private: // Preprocess the markdown message (e.g. handle line breaks). void preprocessMarkdown(QString& markdown); - // Postprocess HTML to account for issues in md4c - QByteArray postprocessHTML(const QByteArray& html); - // Transform markdown syntax into HTML. QString markdownToHtml(const char* markdown); diff --git a/tests/unittests/messageparser_unittest.cpp b/tests/unittests/messageparser_unittest.cpp index 639f2a14..3fffbfc5 100644 --- a/tests/unittests/messageparser_unittest.cpp +++ b/tests/unittests/messageparser_unittest.cpp @@ -108,8 +108,8 @@ TEST_F(MessageParserFixture, ALinkIsParsedCorrectly) } /*! - * WHEN We parse a text body with a link and a + or a -. - * THEN The HTML body should be returned correctly including the link even if MD4C doesn't detect it. + * WHEN We parse a text body with a link containing special characters ( +, -, _, etc.) + * THEN The HTML body should be returned correctly including the link. */ TEST_F(MessageParserFixture, AComplexLinkIsParsedCorrectly) { @@ -119,27 +119,45 @@ TEST_F(MessageParserFixture, AComplexLinkIsParsedCorrectly) QSignalSpy messageParsedSpy(globalEnv.messageParser.data(), &MessageParser::messageParsed); QSignalSpy linkInfoReadySpy(globalEnv.messageParser.data(), &MessageParser::linkInfoReady); - // Parse a message with a link containing a + or a -. - globalEnv.messageParser->parseMessage("msgId_03", - "https://review.jami.net/q/status:open+-is:wip", - false, - linkColor, - backgroundColor); + struct TestCase { + QString link; + QString expectedMessage; + }; + static const std::vector testCases = { + {"https://review.jami.net/q/status:open+-is:wip", + "

https://review.jami.net/q/status:open+-is:wip

\n"}, + {"https://git.jami.net/savoirfairelinux/jami-client-qt/-/issues/1885#note_53907", + "

https://git.jami.net/savoirfairelinux/jami-client-qt/-/issues/1885#note_53907

\n"}, + {"https://www.youtube.com/watch?v=_tEL8bJ7yqU", + "

https://www.youtube.com/watch?v=_tEL8bJ7yqU

\n"}, + {"https://www.youtube.com/watch?v=d_z77s7nifU", + "

https://www.youtube.com/watch?v=d_z77s7nifU

\n"}, + {"http://www.example.com/foo.php?bar[]=1&bar[]=2&bar[]=3", + "

http://www.example.com/foo.php?bar[]=1&bar[]=2&bar[]=3

\n"}, + }; - // Wait for the messageParsed signal which should be emitted once. - messageParsedSpy.wait(); - EXPECT_EQ(messageParsedSpy.count(), 1); + static const QString expectedMessageTemplate = "

%1

\n"; - QList messageParserArguments = messageParsedSpy.takeFirst(); - EXPECT_TRUE(messageParserArguments.at(0).typeId() == qMetaTypeId()); - EXPECT_EQ(messageParserArguments.at(0).toString(), "msgId_03"); - EXPECT_TRUE(messageParserArguments.at(1).typeId() == qMetaTypeId()); - EXPECT_EQ(messageParserArguments.at(1).toString(), - "

https://review.jami.net/q/" - "status:open+-is:wip

\n"); + for (const auto& testCase : testCases) { + globalEnv.messageParser->parseMessage("msgId", + testCase.link, + false, + linkColor, + backgroundColor); - // The rest of the link info is not tested here. + // Wait for the messageParsed signal which should be emitted once. + messageParsedSpy.wait(); + EXPECT_EQ(messageParsedSpy.count(), 1); + + QList messageParserArguments = messageParsedSpy.takeFirst(); + EXPECT_TRUE(messageParserArguments.at(1).typeId() == qMetaTypeId()); + + QString result = messageParserArguments.at(1).toString(); + QString expected = testCase.expectedMessage; + // Convert to std::string to get better error messages in case of test + // failure (GoogleTest can't print QStrings). + EXPECT_EQ(result.toStdString(), expected.toStdString()); + } } /*!