Skip to content

fix: encode unsafe characters in markdown link hrefs#52

Merged
iamsivin merged 4 commits into
mainfrom
fix/link-href-markdown-encoding
Jun 11, 2026
Merged

fix: encode unsafe characters in markdown link hrefs#52
iamsivin merged 4 commits into
mainfrom
fix/link-href-markdown-encoding

Conversation

@iamsivin

@iamsivin iamsivin commented Jun 4, 2026

Copy link
Copy Markdown
Member

Description

The PR fixes links with spaces or parentheses in the URL being rendered as raw markdown ([label](url)) after reopening the editor or on the help center.

Cause

The serialized markdown was invalid. CommonMark does not allow raw whitespace or unescaped parentheses inside link destinations, causing downstream renderers to fail parsing the link.

Solution

Percent-encode whitespace and escape parentheses when serializing hrefs. URLs with whitespace now use the standard [label](url) format instead of autolinks, ensuring links survive markdown serialization and rendering correctly.

@sony-mathew sony-mathew left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iamsivin can you take a look at this review comment:

Findings
[P1] src/schema/markdown/serializer.js:225 still serializes table-cell links with raw mark.attrs.href.
The PR fixes the shared link mark serializer, but serializeCellContent has its own link branch:
[text](${mark.attrs.href})

So links inside article tables still emit invalid markdown when the href has spaces or parentheses. I verified this against PR head a5a2e3f: a table link with https://example.com/a path/foo(bar) serializes as:

| [inside](https://example.com/a path/foo(bar)) | plain |

Parsing that back splits the href at the space and leaves literal markdown in the cell. Chatwoot’s help-center renderer also renders it literally inside the table header, so the PR’s stated help-center fix is incomplete for tables. The fix should reuse the same href escaping/encoding helper in both the shared link.close path and the table-cell link branch.

@iamsivin

Copy link
Copy Markdown
Member Author

@iamsivin can you take a look at this review comment:

Findings [P1] src/schema/markdown/serializer.js:225 still serializes table-cell links with raw mark.attrs.href. The PR fixes the shared link mark serializer, but serializeCellContent has its own link branch: [text](${mark.attrs.href})

So links inside article tables still emit invalid markdown when the href has spaces or parentheses. I verified this against PR head a5a2e3f: a table link with https://example.com/a path/foo(bar) serializes as:

| [inside](https://example.com/a path/foo(bar)) | plain |

Parsing that back splits the href at the space and leaves literal markdown in the cell. Chatwoot’s help-center renderer also renders it literally inside the table header, so the PR’s stated help-center fix is incomplete for tables. The fix should reuse the same href escaping/encoding helper in both the shared link.close path and the table-cell link branch.

@sony-mathew Handled

@iamsivin iamsivin merged commit 836f9e9 into main Jun 11, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants