diff --git a/shell/plugins/notifications/NotificationLogic.js b/shell/plugins/notifications/NotificationLogic.js index b3f7be09..b5a6f3ea 100644 --- a/shell/plugins/notifications/NotificationLogic.js +++ b/shell/plugins/notifications/NotificationLogic.js @@ -77,6 +77,18 @@ function stripImageTags(text) { return out } +// What the card renders, and the last thing to touch the string before Qt parses +// it. The newline rewrite belongs here rather than in the card because it inserts +// `
` into text stripImageTags chose to KEEP, and a kept tag may hold a `<` of +// its own: `` is one tag named `x` to both the +// stripper and Qt, until the rewrite splits it into `` and a live image tag +// the input never contained. Measured against Qt 6.11.2 — the rewritten form +// fetches, the original does not. So strip again after, and what Qt parses is what +// was checked last. +function styledBody(body, app, appIcon) { + return stripImageTags(sanitizeBody(body, app, appIcon).replace(/\r\n|\r|\n/g, "
")) +} + function sanitizeBody(body, app, appIcon) { var text = stripImageTags(String(body || "")) if (!isChromiumDerived(app, appIcon)) return text @@ -438,6 +450,7 @@ if (typeof module !== "undefined") { module.exports = { isChromiumDerived: isChromiumDerived, sanitizeBody: sanitizeBody, + styledBody: styledBody, summaryStartsWithGlyph: summaryStartsWithGlyph, shouldBypassDnd: shouldBypassDnd, isEphemeralApp: isEphemeralApp, diff --git a/shell/plugins/notifications/components/NotificationCard.qml b/shell/plugins/notifications/components/NotificationCard.qml index 1171ddc4..64e3870b 100644 --- a/shell/plugins/notifications/components/NotificationCard.qml +++ b/shell/plugins/notifications/components/NotificationCard.qml @@ -44,7 +44,7 @@ BorderSurface { readonly property bool singleLineToast: sanitizedBody.length === 0 readonly property bool collapseRedundantIcon: singleLineToast && !hasGlyph && summaryStartsWithGlyph readonly property string sanitizedBody: sanitizeBody(body) - readonly property string styledBody: sanitizedBody.replace(/\r\n|\r|\n/g, "
") + readonly property string styledBody: NotificationLogic.styledBody(body, app, appIcon) readonly property color dimColor: Qt.darker(Color.notifications.text, 1.4) readonly property color bodyColor: Qt.darker(Color.notifications.text, 1.15) diff --git a/test/shell.d/notifications-test.sh b/test/shell.d/notifications-test.sh index 6dfc54a8..f06e0481 100644 --- a/test/shell.d/notifications-test.sh +++ b/test/shell.d/notifications-test.sh @@ -44,8 +44,12 @@ function survivingTagNames(text) { return names } +// Assert on styledBody, not sanitizeBody: styledBody is the string the card +// binds to the StyledText, so it is the only one Qt ever parses. Checking the +// sanitizer's output instead would pass a body whose surviving tag the newline +// rewrite later splits open. function assertNoImageSurvives(body, description) { - const out = notifications.sanitizeBody(body, 'Slack', '') + const out = notifications.styledBody(body, 'Slack', '') const names = survivingTagNames(out) assert( !names.includes('img'), @@ -99,6 +103,48 @@ assertNoImageSurvives( 'notifications leave no image tag when U+0085 follows the angle bracket' ) +// The card rewrites newlines to
for the StyledText, which puts tag syntax +// inside a tag the stripper kept: `` is one tag named `x` +// to both the stripper and Qt, and the rewrite splits it into `` and a +// live image tag. Measured against Qt 6.11.2 — the rewritten form issues the GET +// and the original does not — so the strip has to run after the rewrite, which +// is what styledBody() does. +assertNoImageSurvives( + '', + 'notifications leave no image tag when a newline rewrite splits a kept tag' +) + +assertNoImageSurvives( + '', + 'notifications leave no image tag when a CRLF rewrite splits a kept tag' +) + +assertEqual( + notifications.styledBody('', 'Slack', ''), + '', + 'notifications drop the image half of a tag the newline rewrite splits' +) + +// The rewrite itself still happens, and body markup other than images survives it. +assertEqual( + notifications.styledBody('bold\nsecond line', 'Slack', ''), + 'bold
second line', + 'notifications keep body markup and the line break the card renders' +) + +// The order above is only worth anything if the card actually renders it, and no +// JavaScript assertion can see a QML binding. Pin the binding itself: the rewrite +// belongs in the logic module, where the strip runs after it. +const cardQml = fs.readFileSync(path.join(root, 'shell/plugins/notifications/components/NotificationCard.qml'), 'utf8') +assert( + /readonly property string styledBody: NotificationLogic\.styledBody\(body, app, appIcon\)/.test(cardQml), + 'the notification card renders the body that was stripped after the newline rewrite' +) +assert( + !//.test(cardQml), + 'the notification card does not rewrite newlines itself, which would leave tag syntax unchecked' +) + assertEqual( notifications.sanitizeBody('trailing