Skip to content

Commit b868f1d

Browse files
authored
Fix formatting behavior around <p> tags (#799)
<!-- Please read https://github.com/SableClient/Sable/blob/dev/CONTRIBUTING.md before submitting your pull request --> ### Description <!-- Please include a summary of the change. Please also include relevant motivation and context. List any dependencies that are required for this change. --> A few fixes: - When emotes are sent (/me), the leading paragraph block is always dropped so that the action doesn't appear on a new line, subsequent blocks are still sent. - For single line messages, paragraph tags are stripped. - For multiline messages, there's two behaviors, which distinguish the space between lines Sable (and some other clients probably) display. - Single new line = \n - Two new lines = \<p> tag #### Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ### Checklist: - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings ### AI disclosure: - [x] Partially AI assisted (clarify which code was AI assisted and briefly explain what it does). - [ ] Fully AI generated (explain what all the generated code does in moderate detail). <!-- Write any explanation required here, but do not generate the explanation using AI!! You must prove you understand what the code in this PR does. --> Tests were AI generated
2 parents 6c6370d + 1933230 commit b868f1d

9 files changed

Lines changed: 118 additions & 21 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
default: patch
3+
---
4+
5+
Drops paragraph tags when messages are only a single paragraph, use markdown (two new lines) to define a new paragraph rather than a line break.

src/app/components/editor/output.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ export type OutputOptions = {
1919
* a map of regex patterns to replace nicknames with, used when stripNickname is true
2020
*/
2121
nickNameReplacement?: Map<RegExp, string>;
22+
/** When true, markdown HTML omits the leading `<p>` wrapper (for `m.emote` / `/me`). */
23+
forEmote?: boolean;
2224
};
2325

2426
const textToCustomHtml = (node: Text): string => sanitizeText(node.text);
@@ -86,13 +88,13 @@ export const toMatrixCustomHTML = (
8688
}
8789
markdownLines += line;
8890
if (index === targetNodes.length - 1) {
89-
const html = markdownToHtml(markdownLines);
91+
const html = markdownToHtml(markdownLines, { emote: opts.forEmote });
9092
return injectDataMd(html);
9193
}
9294
return '';
9395
}
9496

95-
const parsedMarkdown = markdownToHtml(markdownLines);
97+
const parsedMarkdown = markdownToHtml(markdownLines, { emote: opts.forEmote });
9698
markdownLines = '';
9799
return `${parsedMarkdown}${toMatrixCustomHTML(n, opts)}`;
98100
};

src/app/features/room/RoomInput.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -777,6 +777,7 @@ export const RoomInput = forwardRef<HTMLDivElement, RoomInputProps>(
777777
toMatrixCustomHTML(serializedChildren, {
778778
stripNickname: true,
779779
nickNameReplacement: nicknameReplacement,
780+
forEmote: commandName === Command.Me,
780781
})
781782
);
782783

@@ -851,6 +852,7 @@ export const RoomInput = forwardRef<HTMLDivElement, RoomInputProps>(
851852
toMatrixCustomHTML(serializedChildren, {
852853
stripNickname: true,
853854
nickNameReplacement: nicknameReplacement,
855+
forEmote: commandName === Command.Me,
854856
})
855857
);
856858
}

