fix: improve URL handling in MarkdownBlock and update tests for trailing punctuation

Author: daniel-lxs

webview-ui/src/components/common/MarkdownBlock.tsx

@@ -28,7 +28,7 @@ const remarkUrlToLink = () => {
 			const urlRegex = /https?:\/\/[^\s<>)"]+/g
 			const matches = node.value.match(urlRegex)
 
-			if (!matches) {
+			if (!matches || !parent) {
 				return
 			}
 
@@ -45,25 +45,32 @@ const remarkUrlToLink = () => {
 					const originalUrl = matches[i]
 					const cleanedUrl = cleanedMatches[i]
 					const removedPunctuation = originalUrl.substring(cleanedUrl.length)
-					
+
+					// Create a proper link node with all required properties
 					children.push({
 						type: "link",
 						url: cleanedUrl,
+						title: null,
 						children: [{ type: "text", value: cleanedUrl }],
+						data: {
+							hProperties: {
+								href: cleanedUrl,
+							},
+						},
 					})
-					
+
 					if (removedPunctuation) {
 						children.push({ type: "text", value: removedPunctuation })
 					}
 				}
 			})
 
-			// Fix: Instead of converting the node to a paragraph (which broke things),
-			// we replace the original text node with our new nodes in the parent's children array.
+			// Replace the original text node with our new nodes in the parent's children array.
 			// This preserves the document structure while adding our links.
-			if (parent) {
-				parent.children.splice(index, 1, ...children)
-			}
+			parent.children.splice(index!, 1, ...children)
+
+			// Return SKIP to prevent visiting the newly created nodes
+			return ["skip", index! + children.length]
 		})
 	}
 }
@@ -154,44 +161,42 @@ const MarkdownBlock = memo(({ markdown }: MarkdownBlockProps) => {
 		rehypePlugins: [],
 		rehypeReactOptions: {
 			components: {
-				a: ({ href, children }: any) => {
+				a: ({ href, children, ...props }: any) => {
+					const handleClick = (e: React.MouseEvent<HTMLAnchorElement>) => {
+						// Only process file:// protocol or local file paths
+						const isLocalPath = href.startsWith("file://") || href.startsWith("/") || !href.includes("://")
+
+						if (!isLocalPath) {
+							return
+						}
+
+						e.preventDefault()
+
+						// Handle absolute vs project-relative paths
+						let filePath = href.replace("file://", "")
+
+						// Extract line number if present
+						const match = filePath.match(/(.*):(\d+)(-\d+)?$/)
+						let values = undefined
+						if (match) {
+							filePath = match[1]
+							values = { line: parseInt(match[2]) }
+						}
+
+						// Add ./ prefix if needed
+						if (!filePath.startsWith("/") && !filePath.startsWith("./")) {
+							filePath = "./" + filePath
+						}
+
+						vscode.postMessage({
+							type: "openFile",
+							text: filePath,
+							values,
+						})
+					}
+
 					return (
-						<a
-							href={href}
-							title={href}
-							onClick={(e) => {
-								// Only process file:// protocol or local file paths
-								const isLocalPath =
-									href.startsWith("file://") || href.startsWith("/") || !href.includes("://")
-
-								if (!isLocalPath) {
-									return
-								}
-
-								e.preventDefault()
-
-								// Handle absolute vs project-relative paths
-								let filePath = href.replace("file://", "")
-
-								// Extract line number if present
-								const match = filePath.match(/(.*):(\d+)(-\d+)?$/)
-								let values = undefined
-								if (match) {
-									filePath = match[1]
-									values = { line: parseInt(match[2]) }
-								}
-
-								// Add ./ prefix if needed
-								if (!filePath.startsWith("/") && !filePath.startsWith("./")) {
-									filePath = "./" + filePath
-								}
-
-								vscode.postMessage({
-									type: "openFile",
-									text: filePath,
-									values,
-								})
-							}}>
+						<a {...props} href={href} onClick={handleClick}>
 							{children}
 						</a>
 					)

webview-ui/src/components/common/__tests__/MarkdownBlock.spec.tsx

@@ -18,12 +18,22 @@ vi.mock("@src/context/ExtensionStateContext", () => ({
 describe("MarkdownBlock", () => {
 	it("should correctly handle URLs with trailing punctuation", async () => {
 		const markdown = "Check out this link: https://example.com."
-		render(<MarkdownBlock markdown={markdown} />)
+		const { container } = render(<MarkdownBlock markdown={markdown} />)
 
-		await (async () => {
-			const linkElement = screen.getByRole("link")
-			expect(linkElement).toHaveAttribute("href", "https://example.com")
-			expect(linkElement.textContent).toBe("https://example.com.")
-		})
+		// Wait for the content to be processed
+		await screen.findByText(/Check out this link/, { exact: false })
+
+		// Check for nested links - this should not happen
+		const nestedLinks = container.querySelectorAll("a a")
+		expect(nestedLinks.length).toBe(0)
+
+		// Should have exactly one link
+		const linkElement = screen.getByRole("link")
+		expect(linkElement).toHaveAttribute("href", "https://example.com")
+		expect(linkElement.textContent).toBe("https://example.com")
+
+		// Check that the period is outside the link
+		const paragraph = container.querySelector("p")
+		expect(paragraph?.textContent).toBe("Check out this link: https://example.com.")
 	})
 })