Преглед изворни кода

fix: TaskItem display and copy issues with HTML tags in task messages. (#4444)

* fix: TaskItem display and copy issues with HTML tags in task messages.
* Display issue: HTML tags not displaying in HistoryView.
* Copy issue: HTML tags in message are removed when clicking the copy button in TaskItem.

* perf(webview-ui): add LRU cache to escapeHtml function

- Add LRU cache with 500 item limit for escapeHtml results
- Improves performance by caching frequently escaped strings
- Reduces redundant HTML escaping operations

---------

Co-authored-by: Jimmycai <[email protected]>
Co-authored-by: Daniel Riccio <[email protected]>
forestyoo пре 6 месеци
родитељ
комит
d5f862a6e0

+ 2 - 16
webview-ui/src/components/history/CopyButton.tsx

@@ -10,19 +10,6 @@ type CopyButtonProps = {
 	className?: string
 }
 
-/**
- * Strips only history highlight spans from text while preserving other HTML
- * Targets: <span class="history-item-highlight">content</span>
- * @param text - Text that may contain highlight spans
- * @returns Text with highlight spans removed but content preserved
- */
-const stripHistoryHighlightSpans = (text: string): string => {
-	// Match opening tag, capture content until closing tag
-	// The [\s\S]*? pattern matches any character (including newlines) non-greedily,
-	// which properly handles content with < characters
-	return text.replace(/<span\s+class="history-item-highlight">([\s\S]*?)<\/span>/g, "$1")
-}
-
 export const CopyButton = ({ itemTask, className }: CopyButtonProps) => {
 	const { isCopied, copy } = useClipboard()
 	const { t } = useAppTranslation()
@@ -30,10 +17,9 @@ export const CopyButton = ({ itemTask, className }: CopyButtonProps) => {
 	const onCopy = useCallback(
 		(e: React.MouseEvent) => {
 			e.stopPropagation()
+
 			if (!isCopied) {
-				// Strip only history highlight spans before copying to clipboard
-				const cleanText = stripHistoryHighlightSpans(itemTask)
-				copy(cleanText)
+				copy(itemTask)
 			}
 		},
 		[isCopied, copy, itemTask],

+ 7 - 3
webview-ui/src/components/history/TaskItem.tsx

@@ -9,8 +9,12 @@ import { useAppTranslation } from "@/i18n/TranslationContext"
 import TaskItemHeader from "./TaskItemHeader"
 import TaskItemFooter from "./TaskItemFooter"
 
+interface DisplayHistoryItem extends HistoryItem {
+	highlight?: string
+}
+
 interface TaskItemProps {
-	item: HistoryItem
+	item: DisplayHistoryItem
 	variant: "compact" | "full"
 	showWorkspace?: boolean
 	isSelectionMode?: boolean
@@ -103,8 +107,8 @@ const TaskItem = ({
 							overflowWrap: "anywhere",
 						}}
 						data-testid={isCompact ? undefined : "task-content"}
-						{...(isCompact ? {} : { dangerouslySetInnerHTML: { __html: item.task } })}>
-						{isCompact ? item.task : undefined}
+						{...(item.highlight ? { dangerouslySetInnerHTML: { __html: item.highlight } } : {})}>
+						{item.highlight ? undefined : item.task}
 					</div>
 
 					{/* Task Item Footer */}

+ 1 - 1
webview-ui/src/components/history/useTaskSearch.ts

@@ -48,7 +48,7 @@ export const useTaskSearch = () => {
 
 				return {
 					...result.item,
-					task: highlightFzfMatch(
+					highlight: highlightFzfMatch(
 						result.item.task,
 						positions.filter((p) => p < taskEndIndex),
 					),

+ 30 - 1
webview-ui/src/utils/highlight.ts

@@ -1,3 +1,29 @@
+import { LRUCache } from "lru-cache"
+
+// LRU cache for escapeHtml with reasonable size limit
+const escapeHtmlCache = new LRUCache<string, string>({ max: 500 })
+
+function escapeHtml(text: string): string {
+	// Check cache first
+	const cached = escapeHtmlCache.get(text)
+	if (cached !== undefined) {
+		return cached
+	}
+
+	// Compute escaped text
+	const escaped = text
+		.replace(/&/g, "&amp;")
+		.replace(/</g, "&lt;")
+		.replace(/>/g, "&gt;")
+		.replace(/"/g, "&quot;")
+		.replace(/'/g, "&#39;")
+
+	// Cache the result
+	escapeHtmlCache.set(text, escaped)
+
+	return escaped
+}
+
 export function highlightFzfMatch(
 	text: string,
 	positions: number[],
@@ -39,6 +65,9 @@ export function highlightFzfMatch(
 
 	// Build final string
 	return parts
-		.map((part) => (part.highlight ? `<span class="${highlightClassName}">${part.text}</span>` : part.text))
+		.map((part) => {
+			const escapedText = escapeHtml(part.text)
+			return part.highlight ? `<span class="${highlightClassName}">${escapedText}</span>` : escapedText
+		})
 		.join("")
 }