src/app/features/room/message/MessageEditor.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,11 @@ export const MessageEditor = as<'div', MessageEditorProps>(
170170
const [saveState, save] = useAsyncCallback(
171171
useCallback(async () => {
172172
const oldContent = mEvent.getContent();
173+
const msgtype = mEvent.getContent().msgtype as RoomMessageTextEventContent['msgtype'];
173174
let plainText = toPlainText(editor.children).trim();
174-
let customHtml = trimCustomHtml(toMatrixCustomHTML(editor.children, {}));
175+
let customHtml = trimCustomHtml(
176+
toMatrixCustomHTML(editor.children, { forEmote: msgtype === MsgType.Emote })
177+
);
175178

176179
const [prevBody, prevCustomHtml, prevMentions] = getPrevBodyAndFormattedBody();
177180

@@ -192,8 +195,6 @@ export const MessageEditor = as<'div', MessageEditorProps>(
192195
}
193196
}
194197

195-
const msgtype = mEvent.getContent().msgtype as RoomMessageTextEventContent['msgtype'];
196-
197198
const newContent: IContent = {
198199
msgtype,
199200
body: plainText,

src/app/plugins/markdown/htmlToMarkdown.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,10 @@ describe('htmlToMarkdown', () => {
111111
expect(result).toContain('\\*');
112112
});
113113

114+
it('inserts a blank line between adjacent paragraphs for editor round-trip', () => {
115+
expect(htmlToMarkdown('<p>First</p><p>Second</p>')).toBe('First\n\nSecond');
116+
});
117+
114118
it('encodes mx emoticons as private-use placeholders instead of literal img snippets', () => {
115119
const src = 'mxc://matrix.org/emote';
116120
const html = `<p>hi<img data-mx-emoticon src="${src}" alt="blobcat" title="blobcat" height="32" />bye</p>`;

src/app/plugins/markdown/htmlToMarkdown.ts

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -45,19 +45,36 @@ function isBlockTag(node: ChildNode | undefined): boolean {
4545
}
4646

4747
function processNodes(nodes: ChildNode[]): string {
48-
return nodes
49-
.filter((n, i) => {
50-
if (isText(n) && /^\s*$/.test(n.data)) {
51-
const prev = nodes[i - 1];
52-
const next = nodes[i + 1];
53-
// Ignore whitespace between block tags or at the edges
54-
const isBetweenBlocks = (!prev || isBlockTag(prev)) && (!next || isBlockTag(next));
55-
if (isBetweenBlocks) return false;
56-
}
57-
return true;
58-
})
59-
.map((n) => processNode(n))
60-
.join('');
48+
const filtered = nodes.filter((n, i) => {
49+
if (isText(n) && /^\s*$/.test(n.data)) {
50+
const prev = nodes[i - 1];
51+
const next = nodes[i + 1];
52+
// Ignore whitespace between block tags or at the edges
53+
const isBetweenBlocks = (!prev || isBlockTag(prev)) && (!next || isBlockTag(next));
54+
if (isBetweenBlocks) return false;
55+
}
56+
return true;
57+
});
58+
59+
const parts: string[] = [];
60+
for (let i = 0; i < filtered.length; i += 1) {
61+
const cur = filtered[i]!;
62+
const prev = filtered[i - 1];
63+
// Adjacent <p> blocks must become \n\n in markdown so the editor gets separate Slate
64+
// paragraphs and marked emits <p> per block again on send (single \n would collapse).
65+
if (
66+
i > 0 &&
67+
prev &&
68+
isTag(prev) &&
69+
isTag(cur) &&
70+
prev.name.toLowerCase() === 'p' &&
71+
cur.name.toLowerCase() === 'p'
72+
) {
73+
parts.push('\n');
74+
}
75+
parts.push(processNode(cur));
76+
}
77+
return parts.join('');
6178
}
6279

6380
function processNode(node: ChildNode, listDepth: number = 0, insideCode: boolean = false): string {

src/app/plugins/markdown/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
export * from './utils';
2-
export { markdownToHtml } from './markdownToHtml';
2+
export { markdownToHtml, type MarkdownToHtmlOptions } from './markdownToHtml';
33
export { htmlToMarkdown, injectDataMd } from './markdownPipeline';

src/app/plugins/markdown/markdownToHtml.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,43 @@ describe('markdownToHtml', () => {
143143
expect(result).toContain('Plain text');
144144
});
145145

146+
it('omits a single outer paragraph wrapper for one-paragraph messages', () => {
147+
expect(markdownToHtml('Hello')).toBe('Hello');
148+
expect(markdownToHtml('**bold**')).toBe('<strong>bold</strong>');
149+
expect(markdownToHtml('a<br>b')).toBe('a<br>b');
150+
});
151+
152+
it('keeps paragraph tags when there are multiple paragraphs', () => {
153+
const result = markdownToHtml('First\n\nSecond');
154+
expect(result).toContain('<p>');
155+
expect(result).toContain('First');
156+
expect(result).toContain('Second');
157+
});
158+
159+
it('does not strip the first <p> when there are three paragraphs with extra blank lines', () => {
160+
const result = markdownToHtml('test\n\ntest 2\n\n\ntest 3');
161+
expect(result).toMatch(/^<p>test<\/p>/i);
162+
expect(result).toContain('<p>test 2</p>');
163+
expect(result).toContain('<p>test 3</p>');
164+
expect(result).not.toMatch(/^test<\/p>/i);
165+
});
166+
167+
it('with emote: strips only the first <p> when multiple paragraphs are present', () => {
168+
const result = markdownToHtml('first\n\nsecond', { emote: true });
169+
expect(result).toMatch(/^first<p>/i);
170+
expect(result).toContain('second');
171+
expect(result).not.toMatch(/^<p>first<\/p>\s*<p>second<\/p>$/i);
172+
});
173+
174+
it('with emote: still unwraps a single-paragraph body like the default path', () => {
175+
expect(markdownToHtml('**waves**', { emote: true })).toBe('<strong>waves</strong>');
176+
});
177+
146178
it('handles multiline content', () => {
147179
const result = markdownToHtml('Line 1\nLine 2\nLine 3');
148180
expect(result).toContain('Line 1');
149181
expect(result).toContain('Line 2');
182+
expect(result).toContain('Line 3');
150183
});
151184

152185
it('handles escaped markdown characters', () => {

src/app/plugins/markdown/markdownToHtml.ts

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,43 @@ const unshieldBareMatrixToLinks = (html: string, placeholders: Map<string, strin
8585
return result;
8686
};
8787

88+
/** When the whole message is a single paragraph, drop the redundant wrapper. */
89+
const unwrapSingleOuterParagraph = (html: string): string => {
90+
const trimmed = html.trim();
91+
const m = trimmed.match(/^<p\b[^>]*>([\s\S]*)<\/p>$/i);
92+
if (!m) return html;
93+
const inner = m[1] ?? '';
94+
if (/<\/p>/i.test(inner)) return html;
95+
return inner;
96+
};
97+
98+
/**
99+
* For `m.emote`, the sender display name is added at render time. Strips the leading `<p>…</p>`
100+
* block (when its inner HTML has no `</p>`) so we don't send `<p>` around the action.
101+
*/
102+
const stripLeadingEmoteParagraph = (html: string): string | null => {
103+
const trimmed = html.trim();
104+
const m = trimmed.match(/^<p\b[^>]*>([\s\S]*?)<\/p>/i);
105+
if (!m) return null;
106+
const inner = m[1] ?? '';
107+
if (/<\/p>/i.test(inner)) return null;
108+
const rest = trimmed.slice(m[0].length).trimStart();
109+
return rest.length > 0 ? `${inner}${rest}` : inner;
110+
};
111+
112+
export type MarkdownToHtmlOptions = {
113+
emote?: boolean;
114+
};
115+
88116
/**
89117
* Converts markdown string to sanitized Matrix-compatible HTML.
90118
* Uses marked for parsing and DOMPurify for sanitization per Matrix spec.
91119
*
92120
* @param markdown - Input markdown string
121+
* @param options - Optional; set `emote` for `/me` outgoing HTML
93122
* @returns Sanitized HTML string safe for Matrix client output
94123
*/
95-
export function markdownToHtml(markdown: string): string {
124+
export function markdownToHtml(markdown: string, options?: MarkdownToHtmlOptions): string {
96125
// Decode HTML entities so marked can properly parse markdown syntax
97126
// (e.g., &lt; becomes < for link URLs)
98127
const decoded = decodeHtmlEntities(markdown);
@@ -207,5 +236,9 @@ export function markdownToHtml(markdown: string): string {
207236
matrixToPlaceholders
208237
);
209238

210-
return unshieldedMatrixTo.replace(/<li>(<p><\/p>)?<\/li>/gi, '<li><br></li>');
239+
const listFixed = unshieldedMatrixTo.replace(/<li>(<p><\/p>)?<\/li>/gi, '<li><br></li>');
240+
if (options?.emote) {
241+
return stripLeadingEmoteParagraph(listFixed) ?? unwrapSingleOuterParagraph(listFixed);
242+
}
243+
return unwrapSingleOuterParagraph(listFixed);
211244
}

0 commit comments

Comments
 (0)