-
-
Notifications
You must be signed in to change notification settings - Fork 7
Description
🐛 Bug
The fixUrls() function in lib/src/Utils/ReplaceURLs.ts has two related bugs that corrupt text formatting when processing URLs.
Bug 1: Leading Space Consumption
Description: When fixUrls() processes a URL, it removes the space that precedes the URL, causing words to merge together.
Current Behavior:
fixUrls('Visit example.com')
// Returns: 'Visithttps://example.com'
// Expected: 'Visit https://example.com'Root Cause: Line 8 in ReplaceURLs.ts:
let hyperlink = url.replace(' ', '') // Removes space
return hyperlink // Returns without restoring spaceThe regex captures a leading space with (^| ) to distinguish naked URLs from text, but the replacement callback removes this space and never restores it.
Bug 2: End-of-Line Only Matching
Description: The urlRegex uses a $ (end of line) anchor, causing it to only match the last URL on each line. Multiple URLs per line are not all processed.
Current Behavior:
fixUrls('First https://a.com then https://b.com')
// Returns: 'First https://a.com thenhttps://b.com'
// Only the second URL is matched (and space consumed)
// Expected: Both URLs should be left unchanged (already have https://)Root Cause: Line 3 in ReplaceURLs.ts:
const urlRegex = /(^| )(http:\/\/www\.|https:\/\/www\.|http:\/\/|https:\/\/)?[a-z0-9]+([\-\.]{1}[a-z0-9]+)*\.[a-z]{2,10}(:[0-9]{1,10})?(\/.*)?$/gm
// ^ This $ limits to end of lineImpact Assessment
Current practical impact is LOW because:
fixUrls()is primarily used inpreprocessMarkdown.tswhich immediately follows withconvertNakedUrls()that uses a different, working regex- Video autolinks
<url>are not affected (the>prevents matching) - URLs that already have
https://are processed but the output is still valid
However, the bugs could cause issues if:
fixUrls()is used elsewhere in isolation- Users type multiple naked URLs on the same line
- Text formatting matters for display before markdown conversion
Git History
- Introduced in commit
bbe755a0(Aug 30, 2023): "ItemViewPopup TextView styling / url formatting" - The original
replaceURLs()function returned<a>tags that included the space in the visible text - The refactor to
fixUrls()broke the space preservation
Suggested Fix
Option A: Fix the function (minimal change)
export function fixUrls(message: string): string {
message = message.replace(urlRegex, function (url) {
// Preserve leading space if present
const hasLeadingSpace = url.startsWith(' ')
let hyperlink = url.trim()
if (!/^https?:\/\//.exec(hyperlink)) {
hyperlink = 'https://' + hyperlink
}
// Restore leading space
return hasLeadingSpace ? ' ' + hyperlink : hyperlink
})
return message
}Option B: Fix regex to match multiple URLs per line
Remove the $ anchor and adjust the regex to properly terminate URL matching:
export const urlRegex =
/(^| )(https?:\/\/)?[a-z0-9]+([\-\.]{1}[a-z0-9]+)*\.[a-z]{2,10}(:[0-9]{1,10})?(\/[^\s]*)?(?=\s|$)/gimOption C: Remove fixUrls() from preprocessing pipeline
Since convertNakedUrls() in preprocessMarkdown.ts uses its own working regex, consider whether fixUrls() is still needed in the pipeline at all.
Reproduction Steps
- Call
fixUrls('Visit example.com') - Observe output is
'Visithttps://example.com'(missing space)
Or:
- Call
fixUrls('Check a.com and b.com') - Observe only
b.comgetshttps://prefix
Environment
- File:
lib/src/Utils/ReplaceURLs.ts - Discovered during: TipTap migration testing
- Original commit:
bbe755a0(Aug 2023)