Skip to content

Conversation

@forestyoo
Copy link
Contributor

@forestyoo forestyoo commented Jun 7, 2025

Closes: #3866

Description

This PR fixes TaskItem display and copy issues with HTML tags in task messages.

Display issue: HTML tags not displaying in HistoryView.

  • Reason: In HistoryView, HTML tags within task message are rendered due to the use of dangerouslySetInnerHTML.
  • Solution: Only when the task message needs to be highlighted, should it be escaped and displayed using the dangerouslySetInnerHTML with the escaped string.

Copy issue: HTML tags in message are removed when clicking the copy button in TaskItem.

  • Reason: There was existing code logic that stripped HTML tags from the message during the copy operation, leading to incomplete copied content.
  • Solution:
    1. Removed the aforementioned code logic.
    2. When the task message needs to be highlighted, put the processed message into the highlight field instead of modifying the task field.

Test Procedure

Send Roo the message explain this html code <div>Test</div> to create a task.

  • Display issue: In HistoryView, the <div> tags are not displayed; the message appears as explain this html code Test.
  • Copy issue: When copying the message from HistoryView and pasting it, the copied content is explain this html code Test (HTML tags are lost).

Type of Change

  • 🐛 Bug Fix: Non-breaking change that fixes an issue.
  • New Feature: Non-breaking change that adds functionality.
  • 💥 Breaking Change: Fix or feature that would cause existing functionality to not work as expected.
  • ♻️ Refactor: Code change that neither fixes a bug nor adds a feature.
  • 💅 Style: Changes that do not affect the meaning of the code (white-space, formatting, etc.).
  • 📚 Documentation: Updates to documentation files.
  • ⚙️ Build/CI: Changes to the build process or CI configuration.
  • 🧹 Chore: Other changes that don't modify src or test files.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Code Quality:
    • My code adheres to the project's style guidelines.
    • There are no new linting errors or warnings (npm run lint).
    • All debug code (e.g., console.log) has been removed.
  • Testing:
    • New and/or updated tests have been added to cover my changes.
    • All tests pass locally (npm test).
    • The application builds successfully with my changes.
  • Branch Hygiene: My branch is up-to-date (rebased) with the main branch.
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Changeset: A changeset has been created using npm run changeset if this PR includes user-facing changes or dependency updates.
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

record.mov

Documentation Updates

Additional Notes

Get in Touch

Discord: forestyoo


Important

Fixes display and copy issues with HTML tags in task messages by adjusting HTML handling in TaskItem and CopyButton, and adding HTML escaping in highlight.ts.

  • Display Fix:
    • In TaskItem.tsx, use dangerouslySetInnerHTML only when highlight is present, ensuring HTML tags are displayed correctly.
    • Introduce highlight field in DisplayHistoryItem to store processed message.
  • Copy Fix:
    • In CopyButton.tsx, remove logic stripping HTML tags, allowing full message copy.
  • HTML Escaping:
    • Add escapeHtml() in highlight.ts to escape HTML characters in highlightFzfMatch().
  • Misc:
    • Modify useTaskSearch.ts to use highlight field for storing highlighted messages.

This description was created by Ellipsis for 4322529. You can customize this summary. It will automatically update as commits are pushed.

* Display issue: HTML tags not displaying in HistoryView.
* Copy issue: HTML tags in message are removed when clicking the copy button in TaskItem.
@forestyoo forestyoo requested review from cte, jr and mrubens as code owners June 7, 2025 16:05
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Jun 7, 2025
@daniel-lxs daniel-lxs added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jun 7, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jun 8, 2025
- Add LRU cache with 500 item limit for escapeHtml results
- Improves performance by caching frequently escaped strings
- Reduces redundant HTML escaping operations
Copy link
Member

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

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

Hey @forestyoo, thank you for working on this fix.

It looks good, I added LRU cache to the escapeHtml since this function is called on every render for every single task, this should help with performance.

Other than that this looks like a good fix!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 8, 2025
@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Needs Review] in Roo Code Roadmap Jun 8, 2025
@forestyoo
Copy link
Contributor Author

Hi @daniel-lxs ,

Thanks for the review and for adding the LRU cache optimization—great improvement! Glad the fix looks good.

Appreciate your time!

@daniel-lxs daniel-lxs added PR - Needs Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Jun 9, 2025
@forestyoo
Copy link
Contributor Author

forestyoo commented Jun 10, 2025

Hey @daniel-lxs ,
For this PR, when a task message needs highlighting, I don't modify the task field (instead, the highlighted message goes into the the highlight field). Therefore, I've removed the stripHistoryHighlightSpans logic from CopyButton.

@mrubens mrubens merged commit d5f862a into RooCodeInc:main Jun 12, 2025
10 checks passed
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jun 12, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 12, 2025
@antoine-ak
Copy link

Thank you !!

@forestyoo forestyoo deleted the historyItem branch June 12, 2025 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer PR - Needs Review size:M This PR changes 30-99 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

History view shows HTML from prompts / chat with LLM

4 participants