fix: stray backslash and vanishing signature separator in replies (#15639)
# Pull Request Template ## Description This PR fixes stray `\` characters and missing `--` signature separators when replies contain empty lines above the email signature. Empty lines and Shift+Enter line breaks are now preserved as composed across delivered emails, dashboard bubbles, text channels, and webhook payloads. ### https://github.com/chatwoot/prosemirror-schema/pull/59 ### What changed * Bump `@chatwoot/prosemirror-schema` to 1.4.5, which fixes markdown serialization of empty lines, hard breaks, and setext underline escaping at the source. * Strip the escaped signature delimiter cleanly when toggling or removing the signature, so it never leaves a trailing `\`. * Unescape the serializer's underline escape (`\--` → `--`) in `WebhookContentNormalizer`, so API-channel and webhook consumers receive the original text. * Add end-to-end rendering specs covering every editor output shape with 23 byte-exact fixtures across cmark, both email modes, and all channel renderers. Fixes https://linear.app/chatwoot/issue/CW-8062/email-replies-show-a-stray-backslash-instead-of-the-signature ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? ### How to reproduce On an email inbox with a message signature enabled, type a reply and press Enter at the end, leaving an empty line above the `--` separator, then send. The delivered email and email bubble show a stray `\` where the empty line should be, and the `--` separator disappears. ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules
This commit is contained in:
Sivin Varghese
1
.gitignore
vendored
1
.gitignore
vendored
@@ -104,3 +104,4 @@ CLAUDE.local.md
|
||||
.pnpm-store/*
|
||||
local/
|
||||
Procfile.worktree
|
||||
.nodeterm/project.json
|
||||
|
||||
@@ -128,8 +128,20 @@ export function cleanSignature(signature) {
|
||||
const stripDelimiterHardbreaks = body =>
|
||||
body.replace(/(--)\s*(?:\\\s*)+$/, '$1');
|
||||
|
||||
// Strip standalone blank-paragraph markers (`\` on their own lines).
|
||||
const stripTrailingBlankLine = body => body.replace(/\n(?:\s*\\\n)+$/, '');
|
||||
// Strip standalone blank-paragraph markers (`\` on their own lines), plus
|
||||
// the bare `\` the serializer writes to escape the `--` delimiter into `\--`
|
||||
// (left behind once the delimiter itself is sliced off). Two shapes end in that
|
||||
// escape after the slice: `hey\` LF `\` (Shift+Enter, the trailing `\` on the
|
||||
// content line is a hard-break marker and goes too) and `C:\` LF LF `\` LF `\`
|
||||
// (a blank line separates content from the glue; the serializer emits authored
|
||||
// backslashes verbatim, so a `\` before a blank line is user content and must
|
||||
// stay). A marker's newline is never blank, so the marker alternative only
|
||||
// crosses single newlines ([^\S\n] excludes `\n`) while the plain alternative
|
||||
// starts at the newline and may cross blank lines.
|
||||
const stripTrailingBlankLine = body =>
|
||||
body
|
||||
.replace(/(?:\\\n(?:[^\S\n]*\\\n)*[^\S\n]*\\|\n(?:\s*\\\n)*\s*\\)$/, '')
|
||||
.replace(/\n(?:\s*\\\n)+$/, '');
|
||||
|
||||
/**
|
||||
* Adds the signature delimiter to the beginning of the signature.
|
||||
|
||||
@@ -357,6 +357,22 @@ describe('removeSignature', () => {
|
||||
'hey\n\n'
|
||||
);
|
||||
});
|
||||
it('strips the hard-break marker with the escape when Shift+Enter precedes the delimiter', () => {
|
||||
expect(removeSignature('hey\\\n\\--\n\nHello there', 'Hello there')).toBe(
|
||||
'hey'
|
||||
);
|
||||
});
|
||||
it('keeps an authored trailing backslash when an empty line precedes the delimiter', () => {
|
||||
expect(removeSignature('C:\\\n\n\\\n\\--\n\nBest', 'Best')).toBe('C:\\');
|
||||
});
|
||||
it('strips only the hard-break marker when the content line ends with a backslash', () => {
|
||||
expect(removeSignature('C:\\\\\n\\--\n\nBest', 'Best')).toBe('C:\\');
|
||||
});
|
||||
it('keeps an authored trailing backslash through a Shift+Enter and empty line combo', () => {
|
||||
expect(removeSignature('C:\\\n\n\\\n\\\n\\--\n\nBest', 'Best')).toBe(
|
||||
'C:\\'
|
||||
);
|
||||
});
|
||||
it('preserves trailing backslash in user text when appending', () => {
|
||||
expect(appendSignature('The path is C:\\', 'Best\nAgent')).toContain(
|
||||
'C:\\'
|
||||
|
||||
@@ -174,4 +174,49 @@ After`;
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// Byte-exact editor serializer output for Enter / Shift+Enter combinations.
|
||||
// An empty line is stored as a `\` hard-break chain and an underline below
|
||||
// it is escaped (`\--`, `\-`, `\==`); the bubble must render no backslash
|
||||
// and keep every composed line.
|
||||
describe('editor break and empty-line encoding', () => {
|
||||
const signatureTail =
|
||||
'\n\nThanks \\\nSivin | Chatwoot\n\n';
|
||||
const cases = [
|
||||
['enter above signature', `hey\n\n\\\n\\--${signatureTail}`, 2],
|
||||
['shift+enter then -- on the same line', 'hey\\\n\\--', 1],
|
||||
['two breaks above signature', `hey\n\n\\\n\\\n\\--${signatureTail}`, 3],
|
||||
[
|
||||
'three enters and four breaks above signature',
|
||||
`sd\n\n\\\n\\\n\\\n\\\n\\\n\\\n\\--${signatureTail}`,
|
||||
7,
|
||||
],
|
||||
['equals underline below an empty line', 'a\n\n\\\n\\==', 1],
|
||||
['lone dash after a break', 'a\\\n\\-', 1],
|
||||
['lone dash below an empty line', 'x\n\n\\\n\\-', 1],
|
||||
['bold dashes after a break', 'a\\\n**--**', 1],
|
||||
['dashes led by a bold space below an empty line', 'a\n\n\n --', 0],
|
||||
['dashes trailed by a bold space', 'a\n\n\\\n\\-- ', 1],
|
||||
['empty line between paragraphs', 'a\n\n\\\nb', 1],
|
||||
['double break inside a paragraph', 'a\\\n\\\nb', 2],
|
||||
['plain hard break', 'line one\\\nline two', 1],
|
||||
];
|
||||
|
||||
it.each(cases)(
|
||||
'renders %s without backslash artifacts and keeps every line',
|
||||
(_name, message, breakCount) => {
|
||||
const { formattedMessage } = new MessageFormatter(message);
|
||||
expect(formattedMessage).not.toContain('\\');
|
||||
expect(formattedMessage).not.toMatch(/<h[12]/);
|
||||
expect(formattedMessage.match(/<br/g) || []).toHaveLength(breakCount);
|
||||
}
|
||||
);
|
||||
|
||||
it('keeps the signature delimiter as literal text below the empty line', () => {
|
||||
const { formattedMessage } = new MessageFormatter(
|
||||
`hey\n\n\\\n\\--${signatureTail}`
|
||||
);
|
||||
expect(formattedMessage).toContain('--</p>');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,11 +1,48 @@
|
||||
# Strips CommonMark hard line breaks from stored markdown source (backslash before newline).
|
||||
# ProseMirror / the dashboard editor emits this form so soft breaks survive as markdown;
|
||||
# webhook consumers expect plain newlines without a visible backslash (e.g. WhatsApp gateways).
|
||||
# Also strips trailing newlines introduced by TipTap/ProseMirror trailing paragraph nodes.
|
||||
# The editor also escapes a `-`/`--`/`==` line under a hard break as `\--` so cmark cannot
|
||||
# read the pair as a setext heading. That escape is only ever written as one `\<newline>\`
|
||||
# unit (glue backslash, newline, escape backslash), so it is unescaped only in that exact
|
||||
# context and never inside a code block, where the same byte sequence is user content.
|
||||
# Also strips trailing newlines introduced by TipTap/ProseMirror trailing paragraphs.
|
||||
class Messages::WebhookContentNormalizer
|
||||
DELIMITER_ESCAPE = /(\\\r?\n)\\(?=(?:-+|=+)[ \t]*\r?$)/
|
||||
ESCAPED_DELIMITER_LINE = /\A\\(?:-+|=+)[ \t]*\r?\n?\z/
|
||||
HARD_BREAK_ENDING = /\\\r?\n\z/
|
||||
|
||||
def self.normalize(text)
|
||||
return text if text.blank?
|
||||
|
||||
text.gsub(/\\\r?\n/, "\n").sub(/(\r?\n)+\z/, '')
|
||||
unescape_delimiters(text)
|
||||
.gsub(/\\\r?\n/, "\n")
|
||||
.sub(/(\r?\n)+\z/, '')
|
||||
end
|
||||
|
||||
def self.unescape_delimiters(text)
|
||||
return text unless text.match?(DELIMITER_ESCAPE)
|
||||
|
||||
code_lines = code_block_lines(text)
|
||||
lines = text.lines
|
||||
lines.each_with_index do |line, index|
|
||||
next unless index.positive? && line.match?(ESCAPED_DELIMITER_LINE)
|
||||
next unless lines[index - 1].match?(HARD_BREAK_ENDING)
|
||||
next if code_lines.include?(index + 1)
|
||||
|
||||
lines[index] = line.delete_prefix('\\')
|
||||
end
|
||||
lines.join
|
||||
end
|
||||
|
||||
def self.code_block_lines(text)
|
||||
lines = Set.new
|
||||
CommonMarker.render_doc(text).walk do |node|
|
||||
next unless node.type == :code_block
|
||||
|
||||
lines.merge(node.sourcepos[:start_line]..node.sourcepos[:end_line])
|
||||
end
|
||||
lines
|
||||
end
|
||||
|
||||
private_class_method :unescape_delimiters, :code_block_lines
|
||||
end
|
||||
|
||||
@@ -37,7 +37,7 @@
|
||||
"@amplitude/analytics-browser": "^2.11.10",
|
||||
"@breezystack/lamejs": "^1.2.7",
|
||||
"@chatwoot/ninja-keys": "1.2.3",
|
||||
"@chatwoot/prosemirror-schema": "1.4.4",
|
||||
"@chatwoot/prosemirror-schema": "1.4.5",
|
||||
"@chatwoot/utils": "^0.0.57",
|
||||
"@chatwoot/viz": "^0.1.5",
|
||||
"@formkit/core": "^1.7.2",
|
||||
|
||||
10
pnpm-lock.yaml
generated
10
pnpm-lock.yaml
generated
@@ -28,8 +28,8 @@ importers:
|
||||
specifier: 0.6.2
|
||||
version: 0.6.2
|
||||
'@chatwoot/prosemirror-schema':
|
||||
specifier: 1.4.4
|
||||
version: 1.4.4
|
||||
specifier: 1.4.5
|
||||
version: 1.4.5
|
||||
'@chatwoot/utils':
|
||||
specifier: ^0.0.57
|
||||
version: 0.0.57
|
||||
@@ -452,8 +452,8 @@ packages:
|
||||
'@chatwoot/pico-search@0.6.2':
|
||||
resolution: {integrity: sha512-uZnt/c06sN3epXv42uzweoqKLCY94s+h9vpbw/1SYCPTww4kCo18vUUD3QZ7tPPSBQB130Fjc79tLQMSWoGt4w==}
|
||||
|
||||
'@chatwoot/prosemirror-schema@1.4.4':
|
||||
resolution: {integrity: sha512-HwOEtzcOcX8mLgOeHMngNLLmVQFWJODbJw6yl7VArg1gsZv2gAsjbtqdyGNt0DTwleJsOwBZdNMNrSaAU8V7+A==}
|
||||
'@chatwoot/prosemirror-schema@1.4.5':
|
||||
resolution: {integrity: sha512-+s9aKNjoxbT57qs1VAElPfwx3KVO714H5A9sWTJ9E4wsX9pr3SXvs4acWChDcfprGFCm16ybe/XS0161Ax6raA==}
|
||||
|
||||
'@chatwoot/utils@0.0.57':
|
||||
resolution: {integrity: sha512-L4QrCmimkULY9ZQk0MJRLfwry3Pqq2e7psETqoZGYopMZ+8DSWe4t6cModSjTCBnDjqgp/gTl0oorntCYrDE8w==}
|
||||
@@ -5189,7 +5189,7 @@ snapshots:
|
||||
|
||||
'@chatwoot/pico-search@0.6.2': {}
|
||||
|
||||
'@chatwoot/prosemirror-schema@1.4.4':
|
||||
'@chatwoot/prosemirror-schema@1.4.5':
|
||||
dependencies:
|
||||
linkify-it: 5.0.2
|
||||
markdown-it: 14.3.0
|
||||
|
||||
88
spec/lib/chatwoot_markdown_renderer_editor_output_spec.rb
Normal file
88
spec/lib/chatwoot_markdown_renderer_editor_output_spec.rb
Normal file
@@ -0,0 +1,88 @@
|
||||
require 'rails_helper'
|
||||
|
||||
# Byte-exact markdown the dashboard editor (@chatwoot/prosemirror-schema)
|
||||
# emits for Enter / Shift+Enter combinations. An empty visual line is stored
|
||||
# as a `\` hard-break line glued to the next paragraph, and a dashes/equals
|
||||
# line below it is escaped (`\--`, `\-`, `\==`) so cmark cannot read the pair
|
||||
# as a setext heading. Every channel renderer must show no backslash, no
|
||||
# accidental heading, and keep the composed lines.
|
||||
describe ChatwootMarkdownRenderer do
|
||||
signature_tail = "\n\nThanks \\\nSivin | Chatwoot\n\n"
|
||||
|
||||
fixtures = {
|
||||
enter_above_signature: { markdown: "hey\n\n\\\n\\--#{signature_tail}", email_breaks: 2, whatsapp: "hey\n\n\n--\n\nThanks \nSivin | Chatwoot" },
|
||||
shift_enter_then_dashes_same_line: { markdown: "hey\\\n\\--", email_breaks: 1, whatsapp: "hey\n--" },
|
||||
multiple_breaks_above_signature: { markdown: "hey\n\n\\\n\\\n\\--#{signature_tail}", email_breaks: 3,
|
||||
whatsapp: "hey\n\n\n\n--\n\nThanks \nSivin | Chatwoot" },
|
||||
mixed_enter_and_breaks: { markdown: "mix\n\n\\\n\\\n\\--#{signature_tail}", email_breaks: 3,
|
||||
whatsapp: "mix\n\n\n\n--\n\nThanks \nSivin | Chatwoot" },
|
||||
three_enters_four_breaks: { markdown: "sd\n\n\\\n\\\n\\\n\\\n\\\n\\\n\\--#{signature_tail}", email_breaks: 7,
|
||||
whatsapp: "sd\n\n\n\n\n\n\n\n--\n\nThanks \nSivin | Chatwoot" },
|
||||
equals_underline_below_empty: { markdown: "a\n\n\\\n\\==", email_breaks: 1, whatsapp: "a\n\n\n==" },
|
||||
empty_line_between_text: { markdown: "a\n\n\\\nb", email_breaks: 1, whatsapp: "a\n\n\nb" },
|
||||
two_empty_lines_between_text: { markdown: "a\n\n\\\n\\\nb", email_breaks: 2, whatsapp: "a\n\n\n\nb" },
|
||||
mid_paragraph_double_break: { markdown: "a\\\n\\\nb", email_breaks: 2, whatsapp: "a\n\nb" },
|
||||
plain_hard_break: { markdown: "line one\\\nline two", email_breaks: 1, whatsapp: "line one\nline two" },
|
||||
legacy_space_padded_breaks: { markdown: "zz \n\n\\\n\\\n\\--#{signature_tail}", email_breaks: 3,
|
||||
whatsapp: "zz \n\n\n\n--\n\nThanks \nSivin | Chatwoot" },
|
||||
break_then_list_text: { markdown: "intro\n- item", email_breaks: 0, whatsapp: "intro\n- item" },
|
||||
break_then_thematic: { markdown: "intro\n***", email_breaks: 0, whatsapp: 'intro' },
|
||||
empty_before_pasted_list_text: { markdown: "a\n\n\n- pasted", email_breaks: 0, whatsapp: "a\n\n\n- pasted" },
|
||||
empty_before_list_node: { markdown: "a\n\n\n* item", email_breaks: 0, whatsapp: "a\n\n\n* item" },
|
||||
empty_before_image: { markdown: "a\n\n", email_breaks: 0, whatsapp: 'a' },
|
||||
trailing_breaks_no_signature: { markdown: 'hello', email_breaks: 0, whatsapp: 'hello' },
|
||||
dash_after_break: { markdown: "a\\\n\\-", email_breaks: 1, whatsapp: "a\n-" },
|
||||
dash_below_empty: { markdown: "x\n\n\\\n\\-", email_breaks: 1, whatsapp: "x\n\n\n-" },
|
||||
dash_above_signature: { markdown: "e1\\\n\\-\n\n--#{signature_tail}", email_breaks: 2,
|
||||
whatsapp: "e1\n-\n\n--\n\nThanks \nSivin | Chatwoot" },
|
||||
bare_marker_after_break: { markdown: "e2\\\n\\- \n\n--#{signature_tail}", email_breaks: 2,
|
||||
whatsapp: "e2\n- \n\n--\n\nThanks \nSivin | Chatwoot" },
|
||||
bold_underline_after_break: { markdown: "a\\\n**--**", email_breaks: 1, whatsapp: "a\n*--*",
|
||||
email_delimiter: '<strong>--</strong></p>' },
|
||||
indented_underline_detached: { markdown: "b2\n\n --", email_breaks: 0, whatsapp: "b2\n\n --" },
|
||||
dashes_led_by_bold_space: { markdown: "a\n\n\n --", email_breaks: 0, whatsapp: "a\n\n\n --" },
|
||||
dashes_trailed_by_bold_space: { markdown: "a\n\n\\\n\\-- ", email_breaks: 1, whatsapp: "a\n\n\n--" }
|
||||
}
|
||||
|
||||
html_channels = %w[Channel::Email Channel::WebWidget]
|
||||
text_channels = %w[Channel::Telegram Channel::Whatsapp Channel::Instagram Channel::Line Channel::Sms]
|
||||
hardbreak_modes = [true, false].freeze
|
||||
|
||||
fixtures.each do |name, fixture|
|
||||
context "with #{name.to_s.tr('_', ' ')}" do
|
||||
it 'renders without backslash artifacts or accidental headings on every channel' do
|
||||
hardbreak_modes.each do |hardbreaks|
|
||||
html = described_class.new(fixture[:markdown]).render_message(hardbreaks: hardbreaks).to_s
|
||||
expect(html).not_to include('\\'), "render_message(hardbreaks: #{hardbreaks}) leaked a backslash: #{html.inspect}"
|
||||
expect(html).not_to match(/<h[12]/), "render_message(hardbreaks: #{hardbreaks}) produced a heading: #{html.inspect}"
|
||||
end
|
||||
|
||||
(html_channels + text_channels).each do |channel|
|
||||
rendered = Messages::MarkdownRendererService.new(fixture[:markdown], channel).render
|
||||
expect(rendered).not_to include('\\'), "#{channel} leaked a backslash: #{rendered.inspect}"
|
||||
expect(rendered).not_to match(/<h[12]/), "#{channel} produced a heading: #{rendered.inspect}"
|
||||
end
|
||||
end
|
||||
|
||||
it 'preserves the composed lines in the email html' do
|
||||
html = described_class.new(fixture[:markdown]).render_message(hardbreaks: true).to_s
|
||||
expect(html.scan('<br').size).to eq(fixture[:email_breaks])
|
||||
delimiter = fixture.fetch(:email_delimiter) { fixture[:markdown].include?('--') ? '--</p>' : nil }
|
||||
expect(html).to include(delimiter) if delimiter
|
||||
end
|
||||
|
||||
it 'preserves the composed lines as plain newlines on text channels' do
|
||||
rendered = Messages::MarkdownRendererService.new(fixture[:markdown], 'Channel::Whatsapp').render
|
||||
expect(rendered.rstrip).to eq(fixture[:whatsapp])
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'with the email helper used to build stored email html' do
|
||||
it 'renders the signature shape with the empty line and delimiter intact' do
|
||||
html = Class.new { include EmailHelper }.new.render_email_html("hey\n\n\\\n\\--#{signature_tail}")
|
||||
expect(html).to include("<p><br />\n--</p>")
|
||||
expect(html).not_to include('\\')
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -37,5 +37,41 @@ RSpec.describe Messages::WebhookContentNormalizer do
|
||||
it 'handles both hard line breaks and trailing newlines together' do
|
||||
expect(described_class.normalize("hello\\\nworld\n\n\n")).to eq("hello\nworld")
|
||||
end
|
||||
|
||||
it 'unescapes the signature delimiter the editor escapes below an empty line' do
|
||||
expect(described_class.normalize("hey\n\n\\\n\\--\n\nThanks")).to eq("hey\n\n\n--\n\nThanks")
|
||||
end
|
||||
|
||||
it 'unescapes an equals underline the same way' do
|
||||
expect(described_class.normalize("a\n\n\\\n\\==")).to eq("a\n\n\n==")
|
||||
end
|
||||
|
||||
it 'unescapes a lone dash underline' do
|
||||
expect(described_class.normalize("a\\\n\\-")).to eq("a\n-")
|
||||
end
|
||||
|
||||
it 'keeps backslashes that are not underline escapes' do
|
||||
expect(described_class.normalize('path C:\\temp and a \\- dash')).to eq('path C:\\temp and a \\- dash')
|
||||
end
|
||||
|
||||
it 'keeps a literal escaped delimiter inside a fenced code block' do
|
||||
expect(described_class.normalize("```\n\\--\n```")).to eq("```\n\\--\n```")
|
||||
end
|
||||
|
||||
it 'keeps a fenced escaped delimiter even when it follows a code line ending in a backslash' do
|
||||
expect(described_class.normalize("```text\ncontinued \\\n\\--\n```")).to eq("```text\ncontinued \n\\--\n```")
|
||||
end
|
||||
|
||||
it 'keeps an escaped delimiter that does not follow the editor hard-break glue' do
|
||||
expect(described_class.normalize("hey\n\n\\--")).to eq("hey\n\n\\--")
|
||||
end
|
||||
|
||||
it 'normalizes hard breaks around a fenced code block without touching the escape inside' do
|
||||
expect(described_class.normalize("a\\\nb\n~~~\n\\==\n~~~\nc\\\nd")).to eq("a\nb\n~~~\n\\==\n~~~\nc\nd")
|
||||
end
|
||||
|
||||
it 'does not misread an inline-code line while normalizing what follows' do
|
||||
expect(described_class.normalize("```foo```\nhey\\\nthere")).to eq("```foo```\nhey\nthere")
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user