From d492192b7a9f682dac4a530a5a68ed74f006ddc2 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Thu, 20 Aug 2026 18:26:37 +0200 Subject: fix(compose): correct the header's attribution and harden three tests, item 123 The header still credited CMARK_OPT_SAFE after the .cpp comment and the test were corrected, which left the wrong mechanism named in the file MessageBuilder's author will actually read. Three test weaknesses, each measured rather than assumed. The accented-text test survived a SYMMETRIC latin-1 mutation, since the round trip cancels for codepoints under U+0100, so it now carries a character latin-1 cannot represent. The tasklist test asserted on the bare word "checked", which ordinary prose would satisfy, and now asserts the attribute. And the extension registration is wrapped in a function-local static: cmark-gfm's registry has no once-guard, and this project has a worker thread, so the first call racing itself would tear the registry rather than crash cleanly. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015muoUo2GdxmBDSp5vjYcbE --- src/markdownrenderer.cpp | 15 ++++++++++----- src/markdownrenderer.h | 4 +++- tests/CMakeLists.txt | 2 +- tests/test_markdownrenderer.cpp | 11 +++++++++-- 4 files changed, 23 insertions(+), 9 deletions(-) diff --git a/src/markdownrenderer.cpp b/src/markdownrenderer.cpp index ccb3309..7158981 100644 --- a/src/markdownrenderer.cpp +++ b/src/markdownrenderer.cpp @@ -45,11 +45,16 @@ QString MarkdownRenderer::toHtml(const QString &markdown) if (markdown.isEmpty()) return {}; - // Idempotent and required before cmark_find_syntax_extension() can resolve - // any name. Calling it per render rather than once at startup keeps this - // function free of initialisation order concerns; it is a hash lookup - // after the first call. - cmark_gfm_core_extensions_ensure_registered(); + // Idempotent, and a hash lookup after the first call. The function-local + // static makes the FIRST call thread-safe: cmark-gfm's registry carries no + // once-guard of its own, so two threads racing the first call would tear + // it. Today's only caller is on the UI thread; this costs nothing and + // removes the trap before a worker-thread caller finds it. + static const bool registered = [] { + cmark_gfm_core_extensions_ensure_registered(); + return true; + }(); + Q_UNUSED(registered) // CMARK_OPT_DEFAULT is 0, and CMARK_OPT_SAFE is a NO-OP in cmark-gfm 0.29: // safe mode has been the default since that release, and the flag is kept diff --git a/src/markdownrenderer.h b/src/markdownrenderer.h index 80c52bf..6373fd9 100644 --- a/src/markdownrenderer.h +++ b/src/markdownrenderer.h @@ -32,7 +32,9 @@ namespace MarkdownRenderer { /// /// Three extensions are enabled (autolink, strikethrough, tasklist) and /// tables are deliberately not. Raw HTML in the input is suppressed by -/// CMARK_OPT_SAFE. +/// cmark-gfm's safe mode, which is the DEFAULT in 0.29 and is not the +/// CMARK_OPT_SAFE flag (a no-op); see markdownrenderer.cpp for the +/// measurement. The requirement is that CMARK_OPT_UNSAFE is never set. QString toHtml(const QString &markdown); } // namespace MarkdownRenderer diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 9f6e851..58d5659 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -68,10 +68,10 @@ add_qtmaildir_test(searchterm) add_qtmaildir_test(busyindicator) add_qtmaildir_test(tagstrip) add_qtmaildir_test(messagedetailsdialog) +add_qtmaildir_test(markdownrenderer) add_qtmaildir_test(translations) # Asserts on the tracked .ts rather than the generated .qm: an untranslated # string is dropped by lrelease, so it is invisible in the .qm and shows up # only as English in a running Italian UI. target_compile_definitions(test_translations PRIVATE TRANSLATIONS_DIR="${CMAKE_SOURCE_DIR}/translations") -add_qtmaildir_test(markdownrenderer) diff --git a/tests/test_markdownrenderer.cpp b/tests/test_markdownrenderer.cpp index 0c35b6c..697a28f 100644 --- a/tests/test_markdownrenderer.cpp +++ b/tests/test_markdownrenderer.cpp @@ -74,7 +74,10 @@ void TestMarkdownRenderer::tasklistRenders() const QString html = MarkdownRenderer::toHtml( QStringLiteral("- [ ] todo\n- [x] done")); QVERIFY2(html.contains(QStringLiteral("type=\"checkbox\"")), qPrintable(html)); - QVERIFY2(html.contains(QStringLiteral("checked")), qPrintable(html)); + // Not a bare "checked": that is a common English word ordinary prose + // would satisfy on its own. The attribute is what proves [x] differs + // from [ ]. + QVERIFY2(html.contains(QStringLiteral("checked=\"\"")), qPrintable(html)); } void TestMarkdownRenderer::tablesAreNotEnabled() @@ -127,7 +130,11 @@ void TestMarkdownRenderer::accentedTextSurvivesAsUtf8() // This user writes Italian, so accented text is every message rather // than an edge case, and a UTF-8 round trip through a C library is // exactly where it would be lost. - const QString source = QString::fromUtf8("perch\xC3\xA9 \xC3\xA8 cos\xC3\xAC"); + // + // Includes a character outside latin-1, so a symmetric toLatin1/fromLatin1 + // substitution cannot round-trip it and cancel itself out. Measured: with + // accented latin-1 text alone, mutating both sides together passes. + const QString source = QString::fromUtf8("perch\xC3\xA9 \xC3\xA8 cos\xC3\xAC \xE2\x82\xAC"); const QString html = MarkdownRenderer::toHtml(source); QVERIFY2(html.contains(source), qPrintable(html)); } -- cgit v1.2.3